Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] Compilation Issue with Commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4
@ 2025-11-29 17:14 José Luis Salvador Rufo
  2025-11-29 17:53 ` Yann E. MORIN via buildroot
  0 siblings, 1 reply; 7+ messages in thread
From: José Luis Salvador Rufo @ 2025-11-29 17:14 UTC (permalink / raw)
  To: Baruch Siach via buildroot, Yann E. MORIN

Hello Yann,

The commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4 broke my compilation.
An AI suggested that the problem is because the script
`support/scripts/check-merged` requires the following change:

diff --git a/support/scripts/check-merged b/support/scripts/check-merged
index 447abfd815..23fdaac2c8 100755
--- a/support/scripts/check-merged
+++ b/support/scripts/check-merged
@@ -47,6 +47,8 @@ while getopts "t:ub" OPT; do
        esac
 done

+shift $((OPTIND-1))
+
 if [ "${type}" = "skeleton" ]; then
        strict=true
 else

After applying this change, my compilation works again, in the same
way as reverting commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4.
I haven’t checked the reason for this `shift` myself.

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

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

* Re: [Buildroot] Compilation Issue with Commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4
  2025-11-29 17:14 [Buildroot] Compilation Issue with Commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4 José Luis Salvador Rufo
@ 2025-11-29 17:53 ` Yann E. MORIN via buildroot
  2025-11-29 19:35   ` Edgar Bonet via buildroot
  0 siblings, 1 reply; 7+ messages in thread
From: Yann E. MORIN via buildroot @ 2025-11-29 17:53 UTC (permalink / raw)
  To: José Luis Salvador Rufo; +Cc: buildroot

José, All,

On 2025-11-29 18:14 +0100, José Luis Salvador Rufo spake thusly:
> The commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4 broke my compilation.

Please explain what the problem is.

> An AI suggested that the problem is because the script
> `support/scripts/check-merged` requires the following change:

Did you understand why the suggested change is correct?

In the end, I don't think we specifically care how you came up with the
proposed change, but we need to know why this is correct. We don't need
some hand-waving "I don't understand why but it works, magic!", but
rather an explanation for why this is needed. This is needed for the
future, when we eventually need to revisit that commit later on, to
understand why we thought the fix was correct back then, and see if some
assumptions valid right now would still be then.

However, thanks for disclosing the use of an AI agent; note that my
reply would have been the same even if you had not.

Would you care to resubmit this patch with a proper commit log (look at
the history to get some ideas); you can also look at some directions in
the manual (it is a bit verbose, but contains a lot of important
information):

    https://buildroot.org/downloads/manual/manual.html#submitting-patches

Also, please add in the commit log that this fixes a [previous commit,
e.g. with a line readoing;

    Fixes: 793ebd5d28095c8df45a0d183d273d8a84b3f0a4

When you do that, you can add my:

    Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> diff --git a/support/scripts/check-merged b/support/scripts/check-merged
> index 447abfd815..23fdaac2c8 100755
> --- a/support/scripts/check-merged
> +++ b/support/scripts/check-merged
> @@ -47,6 +47,8 @@ while getopts "t:ub" OPT; do
>         esac
>  done
> 
> +shift $((OPTIND-1))
> +
>  if [ "${type}" = "skeleton" ]; then
>         strict=true
>  else
> 
> After applying this change, my compilation works again, in the same
> way as reverting commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4.
> I haven’t checked the reason for this `shift` myself.
> 
> Greetings.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] 7+ messages in thread

* Re: [Buildroot] Compilation Issue with Commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4
  2025-11-29 17:53 ` Yann E. MORIN via buildroot
