Re: [Buildroot] [PATCH 1/1] support/scripts/check-merged: shift args processed by getopts

["Yann E. MORIN via buildroot" <buildroot@buildroot.org>]

José, All,

On 2025-11-29 21:34 +0100, José Luis Salvador Rufo spake thusly:
> Shift the arguments processed by getopts so that only the root directories
> to check remain as arguments for the subsequent `for root; do` loop.

The actual issue this triggers is that shecks in custom skeleton cause
build failure:

    support/scripts/check-merged -t skeleton -u -b /usr/src/simplek8s/rootfs-skeleton
    The skeleton in -t is not properly setup:
     - /usr/bin should exist, be a directory, and not be a symlink
     - /usr/lib should exist, be a directory, and not be a symlink
    The skeleton in skeleton is not properly setup:
     - /usr/bin should exist, be a directory, and not be a symlink
     - /usr/lib should exist, be a directory, and not be a symlink
    The skeleton in -u is not properly setup:
     - /usr/bin should exist, be a directory, and not be a symlink
     - /usr/lib should exist, be a directory, and not be a symlink
    The skeleton in -b is not properly setup:
     - /usr/bin should exist, be a directory, and not be a symlink
     - /usr/lib should exist, be a directory, and not be a symlink
    [...]

The reason this was not caught earlier, is that the script was only
tested against overlays, not custom skeletons. The difference between
skeletons and overlays, is that the tests are strict for skeletons, and
lax for overlays. Thus, with a custom skeleton, where the merged
symlinks are missing, the tests go wrong and report am incorrect issue,
while with overlays, the checks fails, but they are not critical for
obverlays, so  the script is silent and suceeds.

I think a better commit log would be:

    support/scripts/check-merged: fix checking custom skeletons

    When using a custom skeleton where the merged symlinks ar emising,
    the build fails with errors like:

        support/scripts/check-merged -t skeleton -u -b /usr/src/simplek8s/rootfs-skeleton
        The skeleton in -t is not properly setup:
         - /usr/bin should exist, be a directory, and not be a symlink
         - /usr/lib should exist, be a directory, and not be a symlink
        The skeleton in skeleton is not properly setup:
         - /usr/bin should exist, be a directory, and not be a symlink
         - /usr/lib should exist, be a directory, and not be a symlink
        [...]

    Commit 793ebd5d2809 (support/scripts/check-merged: use getopts
    instead of getopt) intoduced a flawed use of getopts: unlike getopt,
    getopts does not conume the positional arguments. This causes the
    check for directrory validity to all check each option as if they
    were directories.

    For overlays, this is transparently ignored, because the checks are
    only lax for overlays (missing symlinks are OK).

    However, for skeletons, the checks are strict. Because of that, a
    missing symlink is considered an error, when it should be considered
    as being OK.

    The fix is to actually consume the positional args to only keep the
    list of directpries to validate, like is done for example in
    support/download/dl-wrapper.

    Fixes: 793ebd5d28095c8df45a0d183d273d8a84b3f0a4
    Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>
    Acked-by: Yann E. MORIN <yann.morin.1998@free.fr>
    Cc: Edgar Bonet <bonet@grenoble.cnrs.fr>
    Signed-off-by: José Luis Salvador Rufo <salvador.joseluis@gmail.com>

Thank you José for reporting and fixing the issue! Thank you Edgar for
giving a helping hand!

Regards,
Yann E. MORIN.

> Fixes: 793ebd5d28095c8df45a0d183d273d8a84b3f0a4
> Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Edgar Bonet <bonet@grenoble.cnrs.fr>
> Signed-off-by: José Luis Salvador Rufo <salvador.joseluis@gmail.com>
> ---
>  support/scripts/check-merged | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/support/scripts/check-merged b/support/scripts/check-merged
> index 447abfd815..57db8abd21 100755
> --- a/support/scripts/check-merged
> +++ b/support/scripts/check-merged
> @@ -47,6 +47,10 @@ while getopts "t:ub" OPT; do
>  	esac
>  done
>  
> +# Remove the options processed by getopts from $@,
> +# so that $@ now contains only the root directories to check.
> +shift $((OPTIND -1))
> +
>  if [ "${type}" = "skeleton" ]; then
>  	strict=true
>  else
> -- 
> 2.52.0
> 

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