[arch-projects] [MKINITCPIO][PATCH 1/4] Allow mkinitcpio to generate all preset
Dave Reisner
d at falconindy.com
Mon Feb 4 13:14:03 EST 2013
On Mon, Feb 04, 2013 at 06:43:59PM +0100, Sébastien Luttringer wrote:
> Add options -a and --all which ask to mkinitcpio to generate all presets
> registered in /etc/mkinitcpio.d.
>
> This is usefull, by example, when you update mkinitcpio.conf and you want to
> rebuild all initramfs for your kernels.
>
> Signed-off-by: Sébastien Luttringer <seblu at seblu.net>
> ---
Yay! I love code reviews!
This patch is a mess, but I like the idea. Seems to me that it could be
far cleaner if you did something in main() like:
if (( ${#_optpreset[*]} )); then
map process_preset "${_optpreset[@]}"
exit
fi
Just fix process_preset so that it doesn't ever exit, only return $ret.
> bash-completion | 2 +-
> man/mkinitcpio.8.txt | 4 ++
> mkinitcpio | 106 +++++++++++++++++++++++++++++----------------------
> 3 files changed, 65 insertions(+), 47 deletions(-)
>
> diff --git a/bash-completion b/bash-completion
> index 0108863..b9dc888 100644
> --- a/bash-completion
> +++ b/bash-completion
> @@ -62,7 +62,7 @@ _files_from_dirs() {
>
> _mkinitcpio() {
> local action cur prev opts
> - opts=(-A --addhooks -c --config -g --generate -H --hookhelp -h --help -k --kernel
> + opts=(-A --addhooks -a --all -c --config -g --generate -H --hookhelp -h --help -k --kernel
Would prefer using -P, --allpresets over -a. Think "lots of -p". --all
is especially vague, since presets are a feature and not the centerpiece
of mkinitcpio.
> -L --listhooks -M --automods -n --nocolor -p --preset -r --moduleroot
> -S --skiphooks -s --save -t --builddir -V --version -v --verbose -z --compress)
>
> diff --git a/man/mkinitcpio.8.txt b/man/mkinitcpio.8.txt
> index 2b0f524..cd7dd42 100644
> --- a/man/mkinitcpio.8.txt
> +++ b/man/mkinitcpio.8.txt
> @@ -29,6 +29,10 @@ Options
> after all other hooks from the config file. Multiple hooks should be
> comma-separated. This option can be specified multiple times.
>
> +*-a, \--all*::
> + Build all initramfs image(s) according to all specified presets in
Just "images". We're definitely interested in the plural here since we
expect multiple presets, and therefore multiple images.
> + /etc/mkinitcpio.d. See '-p' option for more details about presets.
See _the_ '-p' option
> +
> *-c, \--config* 'config'::
> Use 'config' file to generate the ramdisk. Default: /etc/mkinitcpio.conf
>
> diff --git a/mkinitcpio b/mkinitcpio
> index 9802fd5..e932602 100755
> --- a/mkinitcpio
> +++ b/mkinitcpio
> @@ -5,7 +5,7 @@
>
> declare -r version=%VERSION%
>
> -shopt -s extglob
> +shopt -s extglob nullglob
NAK. nullglob should never be enabled globally. Find out where you need
it and find a way to scope it or work around it. I suspect I've located
this, but I'll defer to you, the author.
>
> ### globals within mkinitcpio, but not intended to be used by hooks
>
> @@ -39,6 +39,7 @@ usage: ${0##*/} [options]
>
> Options:
> -A, --addhooks <hooks> Add specified hooks, comma separated, to image
> + -a, --all Build all presets in $_d_presets
> -c, --config <config> Use alternate config file. (default: /etc/mkinitcpio.conf)
> -g, --generate <path> Generate cpio image and write to specified path
> -H, --hookhelp <hookname> Display help for given hook and exit
> @@ -237,57 +238,65 @@ build_image() {
> }
>
> process_preset() {
> - local preset=$1 preset_image= preset_options=
> + local preset= preset_image= preset_options= ret=0
> local -a preset_mkopts preset_cmd
>
> - # allow path to preset file, else resolve it in $_d_presets
> - if [[ $preset != */* ]]; then
> - printf -v preset '%s/%s.preset' "$_d_presets" "$preset"
> - fi
> + for preset; do
> + # here we need to use subshell to be sure sourcing files doesn't taint
> + # our shell context accross call
> + (
> + # allow path to preset file, else resolve it in $_d_presets
> + if [[ $preset != */* ]]; then
> + printf -v preset '%s/%s.preset' "$_d_presets" "$preset"
> + fi
>
> - . "$preset" || die "Preset not found: \`%s'" "$preset"
> + . "$preset" || die "Preset not found: \`%s'" "$preset"
>
> - # Use -m and -v options specified earlier
> - (( _optquiet )) || preset_mkopts+=(-v)
> - (( _optcolor )) || preset_mkopts+=(-n)
> + # Use -m and -v options specified earlier
> + (( _optquiet )) || preset_mkopts+=(-v)
> + (( _optcolor )) || preset_mkopts+=(-n)
>
> - ret=0
> - for p in "${PRESETS[@]}"; do
> - msg "Building image from preset: '$p'"
> - preset_cmd=("${preset_mkopts[@]}")
> + for p in "${PRESETS[@]}"; do
> + msg "Building image from preset '$p' in '$preset'"
> + preset_cmd=("${preset_mkopts[@]}")
>
> - preset_kver=${p}_kver
> - if [[ ${!preset_kver:-$ALL_kver} ]]; then
> - preset_cmd+=(-k "${!preset_kver:-$ALL_kver}")
> - else
> - warning "No kernel version specified. Skipping image \`%s'" "$p"
> - continue
> - fi
> + preset_kver=${p}_kver
> + if [[ ${!preset_kver:-$ALL_kver} ]]; then
> + preset_cmd+=(-k "${!preset_kver:-$ALL_kver}")
> + else
> + warning "No kernel version specified. Skipping image \`%s'" "$p"
> + continue
> + fi
>
> - preset_config=${p}_config
> - if [[ ${!preset_config:-$ALL_config} ]]; then
> - preset_cmd+=(-c "${!preset_config:-$ALL_config}")
> - else
> - warning "No configuration file specified. Skipping image \`%s'" "$p"
> - continue
> - fi
> + preset_config=${p}_config
> + if [[ ${!preset_config:-$ALL_config} ]]; then
> + preset_cmd+=(-c "${!preset_config:-$ALL_config}")
> + else
> + warning "No configuration file specified. Skipping image \`%s'" "$p"
> + continue
> + fi
>
> - preset_image=${p}_image
> - if [[ ${!preset_image} ]]; then
> - preset_cmd+=(-g "${!preset_image}")
> - else
> - warning "No image file specified. Skipping image \`%s'" "$p"
> - continue
> - fi
> + preset_image=${p}_image
> + if [[ ${!preset_image} ]]; then
> + preset_cmd+=(-g "${!preset_image}")
> + else
> + warning "No image file specified. Skipping image \`%s'" "$p"
> + continue
> + fi
>
> - preset_options=${p}_options
> - if [[ ${!preset_options} ]]; then
> - preset_cmd+=(${!preset_options}) # intentional word splitting
> - fi
> + preset_options=${p}_options
> + if [[ ${!preset_options} ]]; then
> + preset_cmd+=(${!preset_options}) # intentional word splitting
> + fi
> +
> + msg2 "${preset_cmd[*]}"
> + "$0" "${preset_cmd[@]}"
> + (( $? )) && ret=1
> + done
>
> - msg2 "${preset_cmd[*]}"
> - "$0" "${preset_cmd[@]}"
> - (( $? )) && ret=1
> + exit $ret
> + )
> + (( $? )) && ret=1
> done
>
> exit $ret
> @@ -339,8 +348,8 @@ install_modules() {
> trap 'cleanup 130' INT
> trap 'cleanup 143' TERM
>
> -_opt_short='A:c:g:H:hk:nLMp:r:S:st:Vvz:'
> -_opt_long=('add:' 'addhooks:' 'config:' 'generate:' 'hookhelp:' 'help'
> +_opt_short='aA:c:g:H:hk:nLMp:r:S:st:Vvz:'
> +_opt_long=('add:' 'addhooks:' 'all' 'config:' 'generate:' 'hookhelp:' 'help'
> 'kernel:' 'listhooks' 'automods' 'moduleroot:' 'nocolor'
> 'preset:' 'skiphooks:' 'save' 'builddir:' 'version' 'verbose' 'compress:')
>
> @@ -356,6 +365,11 @@ while :; do
> IFS=, read -r -a add <<< "$1"
> _optaddhooks+=("${add[@]}")
> unset add ;;
> + -a|--all)
> + _optpreset=("$_d_presets"/*.preset)
This variable is treated as an array now, but the global def is still a
string. Please fix. It leads to your hacky workaround which I'll point
out below.
> + if ! (( ${#_optpreset} )); then
style nit, please fix:
if (( expr == 0 )); then
But rather, I suspect this is where you wanted nullglob. Just check for
existance:
if [[ ! -e ${_optpreset[0]} ]]; then die ....; fi
> + die "No preset files available in \`%s'." "$_d_presets"
> + fi ;;
> -c|--config)
> shift
> _f_config=$1 ;;
> @@ -378,7 +392,7 @@ while :; do
> cleanup 0 ;;
> -p|--preset)
> shift
> - _optpreset=$1 ;;
> + _optpreset=("$1") ;;
> -n|--nocolor)
> _optcolor=0 ;;
> -v|--verbose)
> @@ -424,7 +438,7 @@ fi
> [[ -e /dev/fd ]] || die "/dev must be mounted!"
>
> # use preset $_optpreset (exits after processing)
> -[[ $_optpreset ]] && process_preset "$_optpreset"
> +(( ${#_optpreset} )) && process_preset "${_optpreset[@]}"
You surely wanted to use (( ${#_optpreset[*]} )) here, but you can't,
since you didn't alter the definition of _optpreset earlier.
$ v=
$ echo ${#v[*]}
1
$ v=()
$ echo ${#v[*]}
0
>
> KERNELVERSION=$(resolve_kernver "$_optkver") || cleanup 1
> _d_kmoduledir=$_optmoduleroot/lib/modules/$KERNELVERSION
> --
> Sébastien "Seblu" Luttringer
>
More information about the arch-projects
mailing list