Permalink
Browse files

commit to some level of style in variable naming

This is an ugly patch, and probably does more than I'd like it to. The
idea is that mkinitcpio adopts some sort of consistent style which I'm
actually happy with. I define 3 kinds of variables:

1) local variables: all lower case, and scoped within functions. Use
freely, as they're well contained.

2) global variables: these are known to mkinitcpio internally, but are
global in scope. They mainly carry runtime configuration and collected
data during the image generation process. These are always lower case,
but carry a leading underscore to denote that they're global.

3) "API" variables: also global in scope, but exist "outside" of
mkinitcpio -- either drawn in from the configuration file, or "exported"
to the install hooks. These are always all upper case. When introducing
new variables, extreme care must be taken to pick names that will not
conflict with the environment inherited by mkinitcpio.

A HACKING file is introduced with a similar description of the above,
and more.

Signed-off-by: Dave Reisner <dreisner@archlinux.org>
  • Loading branch information...
1 parent 4bbca4a commit 367ac227f42ca9c8a7c050da0bcc04248fae29b1 @falconindy committed Sep 28, 2012
Showing with 285 additions and 217 deletions.
  1. +67 −0 HACKING
  2. +6 −6 Makefile
  3. +46 −46 functions
  4. +9 −8 install/autodetect
  5. +53 −56 lsinitcpio
  6. +104 −101 mkinitcpio
