i've been noticing that in the binary_syslinux stage a wholebunch of
packages get installed whilst installing isolinux, and then upon removing
the installed packages at the end of the script, apt is listing a large
number of those that were auto installed as no longer in use and can be
removed via `apt autoremove`. this then persists through package
installation and removal steps throughout remaining scripts.
adding `--auto-remove` to `apt-get remove --purge` and similarly
`--purge-unused` to `aptitude purge` instructs apt to get rid of the unused
auto-installed packages at the same time as removing specific package that
brought them in in the first place, and thus resolves the problem.
Gbp-Dch: Short
For consistency with install entries (both in live-build and
official Debian install discs).
Comparing with live-build created installer entries, grub-legacy
and grub2 both favour vga-788 for GUI entries and vga=normal for
test entries, whilst syslinux uses vga-788 for everything.
Gbp-Dch: Short
apt v2.0.1 introduced support for coloured E:/W:/N: labels. this adds
support to control it based upon our own colour control.
note that with utilities like dpkg we do not do this, but apt only uses
its new colour support automatically when `apt` is used directly, it is
not automatically enabled (per isatty()) for `apt-get`/`aptitude` (the
`apt` developer responsible for adding colour support in response to my
request for it told me that it was deliberately done like this per being
customary to not change behaviour of those tools for compatibility
reasons). colour errors/warnings are useful, so we want to turn it on for
our use of these tools where we can.
Gbp-Dch: Short
...from the `Set_config_defaults` function, to being done directly
in `build.sh` (the component which is also responsible for loading
functions, loaded at the start of every script, including the front
end).
thus the colouring decision will now correctly...
- apply to the frontend, such as to the 'root privileges needed'
error, the 'no such script' error, and the command name
colouring that I want to add (the most significant issue).
- apply to error messages generated by the `Arguments` and
`Read_conffiles` functions, which are called before
`Set_config_defaults` by scripts.
as things were, due to the comparison with "false", colour would
_always_ be used in these places (unless _COLOR_ERR=false or
_COLOR_OUT=false wrt. the new command highlight, were set in the
environment when executing a script throught the frontend).
this would not be a problem for normal terminal use of course,
besides being inconsistent where color were turned off, but would
be a bit of a problem if redirected to a file.
a re-evaluation of _COLOR is performed in `Set_config_defaults` to
adjust _COLOR_OUT and _COLOR_ERR where necessary, to correctly
respond to _COLOR being set in saved config files (disabled by
default but a user could always enable), after the point of config
files being loaded.
_COLOR can still be controlled from the environment just as before,
overriding both _COLOR_OUT and _COLOR_ERR.
note that this does not address the fact that --color|--no-color
do not work in the frontend and thus will not impact the colouring
of to-be-introduced command highlighting. this needs to be
addressed separately.
Gbp-Dch: Short
progress-linux, as discussed in MR #142 ([1]) is a little known distro,
which appears to be little more than a personal project of the original
author of live-build.
given that, the expense of maintaining all of these old hacks for it
cannot be justified. it is not known whether or not live-build is even
used with respect to it since the author abandoned live-build some
years ago.
also, at least one past change in live-build possibly broke progress-linux
compatibility anyway, which would have required progress-linux users of
live-build to use a custom progress-linux config, or a progress-linux
fork of live-build, and there is no knowing how much of the hacks in this
"upstream" codebase any user of progress-linux currently relies upon.
and again, progress-linux appears to just be a personal project of
Daniel's, with afaik very little userbase. (Daniel seems to be the only
developer working on the project which speaks to how small it is).
[1]: https://salsa.debian.org/live-team/live-build/-/merge_requests/142
Gbp-Dch: Short
seems to have been obsolete since all the way back at v1.0~a22-1.
history:
- in 0d0de885e3 it was renamed from
LIVE_ROOT to LH_ROOT, but also seems to have become completely
obsolete at this point, and thus mistakenly not actually removed.
before this it seems to have been used to hold the base directory of
live-build from which paths were constructed, but then this use was
removed making it redundant, but it remained in the code.
- 83bc63f725 renamed to LB_ROOT.
- a79a5bea10 dropped setting the variable
only if not already set, in favour of always setting it depending upon
LB_MODE. but still it remained unused.
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
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
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
Combine the check+create done in each script. (The original functions
are still callable as before, but a new combined `Aquire_lockfile`
function can be called instead, as now used).
Note, a further simplification could be done in removing the passing of
the lock filename in as a parameter since every use of the functions is
with ".lock". The lock functions already have a fallback to ".build/lock"
though. Checking the history, the fallback used to be for a system wide
lock, which was then replaced with this config-tree specific one. As long
as that is not used implicitly by 3rd-party hooks then surely we are free
to change the fallback to ".lock" and further remove passing in a name as
a param...?
history:
db5d2b0dcd0aa8289a37
Gbp-Dch: Short
Closes: #952918
previously this was white+bold. the white aspect was dropped since this
would not be sensible for users with a white background for their terminal.
bold however does not have any effect for me at least so effectively there
is no highlighting at all.
here we reintroduce a colour, one that will work for both black and white
backgrounds of course. purple looks good to me - significantly different
to that used for errors and warnings, and works well with command
highlighting (as enabled separately).
if a script exits due to a failure and `set -e`, we should ensure that an
error message is printed to be clear to the user that something actually
went wrong.
similarly it would be good to print a suitable message should the user
cancel with ctrl+c for instance.
Gbp-Dch: Short
(part of never completed side-by-side multi archi support)
Several scripts make a call to a function called Check_multiarchitectures,
the purpose of which is to adjust the target directory that certain 'live'
and 'install' files are located in. The idea is that a script sets up
'DESTDIR', 'DESTDIR_LIVE' and 'DESTDIR_INSTALL' as appropriate and then
the script appends a suitable arch dependant postfix to the directory
name, depending upon the arch currently being targetted. This would allow
the script to be run multiple times, each for a different architecture.
This is a part of an implementation of allowing multiple architectures to
sit side by side within the same live image, selectable from the
bootloader menus. (As opposed to multiple architectures mixed within the
same userland).
This is evidently the case both from the fact that:
1) The arch specific postfix chosen in that function depends on a var
called LB_CURRENT_ARCHITECTURE, which is never set. In fact going
back through the git history to the introduction of the function in
0d5ff4ca75, the var (even considering
var name changes) has never been set by anything. So effectively the
call to the function has been entirely redundant all this time.
2) The major build stages do not perform multiple executions of substages
per arch. Thus from this perspective it seems that the support was
never fully implemented.
3) If any doubt remained, there is an old branch called 'tmp-multiarch'
which has a couple of commits making progress with completing support,
such as implementing the above missing pieces.
The above mentioned branch is 10 years old and can be considered abandoned.
It is not clear whether the original author ever intended to complete and
merge this; nor is it at all clear at what stage of completion it was at.
At any rate, imo it is not at all particularly useful to have extra code
and complexity in order to be able to cram multiple environments side by
side in one image, not when CDs/DVDs and even to some extent USB pen drives
are so cheap. And who really needs more than one environment so
desperately on just one such medium.
If this was not enough to justify removal, then there is also the fact that
the support that was implemented has become completely broken over the
years with scripts diverging in terms of the variable names the function
modifies such that they are incompatible with it.
A quick assessment of the state of this latter aspect:
good:
- grub-legacy uses the correct var names so is fine
- memtest similarly good
- installer_debian-installer looks okay
questionable:
- binary_linux-image uses the correct vars but might not select the
right kernel and initrd files to copy (seems to copy all)
bad:
- grub-pc is making a redundant call, after functionality was moved
to the loopback script
- loopback is using the wrong vars (INITFS instead of DESTDIR +
DESTDIR_INSTALL + DESTDIR_LIVE), plus is doing its own amd64+i686
thing anyway, so the function call would achieve nothing anyway.
- syslinux is also using the wrong var names so would not work with
it and is not even making the necessary function call. Also the
install paths are fixed in the hard coded cfg files anyway so this
would need addressing with placeholders and sed replacement, but
then it is not entirely clear how things should work with respect
to install entries and multi-arch anyway, are we having multiple
copies of the installer, one for each target arch and then multiple
copies of the install menus, perhaps under different submenus?
So, this removes the artefacts of this never completed feature.
Gbp-Dch: Short
all echo helpers are used as logging functions with output to go to the
terminal. when used in functions that are designed to return a string
though the message printed would get incorrectly captured.
the previous fix done in e3a987d977 was
stupidly flawed; somehow my testing led me to mistakenly believe that
was adequate, but retesting proves that it was not.
here we create a new FD #3 linked to stdout to output the messages on,
which testing shows works as I had actually intended it.
e.g. here:
```
Foo () { if [ "$1" = "a" ]; then printf "foo\n"; else printf "error\n"; fi; }
```
we get:
```
~$ Foo a
foo
~$ Foo b
error
~$ XX="$(Foo a)"
~$ echo "${XX}"
foo
~$ XX="$(Foo b)"
~$ echo "${XX}"
error
```
and as demonstrated, "error" got incorrectly captured by in the variable
whereas here:
```
exec 3>&1
Foo () { if [ "$1" = "a" ]; then printf "foo\n"; else printf "error\n" >&3; fi; }
```
it is different in the last case:
```
~$ XX="$(Foo b)"
error
~$ echo "${XX}"
```
the error successfully makes it to the terminal, and the variable is an
empty string (with a newline automatically printed).
Gbp-Dch: Short
Instances of:
if [ $(which <command> ]
have been replaced with:
if command -v <command> >/dev/null
which is considered to be more robust in a range of environments.
scripts/build/chroot_archives: line 259:
if [ "${LB_APT}" = "aptitude" ] && [ ! $(Chroot chroot "which aptitude") ]
has been left untouched because the chroot might require a more complex command
which would need more testing.
manpages/Makefile: line 42:
@if [ ! -x "$$(which po4a 2>/dev/null)" ]; \
has been left untouched because I am not sufficiently familiar with makefiles.
commit f811656150 enabled the grub-efi
bootloader by default for amd64|i386 architectures, but failed to
recognise the this bootloader is not supported for hdd|netboot images.
this meants that if a user tried to build such an image without explicitly
specifying the bootloader, excluding grub-efi, their build would fail
with an error in the binary_grub-efi stage.
this fixes the problem by only enabling grub-efi by default on supported
image builds.
677415f6d7 (2007) in v1.0~a2-1 added a hack
relating to the loop-aes-utils package and losetup. this commit bundled
a bunch of changes, it was not specific to the hack, and so info about the
hack is limited to a brief comment included within the related change in
defaults:
```
# Workaround for loop-aes-utils divertion
# (loop-aes-utils' losetup lacks features).
```
though it is very similar to the removed fdisk hack in that it seems that
one package may replace a binary from another, moving the original to a
new location, and this hack gives the user the opportunity to select the
original instead of the one put in its place, for use in LB.
the comment mentions a package called loop-aes-utils as being the package
that performs such a diversion, and that the need for the hack was that
losetup itself lacked features, presumably encryption support, and it is
clear that it is the losetup binary that is the focus of the diversion.
looking into the history of loop-aes-utils a little, this package was
dropped from debian back in 2012 (#680748), favouring encrytion support of
dm-crypt/cryptsetup.
double checking file contents of packages, only the mount package carries
an /sbin/losetup file, so presumably this means that dm-setup/cryptsetup
do not perform such a diversion of losetup (i.e. their use is exclusively
done directly).
since the possible diversion is simply gone, that completely removes any
point in having the hack of giving users choice between losetup and the
diverted one. so let's remove this obsolete hack...
8321653cb3 (from 2007) introduced a hack to
work around bug #445304 in gnu-fdisk for users who may have replaced fdisk
with the classic gnu version. the hack allowed users to select an alternate
fdisk binary to use to work around the buggy binary.
bug #445304 is marked as found in v1.0-1 and fixed in v1.2-1, though may
have been fixe din v1.1. it was marked fixed in 2009.
checking the package archive, gnu-fdisk does not actually exist anymore
in debian, with one exception - it is available for arm64 on sid via
debports, and that version is 1.3 so thus includes the necessary fix
anyway.
it is thus pointless now that we still carry this hack.
Gbp-Dch: Short
Current versions of the project files are built upon versions published
and licensed by Daniel Baumann, but are modified copies of those files and
thus need to be marked as such per licensing requirements (afaik he did
not pass along ownership / licensing rights to anyone when he left the
project). We should also be careful to not be misrepresenting such
modified copies as being attributed to Daniel.
Adding a new copyright line referring to "The Debian Live team" should
suffice for this.
The authorship block in man pages has also similarly been updated.
Notes:
- tweaked a copy of daniel copyright lines stating 2014 instead of 2015.
both of these cases were in files that i had personally introduced in
some of my past merged commits that moved some code around. i don't know
why they stated 2014.
- binary_onie was introduced in 2018, so that has a 2018 date instead of
2016 unlike the rest.
- 'efi-image' is a 3rd-party (Canonical Ltd) work that we bundle, but it
has been modified by 674794a8f4 and
36a3ba7634 so I similarly added a
debian live copyright line.
- 'grub-cpmodules' is similar. it was only changed by the indentation fix
of 36a3ba7634 but modification is
modification, and this does help cover any possible future changes that
might be made.
if you execute the bootstrap stage with no internet connection, you get
the following output:
```
[2020-03-10 19:18:46] lb bootstrap
P: Setting up clean exit handler
[2020-03-10 19:18:46] lb bootstrap_cache restore
[2020-03-10 19:18:46] lb bootstrap_debootstrap
P: Begin bootstrapping system...
P: If the following stage fails, the most likely cause of the problem is with your mirror configuration or a caching proxy.
P: Running debootstrap (download-only)...
I: Retrieving InRelease
I: Retrieving Release
E: Failed getting release file http://deb.debian.org/debian/dists/buster/Release
P: Begin unmounting filesystems...
P: Saving caches...
chroot: failed to run command ‘/usr/bin/env’: No such file or directory
```
the last line looked suspicious. investigating it turns out that there was
a deficiency in the exit handler.
when debootstrap fails to download what it needs due to lack of a
connection, that failure due to `set -e` causes the Exit() handler to kick
in. Part of this includes outputting the "Saving caches..." line, before
then making a call to Save_package_cache(). That in turn runs the following
command:
```
Chroot chroot "apt-get autoclean" || true
```
The Chroot() function includes a line starting with:
```
${_LINUX32} chroot "${CHROOT}" /usr/bin/env
```
which is the source of the last output line.
the reason we see this unexpected output is that with bootstrapping having
failed, there is no /usr/bin/env within the chroot so it is bound to fail.
the fact is, the exit handler has no business trying to pretty much
anything that it does if the bootstrap_debootstrap stage has not
completed.
this implements such a restriction and thus resolves the problem of this
unexpected and confusing output in the described situation.
we will now see:
```
[2020-03-10 19:18:46] lb bootstrap
P: Setting up clean exit handler
[2020-03-10 19:18:46] lb bootstrap_cache restore
[2020-03-10 19:18:46] lb bootstrap_debootstrap
P: Begin bootstrapping system...
P: If the following stage fails, the most likely cause of the problem is with your mirror configuration or a caching proxy.
P: Running debootstrap (download-only)...
I: Retrieving InRelease
I: Retrieving Release
E: Failed getting release file http://deb.debian.org/debian/dists/buster/Release
```
LB_APT_SOURCE_ARCHIVES determines whether or not deb-src entries are
desired to be included in apt's sources.list. here, instead of excuding
them we always include them but commented out where they would previously
have been excluded. this means that if a user later changes their mind and
wants to make use of them all they have to do is uncomment them rather
than add the necessary lines.
Gbp-Dch: Short
Closes: #952929
all scripts use `set -e` which means that if getop fails, the subsequent
error check that would print an error in addition to any printed by getopt
itself would never actually be reached.
the first though here would be to remove the pointless error check, but
getopt does not include the word "error" with an unrecognised option
failure, nor does it use colour to highlight problems, both of which mean
that it is a little lacking in terms of highlighting problems to users.
thus we properly capture and use the exit code here and output an
appropriate message per invalid argument vs getopt internal error.
also, removed the redundant stderr redirection which is already done
by Echo_error().
Gbp-Dch: Short
Lyndon Brown
Raphaël Hertzog
Luca Boccassi
johnraff