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@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