[arch-projects] [mkinitcpio][PATCH] functions: add_udev_rules and add_systemd_unit

[Dave Reisner]

On Sat, Apr 13, 2013 at 03:43:35AM +0200, Tom Gundersen wrote:
> add_udev_rules() adds a rules file and pulls in the necessary binaries udev
> rules by automatically pulling in the necessary binaries.
>
> add_systemd_unit() pulls in the necessary binaries, and also Requires= and
> OnFailure= units.
>
> Moreover, it pulls in any symlinks to the unit in any .wants or .requires
> directories.
> ---

It'd be great if you could also prepare some doc for these in mkinitcpio(8).

>  functions | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
>
> diff --git a/functions b/functions
> index 432d700..12df3cd 100644
> --- a/functions
> +++ b/functions
> @@ -569,6 +569,104 @@ add_binary() {
>      return 0
>  }
>
> +add_udev_rules() {
> +    # Add an udev rules file to the initcpio image. Dependencies on binaries
> +    # will be discovered and added.
> +    #   $1: path to rules file (or name of rules file)
> +
> +    local udevdir= rules= rule= entry= key= value= binary=
> +    udevdir=$(pkg-config --variable=udevdir udev)

I'd like to avoid pkg-config as a dependency here. We can use a
hardcoded (sane) PATH and do something like...

  rules=$(PATH=/usr/lib/udev/rules.d:/lib/udev/rules.d type -P "$1")
  if [[ -z $rules ]]; then
    # complain about not found rules
    return 1
  fi

> +
> +    if [[ ${1:0:1} != '/' ]]; then
> +        rules="${udevdir}/rules.d/${1}"
> +    else
> +        rules=$1
> +    fi
> +
> +    add_file "${rules}"
> +
> +    while read rule; do
> +        if [[ ${rule:0:1} == '#' ]]; then
> +            continue
> +        fi
> +
> +        IFS=', '

There's no support for multibyte field separators. What this actually
does is split lines on both ',' and ' ', which I suspect  breaks a lot
of the logic below here. I suspect you want to just split on commas in
the outer while loop:

  while IFS=, read -ra rule; do
    for pair in "${rule[@]}"; do
      IFS='=' read -ra key values <<<"$rule"


> +        for entry in ${rule}; do
> +
> +            IFS='=' read key value <<< "${entry}"
> +
> +            if [[ ( ${key:0:3} == "RUN" &&
> +                    ${key:4:7} != "builtin" ) ||
> +                  ${key:0:7} == "PROGRAM" ||
> +                  ${key:0:15} == "IMPORT{program}" ||
> +                  ${key:0:15} == "ENV{REMOVE_CMD}" ]]; then

Perhaps a case statement? Taking my advice above gives you a nice array
of key/value pairs for each entry in the rule:

  case $key in
    RUN|RUN+|IMPORT{@(builtin|program)}|ENV{REMOVE_CMD})
      binary=${value[0]#\"}
      if [[ ${binary:0:1} = / ]]; then
        add_binary "$binary"
      else
        add_binary "$udevdir/$binary"
      fi
    ;;
  esac

> +
> +                binary=${value//\"/}
> +                if [[ ${binary:0:1} != '/' ]]; then
> +                    binary="${udevdir}/${binary}"
> +                fi
> +
> +                read -d ' ' binary <<< "${binary} "
> +                add_binary "${binary}"
> +            fi
> +        done
> +    done < "$rules"
> +}
> +
> +add_systemd_unit() {
> +    # Add a systemd unit file to the initcpio image. Dependencies on binaries
> +    # and other unit files will be discovered and added.
> +    #   $1: path to rules file (or name of rules file)
> +
> +    local unitdir= unit= rule= entry= key= value= binary= dep=
> +
> +    unitdir=$(pkg-config --variable=systemdsystemunitdir systemd)
> +
> +    if [[ ${1:0:1} != '/' ]]; then
> +        unit="${unitdir}/${1}"
> +    else
> +        unit=$1
> +    fi
> +
> +    add_file "${unit}"
> +
> +    while read entry; do

You should split the line here instead of splitting again below. You
need to make sure to pass the -r flag as well:

  while IFS='=' read -r key values; do
    read -ra values <<<"$values"

    # now values is split on whitespace into an array

    ...
  done

> +        if [[ ${entry:0:1} == '#' ]]; then
> +            continue
> +        fi
> +
> +        IFS='=' read key value <<< "${entry}"
> +
> +	if [[ ${key:0:4} == 'Exec' ]]; then
> +        binary=${value//\"/}
> +        if [[ ${binary:0:1} != '-' ]]; then
> +            read -d ' ' binary <<< "${binary} "
> +            add_binary "${binary}"
> +        fi
> +    fi
> +
> +	if [[ ${key} == 'Requires' || ${key} == 'OnFailure' ]]; then
> +		for dep in ${value}; do
> +			add_systemd_unit ${dep}
> +		done
> +	fi

The per-key behavior is probably better expressed in a case statement:

  case $key in
    Requires|OnFailure)
      map add_systemd_unit "${values[@]}"
      ;;
    Exec)
      add_binary "${values[0]#-}"
      ;;
    ...
  esac

> +
> +    done < "$unit"
> +
> +    for dep in "${unitdir}"/*.{wants,requires}/$(basename $unit); do
> +	if [[ -h "${dep}" ]]; then
> +		add_symlink "${dep}"
> +	fi
> +    done
> +
> +    if [[ -d "${unit}.requires" ]]; then
> +	    for dep in "${unit}".requires/*; do
> +		add_systemd_unit $(basename $dep)
> +	    done
> +    fi
> +}
> +
>  parse_config() {
>      # parse key global variables set by the config file.
>
> --
> 1.8.2.1
>