when the --fdisk and --losetup options were removed, the entries in the
getopt option list should have remained for backwards compatibility such
that the usage warnings can kick in instead of unknown option errors.
Gbp-Dch: Ignore
unless `LIVE_BUILD` is set in the environment when running live-build,
this var will be empty. this will result in the frontend trying to load
the file '/scripts/build.sh', which is doubtful the file intended to be
loaded, if it exists. the intention of checking the path
"${LIVE_BUILD}/scripts/build.sh" is really only to do so if the var is
actually used, so let's only do so if it's non-empty.
since we use `set -e`, if build.sh is not found in either location then
failure will occur. if it is found, presuming it is the real file that we
expect to be found, this file sets the var to a default if it was an empty
string, thus we need not worry about use of the var later in the frontend
script.
also, the var is exported prior to exec'ing the command script, so we know
that in the command scripts it is not going to be empty, and those in
themselves loading build.sh which again exports the var ensures that it
will be set for subsequent loads of component scripts (which happens to
go through the frontend again, not that that matters. so except for at the
start of execution, it should never be found to be empty.
Gbp-Dch: Short
the frontend handles -h|--help directly and correctly redirects to the
man page.
component scripts however fail to load the correct manpage because they
are being directed to `man <script>` instead of `man lb script`.
(affects the top level commands and major build stages which actually have
man pages; the low level components don't and so will always fail anyway).
this is handled for every script in build.sh. this is not stored in the
saved config or anything, so no need to re-evaluate in
`Set_config_defaults`. this just seems to completely pointless.
Gbp-Dch: Short
this function takes one or more required stage fileS _plural_, and exits
if any are missing (or at least it does now after the refactor).
let's rename it to make things more clear
Gbp-Dch: Short
the name of the stage is already printed earlier in the output prior to
the error here being printed. so the error really does not need to include
the script name itself.
now having investigated my suspicions of the functionality and use of
Require_stagefile(), i conclude that it has been fundamentally broken
all the way back to v1.0~a8-1 (or at least usage of it since v1.0.1-2).
gah. (╯°□°)╯︵ ┻━┻
----
very early on in the history of live-build this function took the name of
a _single_ stage file only and did `exit 1` should the file not be found.
this was simple and clearly accomplished "what was on the tin", so to
speak.
in bd1a5ddc82 (2007, 1.0~a8-1) things got
weird. it was modified to support a list of multiple files. but instead of
being written to cause failure if _any_ of the listed files were missing
as perhaps one might expect, it was instead written to fail only if all
files were missing!
if you jump to the conclusion that i'm talking about a simple flipped
logic from a lack or otherwise of a `!` here, you'd be mistaken; there is
a comment inside the function that could not be more clear about what was
intended by the author - "Find at least one of the required stages"! this
makes me thoroughly confused about what they were thinking.
as we'll get to, this was fundamentally flawed (or at least its later use
was), but furthermore there were other notable issues introduced at this
point (but don't worry too much about these, they've all been addressed):
- `NAME` was modified in the loop, using the existing value, but nothing
initially set it...
- the setting of `NAME` seems related to its use in the subsequent error
output, yet they are logically separated; it is only set if a file
exists, while the error is only printed if none exist.
- it is pointlessly using a messy `CONTINUE="true"` based mechanism,
when it could just `return 0`.
- it did not handle correctly the bad use case of no params having been
supplied.
it doesn't seem to have been entirely thought through, despite its
pervasive use throughout the build system.
note that no change was made in that commit to make actual use of the
new multi-param support. it would not be used until about a year later.
the function has remained largely untouched since then. in
c68c0a2708 a notable change was made to add
an initial setting of `NAME`, which partially addressed one of the above
issues. but it did not really address the issue the change was meant to
solve, since the `NAME` as printed in the error was now the name of the
script when what was really wanted was the name of the stagefile. this was
finally fixed properly in d54990695f.
however the weirdly pointless setting of `NAME` persisted in the loop.
finally i personally just refactored the function in the commit prior to
this one, retaining the same functionality but addressing the remaining
of the above minor implementation issues.
looking at usage of the new functionality introduced in
bd1a5ddc82, it does not seem to have been
until 0cbbde2b96 (2008, almost a year after
it was made possible) that changes were made to finally start making use
of the ability to pass more than one filename at a time to the function,
and it would appear that perhaps the author forgot what it actually was
that the function accomplished when used with multiple params, and failed
to double check.
in this first use of multiple parameters, this commit went from passing
single file names to individual calls to the function to passing the files
in one single call, in a commit the purpose of which was described as
simply tidying things up. it was most certainly not intended to change
stage requirements.
unfortunately, a change in requirements did occur as a result since the
new usage of the function was not accomplishing the same as before. this
change completely broke the stage requirements protection mechanism such
that only a single one of the listed stages needed to have completed for
the check to pass, rather than all as expected.
this flaw made it into release v1.0.1-2 and it has existed every since.
in the very next commit from that one,
6204dc0e6d things got even worse. here we
see the config stage being specified commonly as the first stage listed,
which is still the case today. this means that ever since this commit,
if you've already got a config before building (which you inevitably do,
especially after some later commits introduced automatically creating it
if missing), then all other stage requirements are simply ignored.
so it seems pretty damn clear that this function is accomplishing
completely the wrong objective. it _should_ be checking that _all_ files
given to it exist, not just one or more. ¯\_(ツ)_/¯
this FINALLY addresses this mistake.
(not that i wish to berate the author; i've made silly mistakes of my own
before)
- count of params is available as $#, we don't need the pipe-to-wc logic.
- the whole 'CONTINUE' based logic is silly, we can just return once one
of the files is found.
- setting of 'NAME' in the loop was completely pointless.
- the error message for multiple files was not very clear just injecting
a sequence of words into a sentence.
- it did not work correctly if no arguments were given (bad usage)
note, you might question whether the functionality of this function is
correct, as did I; this is tackled in a followup commit whilst this
commit retains the existing functionality!
Gbp-Dch: Short
as suggested by Raphaël
rather than have fixed stagefile filename strings at all in the scripts,
use `$(basename $0)` to use the name of the script (which is the same for
almost all cases anyway, and the stage files are supposed to be almost
exclusively unique per-script). we can thus simplify things by determining
the filename for most use cases within the functions themselves.
this does change the file used by a couple of scripts, affecting backwards
compatibility of executing live-build upon an existing partially or fully
completed build:
- binary_grub-pc used "binary_grub"
- chroot_includes used "includes.chroot"
care had to be taken for the following cases:
- there are some cases like bootstrap_cache, source_debian and
bootstrap_debootstrap which are dealing with more than one file, and/or
otherwise a filename that is not specific to the script itself exactly,
or should not be based upon its name.
- some cases like chroot_cache, bootstrap_cache and
chroot_install-packages need to append something to the end of the name
depending upon which pass/action mode the script is being executed with.
- furthermore in the bootstrap_cache case one of the filenames is used
within the bootstrap_debootstrap and thus needs very careful handling
to be certain that a change in filename of bootstrap_cache does not
break bootstrap_debootstrap.
Gbp-Dch: Short
- avoid all need to pass ".build/" path in stage file names into the
functions
- add a helper to remove a stage file (required to complete the above
properly)
- avoid duplicating filenames within scripts which makes them prone to
mistakes (some instances of which I've actually encountered and had
to fix)
Gbp-Dch: Short
Fixes the following
- Correct version (memtest86/memtest86+) shown instead of fixed 'memtest86+' text
- Ensure correct directory path always used by using replaceable placeholder
Gbp-Dch: Short
all vars affected have been carefully checked to be quite certain
that they are definitely local
where variable is assigned the return value of a function/command, the
local "declaration" is deliberately done on a separate line, since
`local FOO` is actually treated itself as a command rather than a
declaration; will thus always cause $? to be zero, and thus if done on
the same line as such an assignment can not only clobber $? but in doing
so unintentionally blocks failure of a command from triggering the
expected exit from having `set -e`.
also, from testing, i have found that when assigning "${@}" this must be
done on a separate line confusingly as otherwise an error occurs.
Gbp-Dch: Short
The '---' delimiter should appear before the final 'quiet' parameter
(which is used by the debian installer I believe).
This delimiter is added by live-build in syslinux configs, and is present
in both grub2 and syslinux configs in an official debian 7.7 disc image,
suggesting strongly that live-build grub/grub2 menu creation code is in
the wrong here by missing it.
update: this commit previously used -- as was correct at the time, and has
since been updated to use --- per #775128; which was previously tackled in
a separate later commit. the switch to --- was already done for syslinux
(which was not missing the delimiter unlike grub) in
ba6b9adeff
Gbp-Dch: Short
Closes: #775143
I believe that the `quiet` parameter is meant for d-i not the kernel and
thus should be given on the end after a delimiter, as done with syslinux.
Here we switch the order to move it to the end. The addition of the missing
delimiter will be done in a followup commit.
(See #775143)
Gbp-Dch: Short
Current splash makes it very difficult to read menu entries.
Black as a background color is actually interpreted as transparent,
so switching to something else so the highlighted menu entry can be
read more easily.
Gbp-Dch: Short
When building grub2 menu entries the quiet param (meant for d-i)
was excluded from the rescue menu entries instead of expert.
This is the opposite to what is done in the following:
- Menu entries seen in official debian 7.7 disc images (grub2 and syslinux configs)
- Menu entries created for grub (legacy)
- Menu entries created for syslinux
The evidence strongly suggests that the grub2 menu creation was in the wrong!
(See #775143)
Gbp-Dch: Short
thus for some reason if one is connected to a tty and the other a file,
we still get colour in the tty by default.
in terms of options, --color and --no-color override both, no granular
ones added since it's not worth it imo.
this is backwards compatible with custom configs setting `_COLOR`.
it could be argued that setting $_COLOR to "false" for the auto non-tty
cases is redundant, which it is, but it doesn't hurt to do so; it ensures
that if anything (inc. 3rd-party hooks and such) rely on it that it
remains correct; and ensures that if anything in the future mistakenly
uses $_COLOR instead of $_COLOR_OUT|$_COLOR_ERR that at least that will
only be broken for the use case of only one of stdout|sdterr being a tty.
Gbp-Dch: Ignore
...when stdout+stderr connected to a tty (as opposed for example to being
piped to a log file)
very helpful to have colour such that the red/yellow of errors/warnings
can draw the eye to problems.
Gbp-Dch: Short
`DI_PACKAGES` does not need to include `DI_REQ_PACKAGES` so long as
we pass the latter to apt in the one case where it was not already
being given it.
in fact with it including that sub-list meant that in the other
case where it was being given to apt, it actually just resulted in
duplication.
Gbp-Dch: Short
jnqnfe
Raphaël Hertzog