Re: [Buildroot] [PATCH v1 1/4] board/zynqmp/post-build.sh: auto-generate extlinux.conf

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Neal, All,

On 2022-05-22 07:15 -0600, Neal Frager via buildroot spake thusly:
> This patch uses the BR2_ROOTFS_POST_SCRIPT_ARGS to auto-generate the
> extlinux.conf file, so developers will only need to modify the
> board_defconfig file to change the console and boot file system locations.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
>  board/zynqmp/post-build.sh | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/board/zynqmp/post-build.sh b/board/zynqmp/post-build.sh
> index 9fd8bbf2c8..749784c3f3 100755
> --- a/board/zynqmp/post-build.sh
> +++ b/board/zynqmp/post-build.sh
> @@ -4,5 +4,19 @@
>  # in the binaries directory
>  
>  BOARD_DIR="$(dirname $0)"
> +CONSOLE=$2
> +ROOT=$3
> +
> +FILE=$BOARD_DIR/extlinux.conf
> +if test -f "$FILE"; then
> +    echo "$FILE aready exists."
> +else
> +  cat << __HEADER_EOF >> $BOARD_DIR/extlinux.conf

Why would you need append-redirection >> ? Afterall, you just tested
that the file did not already exist... Anyway, see below...

> +label linux
> +  kernel /Image
> +  devicetree /system.dtb
> +  append console=$CONSOLE root=/dev/$ROOT rw rootwait
> +__HEADER_EOF
> +fi
> 
>  install -m 0644 -D $BOARD_DIR/extlinux.conf $BINARIES_DIR/extlinux.conf

We try to avoid generating files in the source tree.

Instead, why not just:

    FILE="${BOARD_DIR}/extlinux.conf"
    if test -f "$FILE"; then
        install -m 0644 -D "${BOARD_DIR}/extlinux.conf" "${BINARIES_DIR}/extlinux.conf"
    else
        mkdir -p "${BINARIES_DIR}"
        cat <<-__HEADER_EOF >"${BINARIES_DIR}/extlinux.conf"
    TAB>label linux
    TAB>  kernel /Image
    TAB>  devicetree /system.dtb
    TAB>  append console=${CONSOLE} root=/dev/${ROOT} rw rootwait
    TAB>__HEADER_EOF
    fi

Note: the <<- rediretion removes leading TABs, so it is allows a nicer
indentation (so, replace TAB> above with an actual TAB, of course).

Also, please always evaluate variables using curly braces and quote the
expansion, like so:   "${foo}"   (in the here-document, quotes are not
needed, of course).

Eventualy, at the end of this series, the script will always be used for
boards that do not already have an extlinux.conf, the condition can be
dropped in a final patch, to just only keep the else part of the
condition, don't you think?

Regards,
Yann E. MORIN.

> -- 
> 2.17.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot