All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] support/scripts/check-merged: shift args processed by getopts
@ 2025-11-29 20:34 José Luis Salvador Rufo
  2025-11-30  7:51 ` Yann E. MORIN via buildroot
  0 siblings, 1 reply; 3+ messages in thread
From: José Luis Salvador Rufo @ 2025-11-29 20:34 UTC (permalink / raw)
  To: buildroot; +Cc: José Luis Salvador Rufo, Yann E . MORIN, Edgar Bonet

Shift the arguments processed by getopts so that only the root directories
to check remain as arguments for the subsequent `for root; do` loop.

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

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Buildroot] [PATCH 1/1] support/scripts/check-merged: shift args processed by getopts
  2025-11-29 20:34 [Buildroot] [PATCH 1/1] support/scripts/check-merged: shift args processed by getopts José Luis Salvador Rufo
@ 2025-11-30  7:51 ` Yann E. MORIN via buildroot
  2025-11-30 17:06   ` Peter Korsgaard
  0 siblings, 1 reply; 3+ messages in thread
From: Yann E. MORIN via buildroot @ 2025-11-30  7:51 UTC (permalink / raw)
  To: José Luis Salvador Rufo; +Cc: buildroot, Edgar Bonet

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Buildroot] [PATCH 1/1] support/scripts/check-merged: shift args processed by getopts
  2025-11-30  7:51 ` Yann E. MORIN via buildroot
@ 2025-11-30 17:06   ` Peter Korsgaard
  0 siblings, 0 replies; 3+ messages in thread
From: Peter Korsgaard @ 2025-11-30 17:06 UTC (permalink / raw)
  To: Yann E. MORIN via buildroot
  Cc: José Luis Salvador Rufo, Yann E. MORIN, Edgar Bonet

>>>>> "Yann" == Yann E MORIN via buildroot <buildroot@buildroot.org> writes:

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

Committed with Yanns commit message (minus a few typos), thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-11-30 17:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-29 20:34 [Buildroot] [PATCH 1/1] support/scripts/check-merged: shift args processed by getopts José Luis Salvador Rufo
2025-11-30  7:51 ` Yann E. MORIN via buildroot
2025-11-30 17:06   ` Peter Korsgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.