@ 2025-11-29 19:35   ` Edgar Bonet via buildroot
  2025-11-29 20:23     ` Yann E. MORIN via buildroot
  2025-11-29 20:38     ` José Luis Salvador Rufo
  0 siblings, 2 replies; 7+ messages in thread
From: Edgar Bonet via buildroot @ 2025-11-29 19:35 UTC (permalink / raw)
  To: Yann E. MORIN, José Luis Salvador Rufo; +Cc: buildroot

Hello!

On 2025-11-29, José Luis Salvador Rufo wrote:
> The commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4 broke my compilation.
> An AI suggested that the problem is because the script
> `support/scripts/check-merged` requires the following change:
> 
> diff --git a/support/scripts/check-merged b/support/scripts/check-merged
> index 447abfd815..23fdaac2c8 100755
> --- a/support/scripts/check-merged
> +++ b/support/scripts/check-merged
> @@ -47,6 +47,8 @@ while getopts "t:ub" OPT; do
>         esac
>  done
> 
> +shift $((OPTIND-1))
> +
>  if [ "${type}" = "skeleton" ]; then
>         strict=true
>  else
> 
> After applying this change, my compilation works again, in the same
> way as reverting commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4.
> I haven’t checked the reason for this `shift` myself.

Yann E. Morin replied:
> Did you understand why the suggested change is correct?

I took a quick look at the commit in question (793ebd5d2809
"support/scripts/check-merged: use getopts instead of getopt"). My
understanding is that:

  • Before that commit, the options-parsing loop (`while :; do`, lines
    44 through 63) consumed (with `shift`) the options and left the
    other arguments in $@. Those extra arguments (the root directories
    to check) where then consumed by the `for root; do` loop lines 125
    through 144.

  • Since that commit, the options-parsing loop does not consume the
    options it finds. The `for root; do` loop over the root directories,
    however, has not been changed. This loop will then interpret the
    options as extra root directories to check.

@José Luis Salvador Rufo: I hope this can help you write a more
appropriate commit message.

Regards,

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

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

* Re: [Buildroot] Compilation Issue with Commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4
  2025-11-29 19:35   ` Edgar Bonet via buildroot
@ 2025-11-29 20:23     ` Yann E. MORIN via buildroot
  2025-11-29 20:38     ` José Luis Salvador Rufo
  1 sibling, 0 replies; 7+ messages in thread
From: Yann E. MORIN via buildroot @ 2025-11-29 20:23 UTC (permalink / raw)
  To: Edgar Bonet; +Cc: José Luis Salvador Rufo, buildroot

Edgar, All,

On 2025-11-29 20:35 +0100, Edgar Bonet spake thusly:
> On 2025-11-29, José Luis Salvador Rufo wrote:
> > The commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4 broke my compilation.
> > An AI suggested that the problem is because the script
> > `support/scripts/check-merged` requires the following change:
[--SNIP--]
> Yann E. Morin replied:
> > Did you understand why the suggested change is correct?
> 
> I took a quick look at the commit in question (793ebd5d2809
> "support/scripts/check-merged: use getopts instead of getopt"). My
> understanding is that:

Thanks for the helping hand! :-)

>   • Before that commit, the options-parsing loop (`while :; do`, lines
>     44 through 63) consumed (with `shift`) the options and left the
>     other arguments in $@. Those extra arguments (the root directories
>     to check) where then consumed by the `for root; do` loop lines 125
>     through 144.
> 
>   • Since that commit, the options-parsing loop does not consume the
>     options it finds. The `for root; do` loop over the root directories,
>     however, has not been changed. This loop will then interpret the
>     options as extra root directories to check.

That's a perfectly correct analysis of the issue. 👍

However, that does not explain why it causes José's build to fail. Here,
it does not fail. Of course, the script is broken, and instrumenting it
shows that it indeed tries to handle the options as if they were
directories to validate.

However, when I run a build here with a few different overlays (properly
merged usr or bin, or not at all), and enabling or disabling the
corresponding MERGED_{USR,BIN} options, do not fail for me. Thsat's the
reason I did not catch the issue when I submitted my patch.

Hence we don't yet have a complete explanations as to what is different
in José's case, that causes the build to fail for them. That is what is
intersting: why does it fail?

And yes, the change *is* correct, and yes, the script *is* currently
borked. Still, there's this behaviour that I can't reproduce, that is
important to understand.

> @José Luis Salvador Rufo: I hope this can help you write a more
> appropriate commit message.

Yes, Your explanations, Edgar, do have their place in th commit log, as
the reason to explain the build failure, that should be described first.

Maybe describe your setup: how many overlays you are using, if they are
merged-usr, merged-bin, or not, and so on...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] 7+ messages in thread

* Re: [Buildroot] Compilation Issue with Commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4
  2025-11-29 19:35   ` Edgar Bonet via buildroot
  2025-11-29 20:23     ` Yann E. MORIN via buildroot
@ 2025-11-29 20:38     ` José Luis Salvador Rufo
  2025-11-29 21:46       ` José Luis Salvador Rufo
  1 sibling, 1 reply; 7+ messages in thread
From: José Luis Salvador Rufo @ 2025-11-29 20:38 UTC (permalink / raw)
  To: Edgar Bonet; +Cc: Yann E. MORIN, buildroot

Thank you, Edgar Bonet, for your explanation. It makes a lot of sense now.