View
67 HACKING
@@ -0,0 +1,67 @@
+Care and feeding of your initramfs generator
+--------------------------------------------
+
+This guide attempts to outline the style used in the mkinitcpio codebase.
+
+
+Bash v. POSIX
+-------------
+Never use POSIX syntax if Bash offers a construct of its own, even if the
+two are effectively identical. This means always using double braces over
+the inferior '[' and 'test'.
+
+
+Variable usage and naming convetions
+------------------------------------
+There are three classifications of variables in mkinitcpio.
+
+1) local variables: all lower case, and scoped within functions. Use
+freely, as they're well contained. Unless you're introducing a new
+option, this is what you want to use.
+
+ local foo=$1
+
+2) global variables: these are known to mkinitcpio internally, but are
+global in scope. They carry runtime configuration and data collected during the
+image generation process. These are always lower case, but carry a leading
+underscore to denote that they're global. It's helpful to prefix these
+variables instead with a '_f_' or '_d_' if they refer to a file or directory,
+respectively.
+
+ _optcolor=1
+ _d_hookdir=/etc/foo.d
+ _f_config=/etc/foo.conf
+
+3) "API" variables: also global in scope, but exist "outside" of
+mkinitcpio -- either drawn in from the configuration file, or "exported"
+to the install hooks. These are always all upper case. When introducing
+new variables, extreme care must be taken to pick names that will not
+conflict with the environment inherited by mkinitcpio.
+
+Function naming
+---------------
+Use all lower case with underscores where appropriate, for easy readability.
+Adhere to POSIX variable naming requirements for the contents of the name,
+that is: only alphanumerics, underscores, and the identifier must not start
+with a number.
+
+
+Quoting
+-------
+Overquoting is preferred to underquoting, but freely avoid quoting in the
+cases when expansion is guaranteed not to happen, such as in single argument
+test expressions or the subject of a case statement.
+
+
+Functions and block statements
+------------------------------
+Always use "top-right, "lower left" for blocks of code and functions.
+
+ do_glob() {
+ local g fn=$1; shift
+
+ for g in "$@"; do
+ "$fn" "$g"
+ done
+ }
+
View
@@ -22,15 +22,15 @@ install: all
mkdir -p ${DESTDIR}
$(foreach dir,${DIRS},install -dm755 ${DESTDIR}${dir};)
- sed -e 's|^CONFIG=.*|CONFIG=/etc/mkinitcpio.conf|' \
- -e 's|^FUNCTIONS=.*|FUNCTIONS=/usr/lib/initcpio/functions|' \
- -e 's|^HOOKDIR=.*|HOOKDIR=({/usr,}/lib/initcpio/hooks)|' \
- -e 's|^INSTDIR=.*|INSTDIR=({/usr,}/lib/initcpio/install)|' \
- -e 's|^PRESETDIR=.*|PRESETDIR=/etc/mkinitcpio.d|' \
+ sed -e 's|^_f_config=.*|_f_config=/etc/mkinitcpio.conf|' \
+ -e 's|^_f_functions=.*|_f_functions=/usr/lib/initcpio/functions|' \
+ -e 's|^_d_hooks=.*|_d_hooks=({/usr,}/lib/initcpio/hooks)|' \
+ -e 's|^_d_install=.*|_d_install=({/usr,}/lib/initcpio/install)|' \
+ -e 's|^_d_presets=.*|_d_presets=/etc/mkinitcpio.d|' \
-e 's|%VERSION%|${VERSION}|g' \
< mkinitcpio > ${DESTDIR}/usr/bin/mkinitcpio
- sed -e 's|\(^declare FUNCTIONS\)=.*|\1=/usr/lib/initcpio/functions|' \
+ sed -e 's|\(^_f_functions\)=.*|\1=/usr/lib/initcpio/functions|' \
-e 's|%VERSION%|${VERSION}|g' \
< lsinitcpio > ${DESTDIR}/usr/bin/lsinitcpio
View
@@ -139,27 +139,31 @@ parseopts() {
plain() {
local mesg=$1; shift
- printf "$BOLD $mesg$NC\n" "$@" >&1
+ printf " $_color_bold$mesg$_color_none\n" "$@" >&1
+}
+
+quiet() {
+ (( _optquiet )) || plain "$@"
}
msg() {
local mesg=$1; shift
- printf "$GREEN==>$NC$BOLD $mesg$NC\n" "$@" >&1
+ printf "$_color_green==>$_color_none $_color_bold$mesg$_color_none\n" "$@" >&1
}
msg2() {
local mesg=$1; shift
- printf "$BLUE ->$NC$BOLD $mesg$NC\n" "$@" >&1
+ printf " $_color_blue->$_color_none $_color_bold$mesg$_color_none\n" "$@" >&1
}
warning() {
local mesg=$1; shift
- printf "$YELLOW==> WARNING:$NC$BOLD $mesg$NC\n" "$@" >&2
+ printf "$_color_yellow==> WARNING:$_color_none $_color_bold$mesg$_color_none\n" "$@" >&2
}
error() {
local mesg=$1; shift
- printf "$RED==> ERROR:$NC$BOLD $mesg$NC\n" "$@" >&2
+ printf "$_color_red==> ERROR:$_color_none $_color_bold$mesg$_color_none\n" "$@" >&2
return 1
}
@@ -242,7 +246,7 @@ all_modules() {
mod=${mod##*/}
mod="${mod%.ko*}"
printf '%s\n' "${mod//-/_}"
- done < <(find "$MODULEDIR" -name '*.ko*' -print0 2>/dev/null | grep -EZz "$@")
+ done < <(find "$_d_kmoduledir" -name '*.ko*' -print0 2>/dev/null | grep -EZz "$@")
(( count ))
}
@@ -269,8 +273,8 @@ add_checked_modules() {
local mod mods
- if [[ -s $MODULE_FILE ]]; then
- mapfile -t mods < <(all_modules "$@" | grep -xFf "$MODULE_FILE")
+ if [[ -s $_f_autodetect_cache ]]; then
+ mapfile -t mods < <(all_modules "$@" | grep -xFf "$_f_autodetect_cache")
else
mapfile -t mods < <(all_modules "$@")
fi
@@ -289,8 +293,8 @@ checked_modules() {
# DEPRECATED: Use add_checked_modules instead
#
- if [[ -s $MODULE_FILE ]]; then
- grep -xFf "$MODULE_FILE" <(all_modules "$@")
+ if [[ -s $_f_autodetect_cache ]]; then
+ grep -xFf "$_f_autodetect_cache" <(all_modules "$@")
return 1
else
all_modules "$@"
@@ -313,7 +317,7 @@ add_module() {
module=${1%.ko*}
# skip expensive stuff if this module has already been added
- (( ADDED_MODULES["${module//-/_}"] )) && return
+ (( _addedmodules["${module//-/_}"] )) && return
while IFS=':= ' read -r -d '' field value; do
case "$field" in
@@ -341,9 +345,9 @@ add_module() {
fi
# aggregate modules and add them all at once to save some forks
- (( QUIET )) || plain "adding module: %s" "$1"
- MODPATHS+=("$path")
- ADDED_MODULES["${module//-/_}"]=1
+ quiet "adding module: %s" "$1"
+ _modpaths+=("$path")
+ _addedmodules["${module//-/_}"]=1
# handle module quirks
case $module in
@@ -397,7 +401,7 @@ add_dir() {
return 0
fi
- (( QUIET )) || plain "adding dir: %s" "$path"
+ quiet "adding dir: %s" "$path"
command install -dm$mode "$BUILDROOT$path"
}
@@ -407,28 +411,26 @@ add_symlink() {
# $1: pathname of symlink on image
# $2: absolute path to target of symlink (optional, can be read from $1)
- local name_=$1 target_=$2
+ local name=$1 target=$2
(( $# == 1 || $# == 2 )) || return 1
- if [[ -z $target_ ]]; then
- target_=$(readlink -f "$name_")
- if [[ -z $target_ ]]; then
- error 'invalid symlink: %s' "$name_"
+ if [[ -z $target ]]; then
+ target=$(readlink -f "$name")
+ if [[ -z $target ]]; then
+ error 'invalid symlink: %s' "$name"
return 1
fi
fi
- add_dir "${name_%/*}"
+ add_dir "${name%/*}"
- if (( ! QUIET )); then
- if [[ -L $BUILDROOT$1 ]]; then
- plain "overwriting symlink %s -> %s" "$name_" "$target_"
- else
- plain "adding symlink: %s -> %s" "$name_" "$target_"
- fi
+ if [[ -L $BUILDROOT$1 ]]; then
+ quiet "overwriting symlink %s -> %s" "$name" "$target"
+ else
+ quiet "adding symlink: %s -> %s" "$name" "$target"
fi
- ln -sfn "$target_" "$BUILDROOT$name_"
+ ln -sfn "$target" "$BUILDROOT$name"
}
add_file() {
@@ -454,12 +456,10 @@ add_file() {
return 1
fi
- if (( ! QUIET )); then
- if [[ -e $BUILDROOT$dest ]]; then
- plain "overwriting file: %s" "$dest"
- else
- plain "adding file: %s" "$dest"
- fi
+ if [[ -e $BUILDROOT$dest ]]; then
+ quiet "overwriting file: %s" "$dest"
+ else
+ quiet "adding file: %s" "$dest"
fi
command install -Dm$mode "$src" "$BUILDROOT$dest"
}
@@ -473,7 +473,7 @@ add_runscript() {
local funcs fn script hookname=${SCRIPT:-${BASH_SOURCE[1]##*/}}
- if ! script=$(find_in_dirs "$hookname" "${HOOKDIR[@]}"); then
+ if ! script=$(find_in_dirs "$hookname" "${_d_hooks[@]}"); then
error "runtime script for \`%s' not found" "$hookname"
return
fi
@@ -485,16 +485,16 @@ add_runscript() {
for fn in "${funcs[@]}"; do
case $fn in
run_earlyhook)
- RUNHOOKS['early']+=" $hookname"
+ _runhooks['early']+=" $hookname"
;;
run_hook)
- RUNHOOKS['hooks']+=" $hookname"
+ _runhooks['hooks']+=" $hookname"
;;
run_latehook)
- RUNHOOKS['late']+=" $hookname"
+ _runhooks['late']+=" $hookname"
;;
run_cleanuphook)
- RUNHOOKS['cleanup']="$hookname ${RUNHOOKS['cleanup']}"
+ _runhooks['cleanup']="$hookname ${_runhooks['cleanup']}"
;;
esac
done
@@ -595,22 +595,22 @@ write_image_config() {
# write the config as runtime config and as a pristine build config
# (for audting purposes) to the image.
- tee "$BUILDROOT/buildconfig" < "$CONFIG" | {
+ tee "$BUILDROOT/buildconfig" < "$_f_config" | {
. /dev/stdin
# sanitize of any extra whitespace
read -ra modules <<<"${MODULES//-/_}"
for mod in "${modules[@]%\?}"; do
# only add real modules (2 == builtin)
- (( ADDED_MODULES["$mod"] == 1 )) && add+=("$mod")
+ (( _addedmodules["$mod"] == 1 )) && add+=("$mod")
done
(( ${#add[*]} )) && printf 'MODULES="%s"\n' "${add[*]}"
printf '%s="%s"\n' \
- 'EARLYHOOKS' "${RUNHOOKS['early']# }" \
- 'HOOKS' "${RUNHOOKS['hooks']# }" \
- 'LATEHOOKS' "${RUNHOOKS['late']# }" \
- 'CLEANUPHOOKS' "${RUNHOOKS['cleanup']% }"
+ 'EARLYHOOKS' "${_runhooks['early']# }" \
+ 'HOOKS' "${_runhooks['hooks']# }" \
+ 'LATEHOOKS' "${_runhooks['late']# }" \
+ 'CLEANUPHOOKS' "${_runhooks['cleanup']% }"
} >"$BUILDROOT/config"
}
@@ -653,7 +653,7 @@ run_build_hook() {
local MODULES= BINARIES= FILES= SCRIPT=
# find script in install dirs
- if ! script=$(find_in_dirs "$hook" "${INSTDIR[@]}"); then
+ if ! script=$(find_in_dirs "$hook" "${_d_install[@]}"); then
error "Hook '$hook' cannot be found"
return 1
fi
View
@@ -3,17 +3,17 @@
build() {
local -a md_devs
- MODULE_FILE=$workdir/autodetect_modules
+ _f_autodetect_cache=$_d_workdir/autodetect_modules
add_if_avail() {
local resolved
# treat this as an alias, since ext3 might be aliased to ext4.
IFS=$'\n' read -rd '' -a resolved < \
- <(modprobe -d "$MODULEROOT" -S "$KERNELVERSION" -R "$1" 2>/dev/null)
+ <(modprobe -d "$_optmoduleroot" -S "$KERNELVERSION" -R "$1" 2>/dev/null)
if (( ${#resolved[*]} )); then
- printf '%s\n' "${resolved[@]}" >>"$MODULE_FILE"
+ printf '%s\n' "${resolved[@]}" >>"$_f_autodetect_cache"
fi
}
@@ -22,7 +22,7 @@ build() {
return 1
fi
- auto_modules >"$MODULE_FILE"
+ auto_modules >"$_f_autodetect_cache"
# detect filesystem for root
if rootfstype=$(findmnt -uno fstype '/'); then
@@ -40,12 +40,13 @@ build() {
# scan for md raid devices
md_devs=(/sys/class/block/md*/md/level)
if [[ -e $md_devs ]]; then
- (( !QUIET )) && plain "found %d mdadm arrays to scan" "${#md_devs[*]}"
- awk '{ gsub(/raid[456]/, "raid456"); print; }' "${md_devs[@]}" | sort -u >>"$MODULE_FILE"
+ quiet "found %d mdadm arrays to scan" "${#md_devs[*]}"
+ awk '{ gsub(/raid[456]/, "raid456"); print; }' "${md_devs[@]}" |
+ sort -u >>"$_f_autodetect_cache"
fi
- if (( !QUIET )) && [[ -s $MODULE_FILE ]]; then
- plain "caching %d modules" $(wc -l < "$MODULE_FILE")
+ if [[ -s $_f_autodetect_cache ]]; then
+ quiet "caching %d modules" $(wc -l < "$_f_autodetect_cache")
fi
}
Oops, something went wrong.

0 comments on commit 367ac22

Please sign in to comment.