Re: [Buildroot] [PATCH 07/13] support/scripts; teach check-merged what to check

From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Arnout Vandecappelle <arnout@rnout.be>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 07/13] support/scripts; teach check-merged what to check
Date: Sat, 17 May 2025 17:07:25 +0200	[thread overview]
Message-ID: <aCimLWtydyZb0lxf@landeda> (raw)
In-Reply-To: <447ef018-8c6c-4800-9013-0239886c9da1@rnout.be>

Arnout, All,

On 2025-05-17 12:24 +0200, Arnout Vandecappelle spake thusly:
> On 14/05/2025 18:58, Yann E. MORIN wrote:
> > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
[--SNIP--]
> > diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
> > index 7d64959711..e583d2d731 100644
> > --- a/package/skeleton-custom/skeleton-custom.mk
> > +++ b/package/skeleton-custom/skeleton-custom.mk
> > @@ -25,10 +25,11 @@ endif
> >   # For a merged /usr, ensure that /lib, /bin and /sbin and their /usr
> >   # counterparts are appropriately setup as symlinks ones to the others.
> > -ifeq ($(BR2_ROOTFS_MERGED_USR),y)
> >   SKELETON_CUSTOM_NOT_MERGED_USR_DIRS = \
> > -	$(shell support/scripts/check-merged $(SKELETON_CUSTOM_PATH))
> > -endif # merged /usr
> > +	$(shell support/scripts/check-merged \
> > +		$(SKELETON_CUSTOM_PATH) \
> > +		$(if $(BR2_ROOTFS_MERGED_USR),merged-usr) \
> > +	)
> 
>  While you're anyway working on this: I think there's little point of doing
> this as a "preliminary" check, I think it's fine to do this check while
> "building" the custom skeleton (i.e. in
> SKELETON_CUSTOM_INSTALL_TARGET_CMDS).
> 
>  That would allow us to move the error message to the check-merged script
> itself, and let it return 1 in case it's not properly merged. And with that,
> both instances can simply call the script without capturing its output. It's
> going to save +-10 lines of code and make things easier to understand IMHO.

I was considering something in that spirit, so your suggesting it just
confirms that it is a good idea, and I'll look into doing that.

> >   ifeq ($(BR2_PACKAGE_SKELETON_CUSTOM)$(BR_BUILDING),yy)
> >   ifneq ($(SKELETON_CUSTOM_NOT_MERGED_USR_DIRS),)
> > diff --git a/support/scripts/check-merged b/support/scripts/check-merged
> > index 9459d975fe..7fea4e1dea 100755
> > --- a/support/scripts/check-merged
> > +++ b/support/scripts/check-merged
> > @@ -11,12 +11,14 @@
> >   #
> >   # Input:
> >   #   $1:     the root directory (skeleton, overlay) to check
> > +#   $*:     the list of merged checks to perform: merged-usr
> 
>  If you make this an option (i.e. --merged-usr), then we can replace $1 with
> a $* that covers all directories, and we can get rid of the foreach in
> Makefile's check of the overlays, and that code could simplify to:
> 
> 	@$(call MESSAGE,"Sanity check in overlays)
> 	support/scripts/check-merged \
> 		$(if $(BR2_ROOTFS_MERGED_USR),--merged-usr) \
> 		$(call qstrip,$(BR2_ROOTFS_OVERLAY))
> 
>  Of course, there's a risk that I'm overengineering things, so feel free to
> ignore my suggestions.

I also like it a bit better, indeed. Let's see what I can come up
with...

Thanks.

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