It was not my intention to submit my previous message/diff as a patch;
it was mostly meant as a "Just For Your Information" or a "Request for
Comments." My bad for not specifying that correctly in the subject.
Let me send a proper patch in my next message. ;-)

Greetings!

El sáb, 29 nov 2025 a las 20:35, Edgar Bonet
(<bonet@grenoble.cnrs.fr>) escribió:
>
> Hello!
>
> On 2025-11-29, José Luis Salvador Rufo wrote:
> > The commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4 broke my compilation.
> > An AI suggested that the problem is because the script
> > `support/scripts/check-merged` requires the following change:
> >
> > diff --git a/support/scripts/check-merged b/support/scripts/check-merged
> > index 447abfd815..23fdaac2c8 100755
> > --- a/support/scripts/check-merged
> > +++ b/support/scripts/check-merged
> > @@ -47,6 +47,8 @@ while getopts "t:ub" OPT; do
> >         esac
> >  done
> >
> > +shift $((OPTIND-1))
> > +
> >  if [ "${type}" = "skeleton" ]; then
> >         strict=true
> >  else
> >
> > After applying this change, my compilation works again, in the same
> > way as reverting commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4.
> > I haven’t checked the reason for this `shift` myself.
>
> Yann E. Morin replied:
> > Did you understand why the suggested change is correct?
>
> I took a quick look at the commit in question (793ebd5d2809
> "support/scripts/check-merged: use getopts instead of getopt"). My
> understanding is that:
>
>   • Before that commit, the options-parsing loop (`while :; do`, lines
>     44 through 63) consumed (with `shift`) the options and left the
>     other arguments in $@. Those extra arguments (the root directories
>     to check) where then consumed by the `for root; do` loop lines 125
>     through 144.
>
>   • Since that commit, the options-parsing loop does not consume the
>     options it finds. The `for root; do` loop over the root directories,
>     however, has not been changed. This loop will then interpret the
>     options as extra root directories to check.
>
> @José Luis Salvador Rufo: I hope this can help you write a more
> appropriate commit message.
>
> Regards,
>
> Edgar.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] Compilation Issue with Commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4
  2025-11-29 20:38     ` José Luis Salvador Rufo
@ 2025-11-29 21:46       ` José Luis Salvador Rufo
  2025-11-30  7:36         ` Yann E. MORIN via buildroot
  0 siblings, 1 reply; 7+ messages in thread
From: José Luis Salvador Rufo @ 2025-11-29 21:46 UTC (permalink / raw)
  To: Edgar Bonet; +Cc: Yann E. MORIN, buildroot

Hello Yann, all.

For disclosure, here is my build information that triggered the error:

build log:
```
[...]
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
[...]
```

Everything is running inside a container:
- /usr/src/simplek8s/ is my out-of-tree repository.
- /usr/src/simplek8s/buildroot is the buildroot git repository.

Some configs:
```
[...]
BR2_ROOTFS_SKELETON_CUSTOM=y
BR2_ROOTFS_SKELETON_CUSTOM_PATH="$(CONFIG_DIR)/rootfs-skeleton"
BR2_ROOTFS_OVERLAY="$(CONFIG_DIR)/rootfs-overlay"
[...]
```

Makefile (mostly, the `$(CURDIR)/buildroot`):
```
ifeq ("$(origin V)", "command line")
VERBOSE := $(V)
endif
ifneq ($(VERBOSE),1)
Q := @
endif

lastword = $(word $(words $(1)),$(1))
makedir := $(dir $(call lastword,$(MAKEFILE_LIST)))

MAKEARGS := -C $(CURDIR)/buildroot
MAKEARGS += O=$(if $(patsubst /%,,$(makedir)),$(CURDIR)/)$(patsubst
%/,%,$(makedir))

MAKEFLAGS += --no-print-directory

.PHONY: _all $(MAKECMDGOALS)

all     := $(filter-out Makefile,$(MAKECMDGOALS))

_all:
       $(Q)umask 0022 && $(MAKE) $(MAKEARGS) $(all)

Makefile:;

$(all): _all
       @:

%/: _all
       @:
```

rootfs skeleton:
```
$ tree -a rootfs-skeleton
rootfs-skeleton
├── etc
│   ├── group
│   ├── passwd
│   ├── resolv.conf
│   └── shadow
├── root
│   └── .gitkeep
├── run
│   └── .gitkeep
├── usr
│   ├── bin
│   │   └── .gitkeep
│   └── lib
│       └── .gitkeep
└── var
    ├── run -> ../run
    └── usr
        └── local
            └── .gitkeep


