From c9e7ab8c29a21f994b670ad6703e63940acf4480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Jul 2022 07:55:37 +0200 Subject: [PATCH 01/13] kernel-install: fix invocation as installkernel Fixes #23681. --- src/kernel-install/kernel-install.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 96084860fd..6f5803584e 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -83,7 +83,7 @@ if [ "${0##*/}" = "installkernel" ]; then # kernel's install.sh invokes us as # /sbin/installkernel # We ignore the last two arguments. - set -- "$1" + set -- "${1:?}" "${2:?}" else COMMAND="$1" [ $# -ge 1 ] && shift From 5e193bef898d9851fcee6e7d01e5728f03b43868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Jul 2022 09:09:28 +0200 Subject: [PATCH 02/13] kernel-install: do not let config file override variables This implementation is pretty ugly, but POSIX sh doesn't allow indirect variable expansion, so I couldn't figure out a way to make it less verbose. --- src/kernel-install/kernel-install.in | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 6f5803584e..c3181ef5f5 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -101,9 +101,12 @@ else shift fi -# These two settings are settable in install.conf +# These two settings are only settable via install.conf layout= initrd_generator= +# These two settings can be inherited from the environment +_MACHINE_ID_SAVED="$MACHINE_ID" +_BOOT_ROOT_SAVED="$BOOT_ROOT" if [ -r "/etc/kernel/install.conf" ]; then install_conf="/etc/kernel/install.conf" @@ -117,7 +120,6 @@ if [ -n "$install_conf" ]; then [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && echo "Reading $install_conf…" # shellcheck source=/dev/null . "$install_conf" - # FIXME: This may override configuration in environment variables, e.g. $BOOT_ROOT. fi [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && [ -n "$layout" ] && \ @@ -125,10 +127,23 @@ fi [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && [ -n "$initrd_generator" ] && \ echo "$install_conf configures initrd_generator=$initrd_generator" -[ -n "$MACHINE_ID" ] && [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ - echo "machine-id $MACHINE_ID set via environment or install.conf" -[ -n "$BOOT_ROOT" ] && [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ - echo "BOOT_ROOT=$BOOT_ROOT set via environment or install.conf" +if [ -n "$_MACHINE_ID_SAVED" ]; then + MACHINE_ID="$_MACHINE_ID_SAVED" + [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ + echo "MACHINE_ID=$MACHINE_ID set via environment" +else + [ -n "$MACHINE_ID" ] && [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ + echo "MACHINE_ID=$MACHINE_ID set via install.conf" +fi + +if [ -n "$_BOOT_ROOT_SAVED" ]; then + BOOT_ROOT="$_BOOT_ROOT_SAVED" + [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ + echo "BOOT_ROOT=$BOOT_ROOT set via environment" +else + [ -n "$BOOT_ROOT" ] && [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ + echo "BOOT_ROOT=$BOOT_ROOT set via install.conf" +fi # If /etc/machine-id is initialized we'll use it, otherwise we'll use a freshly # generated one. If the user configured an explicit machine ID to use in From f5f5047ff1d4008687186a45066f94f5b3a830fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Jul 2022 10:52:12 +0200 Subject: [PATCH 03/13] man: rework documentation of kernel-install config Variables read by kernel-install and those exported by it were described without any clear separation. So in particular it was pretty hard to answer a question like "what variables can be set in install.conf". The in- and out-variables are now split into two separate subsections. --- man/kernel-install.xml | 155 ++++++++++++++++++++++++----------------- 1 file changed, 90 insertions(+), 65 deletions(-) diff --git a/man/kernel-install.xml b/man/kernel-install.xml index 5ae86aca18..bb22443854 100644 --- a/man/kernel-install.xml +++ b/man/kernel-install.xml @@ -144,6 +144,7 @@ The <varname>$BOOT</varname> partition + The partition where the kernels and Boot Loader Specification snippets are located is called $BOOT. kernel-install determines the location of this partition by checking @@ -173,77 +174,91 @@ Environment variables - If is used, $KERNEL_INSTALL_VERBOSE=1 will be set for - the plugins. They may output additional logs in this case. + + Environment variables exported for plugins - If $MACHINE_ID is set and not empty when kernel-install is - invoked, it will be used as MACHINE-ID, overriding any automatic detection - attempts. The value must be a valid machine ID (32 hexadecimal characters). + If is used, $KERNEL_INSTALL_VERBOSE=1 will be + exported for plugins. They may output additional logs in this case. - $KERNEL_INSTALL_MACHINE_ID is set for the plugins to the desired - MACHINE-ID to use. It's always a 128bit ID, and typically the ID from - /etc/machine-id or the one passed in via $MACHINE_ID. (If no - machine ID was specified via these methods it might be generated randomly by - kernel-install, in which case it only applies to this invocation.) + $KERNEL_INSTALL_MACHINE_ID is set for the plugins to the desired machine-id to + use. It's always a 128-bit ID. Normally it's read from /etc/machine-id, but it can + also be overriden via $MACHINE_ID (see below). If not specified via these methods a + fallback value will generated by kernel-install, and used only for a single + invocation. - $KERNEL_INSTALL_ENTRY_TOKEN is set for the plugins to the desired entry "token" - to use. It's an identifier that shall be used to identify the local installation, and is often the - machine ID, i.e. same as $KERNEL_INSTALL_MACHINE_ID, but might also be a different - type of identifier, for example a fixed string or the ID=, - IMAGE_ID= values from /etc/os-release. The string passed here - will be used to name Boot Loader Specification entries, or the directories the kernel image and initial - RAM disk images are placed into. Note that while oftentimes - $KERNEL_INSTALL_ENTRY_TOKEN and $KERNEL_INSTALL_MACHINE_ID are set - to the same value, the latter is guaranteed to be a valid 32 character ID in lowercase hexadecimals while - the former can be any short string. The entry token to use is read from - /etc/kernel/entry-token, if it exists. Otherwise a few possible candidates below the - $BOOT are searched for Boot Loader Specification Type 1 entry directories, and if - found the entry token is derived from that. If that is not successful the machine ID is used as - fallback. + $KERNEL_INSTALL_ENTRY_TOKEN is set for the plugins to the desired entry + "token" to use. It's an identifier that shall be used to identify the local installation, and is often + the machine ID, i.e. same as $KERNEL_INSTALL_MACHINE_ID, but might also be a + different type of identifier, for example a fixed string or the ID=, + IMAGE_ID= values from /etc/os-release. The string passed here + will be used to name Boot Loader Specification entries, or the directories the kernel image and initial + RAM disk images are placed into. - $KERNEL_INSTALL_BOOT_ROOT is set for the plugins to the absolute path of the - root directory (mount point, usually) of the hierarchy where boot loader entries, kernel images, and - associated resources should be placed. This usually is the path where the XBOOTLDR partition or the ESP - (EFI System Partition) are mounted, and also conceptually referred to as $BOOT. Can be - overridden by setting $BOOT_ROOT. + Note that while $KERNEL_INSTALL_ENTRY_TOKEN and + $KERNEL_INSTALL_MACHINE_ID are often set to the same value, the latter is guaranteed + to be a valid 32 character ID in lowercase hexadecimals while the former can be any short string. The + entry token to use is read from /etc/kernel/entry-token, if it exists. Otherwise a + few possible candidates below $BOOT are checked for Boot Loader Specification Type 1 + entry directories, and if found the entry token is derived from that. If that is not successful, + $KERNEL_INSTALL_MACHINE_ID is used as fallback. - $KERNEL_INSTALL_LAYOUT=bls|other|... is set for the plugins to specify the - installation layout. Defaults to if - $BOOT/ENTRY-TOKEN exists, or - otherwise. Additional layout names may be defined by convention. If a plugin uses a special layout, it's - encouraged to declare its own layout name and configure layout= in - install.conf upon initial installation. The following values are currently - understood: + $KERNEL_INSTALL_BOOT_ROOT is set for the plugins to the absolute path of the + root directory (mount point, usually) of the hierarchy where boot loader entries, kernel images, and + associated resources should be placed. This usually is the path where the XBOOTLDR partition or the ESP + (EFI System Partition) are mounted, and also conceptually referred to as $BOOT. Can + be overridden by setting $BOOT_ROOT (see below). - - - bls - - Standard Boot Loader - Specification Type #1 layout, compatible with - systemd-boot7: - entries in - $BOOT/loader/entries/ENTRY-TOKEN-KERNEL-VERSION[+TRIES].conf, - kernel and initrds under - $BOOT/ENTRY-TOKEN/KERNEL-VERSION/ - Implemented by 90-loaderentry.install. - - - - other - - Some other layout not understood natively by kernel-install. - - - + $KERNEL_INSTALL_LAYOUT=bls|other|... is set for the plugins to specify the + installation layout. Defaults to if + $BOOT/ENTRY-TOKEN exists, or + otherwise. Additional layout names may be defined by convention. If a plugin uses a special layout, + it's encouraged to declare its own layout name and configure layout= in + install.conf upon initial installation. The following values are currently + understood: - $KERNEL_INSTALL_INITRD_GENERATOR is set for plugins to select the initrd - generator. This may be configured as initrd_generator= in - install.conf. See below. + + + bls + + Standard Boot Loader + Specification Type #1 layout, compatible with + systemd-boot7: + entries in + $BOOT/loader/entries/ENTRY-TOKEN-KERNEL-VERSION[+TRIES].conf, + kernel and initrds under + $BOOT/ENTRY-TOKEN/KERNEL-VERSION/ + Implemented by 90-loaderentry.install. + + + + other + + Some other layout not understood natively by kernel-install. + + + - $KERNEL_INSTALL_STAGING_AREA is set for plugins to a path to a directory. - Plugins may drop files in that directory, and they will be installed as part of the loader entry, based - on the file name and extension. + $KERNEL_INSTALL_INITRD_GENERATOR is set for plugins to select the initrd + generator. This may be configured as initrd_generator= in + install.conf, see below. + + $KERNEL_INSTALL_STAGING_AREA is set for plugins to a path to a directory. + Plugins may drop files in that directory, and they will be installed as part of the loader entry, based + on the file name and extension. + + + + Environment variables understood by <command>kernel-install</command> + + $MACHINE_ID can be set for kernel-install to override + $KERNEL_INSTALL_MACHINE_ID, the machine ID. + + $BOOT_ROOT can be set for kernel-install to override + $KERNEL_INSTALL_BOOT_ROOT, the installation location for boot entries. + + Those variables may also be set in install.conf. Variables set in the + environment take precedence over the values specified in the config file. + @@ -327,8 +342,18 @@ Configuration options for kernel-install, as a series of KEY=VALUE assignments, compatible with shell - syntax. This currently supports two keys: layout= and - initrd_generator=, for details see the Environment variables section above. + syntax, following the same rules as described in + os-release5. + /etc/kernel/install.conf will be read if present, and + /usr/lib/kernel/install.conf otherwise. This file is optional. + + + Currently, the following keys are supported: + MACHINE_ID=, + BOOT_ROOT=, + layout=, + initrd_generator=. + See the Environment variables section above for details. From 035f8acdf737ee5e0c49feaa14dd8cfcea0ffa5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Jul 2022 10:58:01 +0200 Subject: [PATCH 04/13] kernel-install: do not silently ignore files we can't read 'test -r' is changed to 'test -f' everywhere. If the file exists but we cannot read it, it would be better if we fail with a permission error. E.g. if /etc/kernel/cmdline is unreadable, and we're running something as non-root, we shouldn't fall back to /usr/lib/kernel/cmdline. This commit doesn't resolve this fully, because we're not running with 'set -e', but this is a preparator step. --- src/kernel-install/90-loaderentry.install | 10 +++++----- src/kernel-install/kernel-install.in | 16 ++++++++++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/kernel-install/90-loaderentry.install b/src/kernel-install/90-loaderentry.install index 549437c7cd..ee55965110 100644 --- a/src/kernel-install/90-loaderentry.install +++ b/src/kernel-install/90-loaderentry.install @@ -52,10 +52,10 @@ case "$COMMAND" in ;; esac -if [ -r /etc/os-release ]; then +if [ -f /etc/os-release ]; then # shellcheck source=/dev/null . /etc/os-release -elif [ -r /usr/lib/os-release ]; then +elif [ -f /usr/lib/os-release ]; then # shellcheck source=/dev/null . /usr/lib/os-release fi @@ -65,9 +65,9 @@ fi SORT_KEY="$IMAGE_ID" [ -z "$SORT_KEY" ] && SORT_KEY="$ID" -if [ -r /etc/kernel/cmdline ]; then +if [ -f /etc/kernel/cmdline ]; then BOOT_OPTIONS="$(tr -s "$IFS" ' ' &2 diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index c3181ef5f5..044ba9f6f2 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -108,9 +108,9 @@ initrd_generator= _MACHINE_ID_SAVED="$MACHINE_ID" _BOOT_ROOT_SAVED="$BOOT_ROOT" -if [ -r "/etc/kernel/install.conf" ]; then +if [ -f "/etc/kernel/install.conf" ]; then install_conf="/etc/kernel/install.conf" -elif [ -r "/usr/lib/kernel/install.conf" ]; then +elif [ -f "/usr/lib/kernel/install.conf" ]; then install_conf="/usr/lib/kernel/install.conf" else install_conf= @@ -150,11 +150,14 @@ fi # /etc/machine-info to use for our purpose, we'll use that instead (for # compatibility). # shellcheck source=/dev/null -if [ -z "$MACHINE_ID" ] && [ -r /etc/machine-info ] && . /etc/machine-info && MACHINE_ID="$KERNEL_INSTALL_MACHINE_ID"; then +if [ -z "$MACHINE_ID" ] && [ -f /etc/machine-info ]; then + . /etc/machine-info + MACHINE_ID="$KERNEL_INSTALL_MACHINE_ID" [ -n "$MACHINE_ID" ] && [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ echo "machine-id $MACHINE_ID acquired from /etc/machine-info" fi -if [ -z "$MACHINE_ID" ] && [ -r /etc/machine-id ] && read -r MACHINE_ID Date: Fri, 1 Jul 2022 10:58:01 +0200 Subject: [PATCH 05/13] kernel-install: allow overriding the path to config files It's pretty hard to write tests without this. I started out by adding separate variables for each of the files we read, but there's a bunch, and in practice it's good enough to just override the directory. --- man/kernel-install.xml | 23 ++++++++++++++++------- src/kernel-install/90-loaderentry.install | 14 ++++++++++---- src/kernel-install/kernel-install.in | 14 +++++++++----- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/man/kernel-install.xml b/man/kernel-install.xml index bb22443854..bde30ab16a 100644 --- a/man/kernel-install.xml +++ b/man/kernel-install.xml @@ -250,13 +250,18 @@ Environment variables understood by <command>kernel-install</command> + $KERNEL_INSTALL_CONF_ROOT can be set to override the location of the + configuration files read by kernel-install. When set, + install.conf, entry-token, and other files will be + read from this directory. + $MACHINE_ID can be set for kernel-install to override $KERNEL_INSTALL_MACHINE_ID, the machine ID. $BOOT_ROOT can be set for kernel-install to override $KERNEL_INSTALL_BOOT_ROOT, the installation location for boot entries. - Those variables may also be set in install.conf. Variables set in the + The last two variables may also be set in install.conf. Variables set in the environment take precedence over the values specified in the config file. @@ -286,9 +291,10 @@ Read by 90-loaderentry.install. The content of the file - /etc/kernel/cmdline specifies the kernel command line to use. If that file does not - exist, /usr/lib/kernel/cmdline is used. If that also does not exist, - /proc/cmdline is used. + /etc/kernel/cmdline specifies the kernel command line to use. If that file + does not exist, /usr/lib/kernel/cmdline is used. If that also does not + exist, /proc/cmdline is used. $KERNEL_INSTALL_CONF_ROOT + may be used to override the path. @@ -301,7 +307,8 @@ $BOOT/loader/entries/MACHINE-ID-KERNEL-VERSION+TRIES.conf. This is useful for boot loaders such as systemd-boot7 which - implement boot attempt counting with a counter embedded in the entry file name. + implement boot attempt counting with a counter embedded in the entry file name. + $KERNEL_INSTALL_CONF_ROOT may be used to override the path. @@ -310,8 +317,9 @@ If this file exists it is read and used as "entry token" for this system, i.e. is used for - naming Boot Loader Specification entries, see - $KERNEL_INSTALL_ENTRY_TOKEN above for details. + naming Boot Loader Specification entries, see $KERNEL_INSTALL_ENTRY_TOKEN + above for details. $KERNEL_INSTALL_CONF_ROOT may be used to override the + path. @@ -346,6 +354,7 @@ os-release5. /etc/kernel/install.conf will be read if present, and /usr/lib/kernel/install.conf otherwise. This file is optional. + $KERNEL_INSTALL_CONF_ROOT may be used to override the path. Currently, the following keys are supported: diff --git a/src/kernel-install/90-loaderentry.install b/src/kernel-install/90-loaderentry.install index ee55965110..b700a7b2a6 100644 --- a/src/kernel-install/90-loaderentry.install +++ b/src/kernel-install/90-loaderentry.install @@ -65,7 +65,11 @@ fi SORT_KEY="$IMAGE_ID" [ -z "$SORT_KEY" ] && SORT_KEY="$ID" -if [ -f /etc/kernel/cmdline ]; then +if [ -n "$KERNEL_INSTALL_CONF_ROOT" ]; then + if [ -f "$KERNEL_INSTALL_CONF_ROOT/cmdline" ]; then + BOOT_OPTIONS="$(tr -s "$IFS" ' ' <"$KERNEL_INSTALL_CONF_ROOT/cmdline")" + fi +elif [ -f /etc/kernel/cmdline ]; then BOOT_OPTIONS="$(tr -s "$IFS" ' ' &2 + echo "$TRIES_FILE does not contain an integer." >&2 exit 1 fi LOADER_ENTRY="$BOOT_ROOT/loader/entries/$ENTRY_TOKEN-$KERNEL_VERSION+$TRIES.conf" diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 044ba9f6f2..96595fa295 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -108,7 +108,9 @@ initrd_generator= _MACHINE_ID_SAVED="$MACHINE_ID" _BOOT_ROOT_SAVED="$BOOT_ROOT" -if [ -f "/etc/kernel/install.conf" ]; then +if [ -n "$KERNEL_INSTALL_CONF_ROOT" ]; then + install_conf="$KERNEL_INSTALL_CONF_ROOT/install.conf" +elif [ -f "/etc/kernel/install.conf" ]; then install_conf="/etc/kernel/install.conf" elif [ -f "/usr/lib/kernel/install.conf" ]; then install_conf="/usr/lib/kernel/install.conf" @@ -116,7 +118,7 @@ else install_conf= fi -if [ -n "$install_conf" ]; then +if [ -f "$install_conf" ]; then [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && echo "Reading $install_conf…" # shellcheck source=/dev/null . "$install_conf" @@ -171,10 +173,12 @@ fi # $BOOT where we want to place the kernel/initrd and related resources, as well # for naming the .conf boot loader spec entry. Typically this is just the # machine ID, but it can be anything else, too, if we are told so. -if [ -z "$ENTRY_TOKEN" ] && [ -f /etc/kernel/entry-token ]; then - read -r ENTRY_TOKEN Date: Fri, 1 Jul 2022 12:50:17 +0200 Subject: [PATCH 06/13] kernel-install/90-loaderentry: make ownership change optional This can be useful when running as non-root, e.g. for tests or when installing to a chroot. --- src/kernel-install/90-loaderentry.install | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/kernel-install/90-loaderentry.install b/src/kernel-install/90-loaderentry.install index b700a7b2a6..d6b90a7b1d 100644 --- a/src/kernel-install/90-loaderentry.install +++ b/src/kernel-install/90-loaderentry.install @@ -105,10 +105,11 @@ if ! [ -d "$ENTRY_DIR_ABS" ]; then exit 1 fi -install -g root -o root -m 0644 "$KERNEL_IMAGE" "$ENTRY_DIR_ABS/linux" || { +install -m 0644 "$KERNEL_IMAGE" "$ENTRY_DIR_ABS/linux" || { echo "Error: could not copy '$KERNEL_IMAGE' to '$ENTRY_DIR_ABS/linux'." >&2 exit 1 } +chown root.root "$ENTRY_DIR_ABS/linux" || : shift "$INITRD_OPTIONS_SHIFT" # All files listed as arguments, and staged files starting with "initrd" are installed as initrds. @@ -121,10 +122,11 @@ for initrd in "$@" "${KERNEL_INSTALL_STAGING_AREA}"/initrd*; do initrd_basename="${initrd##*/}" [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && echo "Installing $ENTRY_DIR_ABS/$initrd_basename" - install -g root -o root -m 0644 "$initrd" "$ENTRY_DIR_ABS/$initrd_basename" || { + install -m 0644 "$initrd" "$ENTRY_DIR_ABS/$initrd_basename" || { echo "Error: could not copy '$initrd' to '$ENTRY_DIR_ABS/$initrd_basename'." >&2 exit 1 } + chown root.root "$ENTRY_DIR_ABS/$initrd_basename" || : done mkdir -p "${LOADER_ENTRY%/*}" || { From 6755285ccb55fc1f790e0a2c5202987acb52c98a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Jul 2022 12:55:32 +0200 Subject: [PATCH 07/13] kernel-install: allow overriding the plugin list too The use of IFS= is dropped. Let's just iterate over the list, using any whitespace as separator. --- man/kernel-install.xml | 5 +++++ src/kernel-install/kernel-install.in | 25 +++++++++++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/man/kernel-install.xml b/man/kernel-install.xml index bde30ab16a..7ad7f86c6d 100644 --- a/man/kernel-install.xml +++ b/man/kernel-install.xml @@ -255,6 +255,11 @@ install.conf, entry-token, and other files will be read from this directory. + $KERNEL_INSTALL_PLUGINS can be set to override the list of plugins executed by + kernel-install. The argument is a whitespace-separated list of paths. + KERNEL_INSTALL_PLUGINS=: may be used to prevent any plugins from running. + + $MACHINE_ID can be set for kernel-install to override $KERNEL_INSTALL_MACHINE_ID, the machine ID. diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 96595fa295..30b28f4658 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -312,15 +312,20 @@ MAKE_ENTRY_DIR_ABS=$? ret=0 -PLUGINS="$( - dropindirs_sort ".install" \ - "/etc/kernel/install.d" \ - "/usr/lib/kernel/install.d" -)" -IFS=" -" +if [ -z "$KERNEL_INSTALL_PLUGINS" ]; then + KERNEL_INSTALL_PLUGINS="$( + dropindirs_sort ".install" \ + "/etc/kernel/install.d" \ + "/usr/lib/kernel/install.d" + )" +fi -[ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && printf '%s\n' "Plugin files:" "$PLUGINS" +if [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ]; then + printf '%s\n' "Plugin files:" + for f in $KERNEL_INSTALL_PLUGINS; do + printf '%s\n' "$f" + done +fi case "$COMMAND" in add) @@ -346,7 +351,7 @@ case "$COMMAND" in fi fi - for f in $PLUGINS; do + for f in $KERNEL_INSTALL_PLUGINS; do [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && echo "+$f add $KERNEL_VERSION $ENTRY_DIR_ABS" "$@" "$f" add "$KERNEL_VERSION" "$ENTRY_DIR_ABS" "$@" @@ -357,7 +362,7 @@ case "$COMMAND" in ;; remove) - for f in $PLUGINS; do + for f in $KERNEL_INSTALL_PLUGINS; do [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && echo "+$f remove $KERNEL_VERSION $ENTRY_DIR_ABS" "$f" remove "$KERNEL_VERSION" "$ENTRY_DIR_ABS" err=$? From c90cb977a1b5d570a0c7788d6ac0a3025b417fc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Jul 2022 13:04:11 +0200 Subject: [PATCH 08/13] kernel-install: mark the plugins as executable in git We mark them as executable during installation, but it's also nice to mark them as such in sources, so they can be executed directly. --- src/kernel-install/50-depmod.install | 0 src/kernel-install/90-loaderentry.install | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 src/kernel-install/50-depmod.install mode change 100644 => 100755 src/kernel-install/90-loaderentry.install diff --git a/src/kernel-install/50-depmod.install b/src/kernel-install/50-depmod.install old mode 100644 new mode 100755 diff --git a/src/kernel-install/90-loaderentry.install b/src/kernel-install/90-loaderentry.install old mode 100644 new mode 100755 From f875e6bc399614401b245751dec04918407e754d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Jul 2022 13:08:31 +0200 Subject: [PATCH 09/13] test-kernel-install: add a simple test that kernel-install copies the files I opted to tweaking kernel-install to allow overriding config (with $KERNEL_INSTALL_CONF_ROOT, $KERNEL_INSTALL_PLUGINS). An alternative would be to build a test environment in test/. We can still do that, but I think it's nice to have a simple test that is very quick and easy to debug. Invocation as installkernel is for #23681. --- meson.build | 9 ++- src/kernel-install/meson.build | 4 +- src/kernel-install/test-kernel-install.sh | 83 +++++++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100755 src/kernel-install/test-kernel-install.sh diff --git a/meson.build b/meson.build index 2e8b69c96a..ff0488145b 100644 --- a/meson.build +++ b/meson.build @@ -3751,7 +3751,7 @@ executable( install : true, install_dir : rootlibexecdir) -public_programs += custom_target( +exe = custom_target( 'kernel-install', input : kernel_install_in, output : 'kernel-install', @@ -3759,6 +3759,13 @@ public_programs += custom_target( install : want_kernel_install, install_mode : 'rwxr-xr-x', install_dir : bindir) +public_programs += exe + +if want_tests != 'false' + test('test-kernel-install', + test_kernel_install_sh, + args : [exe.full_path(), loaderentry_install]) +endif ############################################################ diff --git a/src/kernel-install/meson.build b/src/kernel-install/meson.build index 06c1c3aafb..90a0e3ae49 100644 --- a/src/kernel-install/meson.build +++ b/src/kernel-install/meson.build @@ -1,10 +1,11 @@ # SPDX-License-Identifier: LGPL-2.1-or-later kernel_install_in = files('kernel-install.in') +loaderentry_install = files('90-loaderentry.install') if want_kernel_install install_data('50-depmod.install', - '90-loaderentry.install', + loaderentry_install, install_mode : 'rwxr-xr-x', install_dir : kernelinstalldir) @@ -16,4 +17,5 @@ if want_kernel_install mkdir_p.format(sysconfdir / 'kernel/install.d')) endif + test_kernel_install_sh = find_program('test-kernel-install.sh') endif diff --git a/src/kernel-install/test-kernel-install.sh b/src/kernel-install/test-kernel-install.sh new file mode 100755 index 0000000000..b350b4d1ef --- /dev/null +++ b/src/kernel-install/test-kernel-install.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -eu +set -o pipefail + +kernel_install="${1:?}" +plugin="${2:?}" + +D="$(mktemp --tmpdir --directory "test-kernel-install.XXXXXXXXXX")" + +# shellcheck disable=SC2064 +trap "rm -rf '$D'" EXIT INT QUIT PIPE +mkdir -p "$D/boot" +mkdir -p "$D/efi" +mkdir -p "$D/sources" + +echo 'buzy image' >"$D/sources/linux" +echo 'the initrd' >"$D/sources/initrd" +echo 'the-token' >"$D/sources/entry-token" +echo 'opt1 opt2' >"$D/sources/cmdline" + +cat >"$D/sources/install.conf" <"$D/sources/tries" + +"$kernel_install" -v add 1.1.1 "$D/sources/linux" "$D/sources/initrd" +entry="$BOOT_ROOT/loader/entries/the-token-1.1.1+56.conf" +test -f "$entry" +grep -qE '^title ' "$entry" +grep -qE '^version +1.1.1' "$entry" +grep -qE '^options +opt1 opt2' "$entry" +grep -qE '^linux .*/the-token/1.1.1/linux' "$entry" +grep -qE '^initrd .*/the-token/1.1.1/initrd' "$entry" + +grep -qE 'image' "$BOOT_ROOT/the-token/1.1.1/linux" +grep -qE 'initrd' "$BOOT_ROOT/the-token/1.1.1/initrd" From 00e6278cac0cfca4f744caf76d6d67bd18f6c7b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 2 Jul 2022 14:41:41 +0200 Subject: [PATCH 10/13] kernel-install: raise fuss if plugins are called without the expected parameters --- src/kernel-install/50-depmod.install | 4 ++-- src/kernel-install/90-loaderentry.install | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/kernel-install/50-depmod.install b/src/kernel-install/50-depmod.install index 67319afe61..fdb7545dc9 100755 --- a/src/kernel-install/50-depmod.install +++ b/src/kernel-install/50-depmod.install @@ -18,8 +18,8 @@ # You should have received a copy of the GNU Lesser General Public License # along with systemd; If not, see . -COMMAND="$1" -KERNEL_VERSION="$2" +COMMAND="${1:?}" +KERNEL_VERSION="${2:?}" case "$COMMAND" in add) diff --git a/src/kernel-install/90-loaderentry.install b/src/kernel-install/90-loaderentry.install index d6b90a7b1d..a640ea42ca 100755 --- a/src/kernel-install/90-loaderentry.install +++ b/src/kernel-install/90-loaderentry.install @@ -18,9 +18,9 @@ # You should have received a copy of the GNU Lesser General Public License # along with systemd; If not, see . -COMMAND="$1" -KERNEL_VERSION="$2" -ENTRY_DIR_ABS="$3" +COMMAND="${1:?}" +KERNEL_VERSION="${2:?}" +ENTRY_DIR_ABS="${3:?}" KERNEL_IMAGE="$4" INITRD_OPTIONS_SHIFT=4 From 852f98d6aae691574a933691b34133f35958b0d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 2 Jul 2022 14:44:01 +0200 Subject: [PATCH 11/13] kernel-install: return 0 for unknown verbs in plugins In practice this makes little difference, because kernel-install will only call the plugins for 'add' or 'remove', and if we were to add a new verb to kernel-install, we'd just change the plugins at the same time. But our plugins serve as documentation for external plugins too, and there it's better to silently ignore unknown verbs so that we can add new verbs in the future. (50-depomod.install was already like that.) --- src/kernel-install/50-depmod.install | 1 + src/kernel-install/90-loaderentry.install | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/kernel-install/50-depmod.install b/src/kernel-install/50-depmod.install index fdb7545dc9..b43333762b 100755 --- a/src/kernel-install/50-depmod.install +++ b/src/kernel-install/50-depmod.install @@ -44,4 +44,5 @@ case "$COMMAND" in ;; *) exit 0 + ;; esac diff --git a/src/kernel-install/90-loaderentry.install b/src/kernel-install/90-loaderentry.install index a640ea42ca..96e24194c4 100755 --- a/src/kernel-install/90-loaderentry.install +++ b/src/kernel-install/90-loaderentry.install @@ -48,7 +48,7 @@ case "$COMMAND" in add) ;; *) - exit 1 + exit 0 ;; esac From b9b55a884bcc111c5f9bab3df958b6bd71ee25e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 2 Jul 2022 14:45:07 +0200 Subject: [PATCH 12/13] kernel-install: use set -e This means that we'll fail hard if something goes wrong, e.g. reading of a config file. I think this is appropriate. If errors should be ignored, the caller should do that on their end. --- src/kernel-install/50-depmod.install | 2 ++ src/kernel-install/90-loaderentry.install | 2 ++ src/kernel-install/kernel-install.in | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/kernel-install/50-depmod.install b/src/kernel-install/50-depmod.install index b43333762b..d4b991cfd6 100755 --- a/src/kernel-install/50-depmod.install +++ b/src/kernel-install/50-depmod.install @@ -18,6 +18,8 @@ # You should have received a copy of the GNU Lesser General Public License # along with systemd; If not, see . +set -e + COMMAND="${1:?}" KERNEL_VERSION="${2:?}" diff --git a/src/kernel-install/90-loaderentry.install b/src/kernel-install/90-loaderentry.install index 96e24194c4..b34bbd83cd 100755 --- a/src/kernel-install/90-loaderentry.install +++ b/src/kernel-install/90-loaderentry.install @@ -18,6 +18,8 @@ # You should have received a copy of the GNU Lesser General Public License # along with systemd; If not, see . +set -e + COMMAND="${1:?}" KERNEL_VERSION="${2:?}" ENTRY_DIR_ABS="${3:?}" diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 30b28f4658..a597e41133 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -20,6 +20,8 @@ skip_remaining=77 +set -e + usage() { echo "Usage:" From dd2ebd1b1d5a6e5316dce252cfe3f6e7777f341c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 2 Jul 2022 14:55:05 +0200 Subject: [PATCH 13/13] kernel-install: suppress bogus shellcheck hint I don't quite understand this, but '{ ! true; }' is not the same as '( ! true )'. In interactive mode, it seems to work as expected. But in a script, it doesn't. --- src/kernel-install/test-kernel-install.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/kernel-install/test-kernel-install.sh b/src/kernel-install/test-kernel-install.sh index b350b4d1ef..eda3eaaebf 100755 --- a/src/kernel-install/test-kernel-install.sh +++ b/src/kernel-install/test-kernel-install.sh @@ -1,5 +1,6 @@ #!/usr/bin/env bash # SPDX-License-Identifier: LGPL-2.1-or-later +# shellcheck disable=SC2235 set -eu set -o pipefail