Re: [ptxdist] [PATCH v6 3/5] scripts: add helper to inject files into a source directory

[Alexander Dahl <ada@thorsis.com>]

Hello Michael,

I know this has been applied already, but while working on the extension I 
asked about last week, I stumbled over some things.  See below.

Am Samstag, 29. Januar 2022, 08:03:28 CEST schrieb Michael Riesch:
> Some packages may require certain files that are maintained
> or generated outside of their source repository. For example,
> binary firmware blobs could be excluded from the sources due
> to licensing issues. Add a helper that allows to inject certain
> files into the source directory (usually in the prepare stage).
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
> 
> Notes:
>     v6:
>     - replaced 'break' with 'return'
> 
>  rules/post/ptxd_make_world_inject.make | 19 ++++++++++++
>  scripts/lib/ptxd_make_world_inject.sh  | 42 ++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
>  create mode 100644 rules/post/ptxd_make_world_inject.make
>  create mode 100644 scripts/lib/ptxd_make_world_inject.sh
> 
> diff --git a/rules/post/ptxd_make_world_inject.make
> b/rules/post/ptxd_make_world_inject.make new file mode 100644
> index 000000000..b7d28e92f
> --- /dev/null
> +++ b/rules/post/ptxd_make_world_inject.make
> @@ -0,0 +1,19 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2021 by Michael Riesch <michael.riesch@wolfvision.net>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +world/inject/env = \
> +	$(call world/env, $(1)) \
> +	pkg_inject_path="$($(1)_INJECT_PATH)" \
> +	pkg_inject_files="$($(1)_INJECT_FILES)" \
> +	pkg_source="$($(1)_DIR)"

Why passing 'pkg_source' here?  world/env/impl and thus world/env has 
'pkg_dir' and it has exactly the same content which is available in the shell 
script later.

Second thing: world/env is assigned in 
'rules/post/ptxd_make_world_common.make' and the interesting part looks like 
this:

    pkg_dir="$(call ptx/escape,$($(1)_DIR))"

Same for all the other variables with paths.  Why was ptx/escape not used on 
<PKG>_INJECT_PATH and <PKG>_INJECT_FILES here?

> +
> +world/inject = \
> +	$(call world/inject/env,$(strip $(1))) \
> +	ptxd_make_world_inject
> +
> +# vim: syntax=make
> diff --git a/scripts/lib/ptxd_make_world_inject.sh
> b/scripts/lib/ptxd_make_world_inject.sh new file mode 100644
> index 000000000..fe4eb8363
> --- /dev/null
> +++ b/scripts/lib/ptxd_make_world_inject.sh
> @@ -0,0 +1,42 @@
> +#!/bin/bash
> +#
> +# Copyright (C) 2021 by Michael Riesch <michael.riesch@wolfvision.net>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +ptxd_make_inject() {
> +    local source target
> +
> +    source="$(echo ${inject_file} | cut -d ":" -f 1)"
> +    target="${pkg_source}/$(echo ${inject_file} | cut -d ":" -f 2)"
> +    if [ -z "${target}" ]; then
> +	target="${source}"
> +    fi

Here ${target} can never be empty, because if both ${pkg_source} and $(echo …) 
are empty, there's still the '/' in the middle.  So the last three lines of 
this block make no sense.  Some leftover from earlier iterations?

Greets
Alex

> +
> +    if [[ "${source}" =~ ^/.* ]]; then
> +	ptxd_bailout "'${source}' must not be an absolute path!" \
> +	    "Use <PKG>_INJECT_PATH to specify the search path."
> +    fi
> +
> +    if ! ptxd_in_path pkg_inject_path "${source}"; then
> +	ptxd_bailout "Blob '${source}' not found in '${pkg_inject_path}'."
> +    fi
> +    source="${ptxd_reply}"
> +
> +    echo -e "\nInject file $(ptxd_print_path ${source}) into" \
> +	 "$(ptxd_print_path ${target})..."
> +    cp ${source} ${target}
> +}
> +export -f ptxd_make_inject
> +
> +
> +ptxd_make_world_inject() {
> +    ptxd_make_world_init || return
> +
> +    for inject_file in ${pkg_inject_files}; do
> +	ptxd_make_inject || return
> +    done
> +}
> +export -f ptxd_make_world_inject