11 directories, 4 files
```

Greetings.

El sáb, 29 nov 2025 a las 21:38, José Luis Salvador Rufo
(<salvador.joseluis@gmail.com>) escribió:
>
> Thank you, Edgar Bonet, for your explanation. It makes a lot of sense now.
>
> It was not my intention to submit my previous message/diff as a patch;
> it was mostly meant as a "Just For Your Information" or a "Request for
> Comments." My bad for not specifying that correctly in the subject.
> Let me send a proper patch in my next message. ;-)
>
> Greetings!
>
> El sáb, 29 nov 2025 a las 20:35, Edgar Bonet
> (<bonet@grenoble.cnrs.fr>) escribió:
> >
> > Hello!
> >
> > On 2025-11-29, José Luis Salvador Rufo wrote:
> > > The commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4 broke my compilation.
> > > An AI suggested that the problem is because the script
> > > `support/scripts/check-merged` requires the following change:
> > >
> > > diff --git a/support/scripts/check-merged b/support/scripts/check-merged
> > > index 447abfd815..23fdaac2c8 100755
> > > --- a/support/scripts/check-merged
> > > +++ b/support/scripts/check-merged
> > > @@ -47,6 +47,8 @@ while getopts "t:ub" OPT; do
> > >         esac
> > >  done
> > >
> > > +shift $((OPTIND-1))
> > > +
> > >  if [ "${type}" = "skeleton" ]; then
> > >         strict=true
> > >  else
> > >
> > > After applying this change, my compilation works again, in the same
> > > way as reverting commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4.
> > > I haven’t checked the reason for this `shift` myself.
> >
> > Yann E. Morin replied:
> > > Did you understand why the suggested change is correct?
> >
> > I took a quick look at the commit in question (793ebd5d2809
> > "support/scripts/check-merged: use getopts instead of getopt"). My
> > understanding is that:
> >
> >   • Before that commit, the options-parsing loop (`while :; do`, lines
> >     44 through 63) consumed (with `shift`) the options and left the
> >     other arguments in $@. Those extra arguments (the root directories
> >     to check) where then consumed by the `for root; do` loop lines 125
> >     through 144.
> >
> >   • Since that commit, the options-parsing loop does not consume the
> >     options it finds. The `for root; do` loop over the root directories,
> >     however, has not been changed. This loop will then interpret the
> >     options as extra root directories to check.
> >
> > @José Luis Salvador Rufo: I hope this can help you write a more
> > appropriate commit message.
> >
> > Regards,
> >
> > Edgar.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] Compilation Issue with Commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4
  2025-11-29 21:46       ` José Luis Salvador Rufo
@ 2025-11-30  7:36         ` Yann E. MORIN via buildroot
  0 siblings, 0 replies; 7+ messages in thread
From: Yann E. MORIN via buildroot @ 2025-11-30  7:36 UTC (permalink / raw)
  To: José Luis Salvador Rufo; +Cc: Edgar Bonet, buildroot

José, All,

On 2025-11-29 22:46 +0100, José Luis Salvador Rufo spake thusly:
> For disclosure, here is my build information that triggered the error:
> 
> build log:
> ```
> [...]
> 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
> [...]
> ```
[--SNIP--]
> Some configs:
> ```
> [...]
> BR2_ROOTFS_SKELETON_CUSTOM=y
> BR2_ROOTFS_SKELETON_CUSTOM_PATH="$(CONFIG_DIR)/rootfs-skeleton"
> BR2_ROOTFS_OVERLAY="$(CONFIG_DIR)/rootfs-overlay"
> [...]
> ```

Ah, so this is what I missed: use of a custom skeleton. I tested all
kinds of overlays, but forgot to test a custom skeleton.

So, it did not break for overlays, while it breaks for skeleton, but
that is not weird: the checks for the skeleton are strict, while they
are lax for the overlays. So, that's what causes the failure for you.

This is important information, that should have been given in the commit
log (maybe not so extensively, but still),

Thank you for the feedback! 👍

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] 7+ messages in thread

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

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-29 17:14 [Buildroot] Compilation Issue with Commit 793ebd5d28095c8df45a0d183d273d8a84b3f0a4 José Luis Salvador Rufo
2025-11-29 17:53 ` Yann E. MORIN via buildroot
2025-11-29 19:35   ` Edgar Bonet via buildroot
2025-11-29 20:23     ` Yann E. MORIN via buildroot
2025-11-29 20:38     ` José Luis Salvador Rufo
2025-11-29 21:46       ` José Luis Salvador Rufo
2025-11-30  7:36         ` Yann E. MORIN via buildroot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox