[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Carlos Santos]

Since commit 0db34529f48 we use rsync with the --keep-dirlinks option to
prevent overlays from accidentally overwriding /{usr,bin,sbin,lib} links
when BR2_ROOTFS_MERGED_USR option is enabled. Unfortunately this also
prevents replacing a symlink by a directory on purpose (e.g. /var/log,
to persist system logs).

Steps to reproduce:

- enable BR2_ROOTFS_MERGED_USR and BR2_PACKAGE_SKELETON_INIT_SYSV
- mkdir some_path/rootfs-overlay/var/log
- enable BR2_ROOTFS_OVERLAY="some_path/rootfs-overlay"
- run 'make'
- 'target/var/log' is still a symlink to '../tmp', not a directory

Fix the problem by adding a step in target-finalize that checks each
overlay, using the same criteria used in skeleton-custom.mk.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 Makefile | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 957ba6634a..1b306f8556 100644
--- a/Makefile
+++ b/Makefile
@@ -751,9 +751,40 @@ endif
 	@$(call MESSAGE,"Sanitizing RPATH in target tree")
 	$(TOPDIR)/support/scripts/fix-rpath target
 
+# 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)
+
+	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
+		$(call MESSAGE,"Sanity check in overlay $(d)"); \
+		lib_inode=$$(stat -c '%i' $(d)/lib/. 2>/dev/null || true); \
+		bin_inode=$$(stat -c '%i' $(d)/bin/. 2>/dev/null || true); \
+		sbin_inode=$$(stat -c '%i' $(d)/sbin/. 2>/dev/null || true); \
+		usr_lib_inode=$$(stat -c '%i' $(d)/usr/lib/. 2>/dev/null || true); \
+		usr_bin_inode=$$(stat -c '%i' $(d)/usr/bin/. 2>/dev/null || true); \
+		usr_sbin_inode=$$(stat -c '%i' $(d)/usr/sbin/. 2>/dev/null || true); \
+		not_merged_dirs=""; \
+		test -z "$$lib_inode" || \
+			test "$$lib_inode" = "$$usr_lib_inode" || \
+				not_merged_dirs="$$not_merged_dirs /lib"; \
+		test -z "$$bin_inode" || \
+			test "$$bin_inode" = "$$usr_bin_inode" || \
+				not_merged_dirs="$$not_merged_dirs /bin"; \
+		test -z "$$sbin_inode" || \
+			test "$$sbin_inode" = "$$usr_sbin_inode" || \
+				not_merged_dirs="$$not_merged_dirs /sbin"; \
+		test -n "$$not_merged_dirs" && { \
+			echo "ERROR: The custom skeleton in $(d) is not" \
+				"using a merged /usr for the following directories:" \
+				$$not_merged_dirs; \
+			exit 1; \
+		} || true)
+
+endif # merged /usr
+
 	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
 		$(call MESSAGE,"Copying overlay $(d)"); \
-		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
+		rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
 			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
 			$(d)/ $(TARGET_DIR)$(sep))
 
-- 
2.14.3

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Yann E. MORIN]

Carlos, All,

On 2018-05-03 09:19 -0300, Carlos Santos spake thusly:
> Since commit 0db34529f48 we use rsync with the --keep-dirlinks option to
> prevent overlays from accidentally overwriding /{usr,bin,sbin,lib} links
> when BR2_ROOTFS_MERGED_USR option is enabled. Unfortunately this also
> prevents replacing a symlink by a directory on purpose (e.g. /var/log,
> to persist system logs).

In that case, I suggest your use a post-build script instead.

In facct, I think that any modifications that change the layout of the
filesystem should be done as a post-build script rather than an overlay.

So, I am pretty much reluctant to see this patch go in.

Regards,
Yann E. MORIN.

> Steps to reproduce:
> 
> - enable BR2_ROOTFS_MERGED_USR and BR2_PACKAGE_SKELETON_INIT_SYSV
> - mkdir some_path/rootfs-overlay/var/log
> - enable BR2_ROOTFS_OVERLAY="some_path/rootfs-overlay"
> - run 'make'
> - 'target/var/log' is still a symlink to '../tmp', not a directory
> 
> Fix the problem by adding a step in target-finalize that checks each
> overlay, using the same criteria used in skeleton-custom.mk.
> 
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
>  Makefile | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 957ba6634a..1b306f8556 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -751,9 +751,40 @@ endif
>  	@$(call MESSAGE,"Sanitizing RPATH in target tree")
>  	$(TOPDIR)/support/scripts/fix-rpath target
>  
> +# 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)
> +
> +	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
> +		$(call MESSAGE,"Sanity check in overlay $(d)"); \
> +		lib_inode=$$(stat -c '%i' $(d)/lib/. 2>/dev/null || true); \
> +		bin_inode=$$(stat -c '%i' $(d)/bin/. 2>/dev/null || true); \
> +		sbin_inode=$$(stat -c '%i' $(d)/sbin/. 2>/dev/null || true); \
> +		usr_lib_inode=$$(stat -c '%i' $(d)/usr/lib/. 2>/dev/null || true); \
> +		usr_bin_inode=$$(stat -c '%i' $(d)/usr/bin/. 2>/dev/null || true); \
> +		usr_sbin_inode=$$(stat -c '%i' $(d)/usr/sbin/. 2>/dev/null || true); \
> +		not_merged_dirs=""; \
> +		test -z "$$lib_inode" || \
> +			test "$$lib_inode" = "$$usr_lib_inode" || \
> +				not_merged_dirs="$$not_merged_dirs /lib"; \
> +		test -z "$$bin_inode" || \
> +			test "$$bin_inode" = "$$usr_bin_inode" || \
> +				not_merged_dirs="$$not_merged_dirs /bin"; \
> +		test -z "$$sbin_inode" || \
> +			test "$$sbin_inode" = "$$usr_sbin_inode" || \
> +				not_merged_dirs="$$not_merged_dirs /sbin"; \
> +		test -n "$$not_merged_dirs" && { \
> +			echo "ERROR: The custom skeleton in $(d) is not" \
> +				"using a merged /usr for the following directories:" \
> +				$$not_merged_dirs; \
> +			exit 1; \
> +		} || true)
> +
> +endif # merged /usr
> +
>  	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
>  		$(call MESSAGE,"Copying overlay $(d)"); \
> -		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
> +		rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
>  			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
>  			$(d)/ $(TARGET_DIR)$(sep))
>  
> -- 
> 2.14.3
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Carlos Santos]

> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "buildroot" <buildroot@buildroot.org>
> Sent: Saturday, May 5, 2018 7:01:00 AM
> Subject: Re: [Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

> Carlos, All,
> 
> On 2018-05-03 09:19 -0300, Carlos Santos spake thusly:
>> Since commit 0db34529f48 we use rsync with the --keep-dirlinks option to
>> prevent overlays from accidentally overwriding /{usr,bin,sbin,lib} links
>> when BR2_ROOTFS_MERGED_USR option is enabled. Unfortunately this also
>> prevents replacing a symlink by a directory on purpose (e.g. /var/log,
>> to persist system logs).
> 
> In that case, I suggest your use a post-build script instead.

That was my first approach.

> In facct, I think that any modifications that change the layout of the
> filesystem should be done as a post-build script rather than an overlay.
> 
> So, I am pretty much reluctant to see this patch go in.

This change does not prevent the user from using a post-build but I'd
prefer to let Buildroot check the correctness of user changes as much
as possible instead of just prohibiting them.

Overlays are simpler to create and less error prone than scripts. They
are also self-documented, since a "find" or "tree" command shows the
resulting structure.

-- 
Carlos Santos (Casantos) - DATACOM, P&D

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Carlos, All,
 > On 2018-05-03 09:19 -0300, Carlos Santos spake thusly:
 >> Since commit 0db34529f48 we use rsync with the --keep-dirlinks option to
 >> prevent overlays from accidentally overwriding /{usr,bin,sbin,lib} links
 >> when BR2_ROOTFS_MERGED_USR option is enabled. Unfortunately this also
 >> prevents replacing a symlink by a directory on purpose (e.g. /var/log,
 >> to persist system logs).

 > In that case, I suggest your use a post-build script instead.

 > In facct, I think that any modifications that change the layout of the
 > filesystem should be done as a post-build script rather than an overlay.

Why? Overlays is the the recommended way to add/override stuff, and the
'everything-you-put-in-the-overlay-will-override-the-buildroot-defaults'
is a pretty nice and easy to understand behaviour.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Yann E. MORIN]

Carlos, All,

On 2018-05-05 10:47 -0300, Carlos Santos spake thusly:
> > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > To: "Carlos Santos" <casantos@datacom.ind.br>
> > Cc: "buildroot" <buildroot@buildroot.org>
> > Sent: Saturday, May 5, 2018 7:01:00 AM
> > Subject: Re: [Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled
> 
> > Carlos, All,
> > 
> > On 2018-05-03 09:19 -0300, Carlos Santos spake thusly:
> >> Since commit 0db34529f48 we use rsync with the --keep-dirlinks option to
> >> prevent overlays from accidentally overwriding /{usr,bin,sbin,lib} links
> >> when BR2_ROOTFS_MERGED_USR option is enabled. Unfortunately this also
> >> prevents replacing a symlink by a directory on purpose (e.g. /var/log,
> >> to persist system logs).
> > 
> > In that case, I suggest your use a post-build script instead.
> 
> That was my first approach.
> 
> > In facct, I think that any modifications that change the layout of the
> > filesystem should be done as a post-build script rather than an overlay.
> > 
> > So, I am pretty much reluctant to see this patch go in.
> 
> This change does not prevent the user from using a post-build but I'd
> prefer to let Buildroot check the correctness of user changes as much
> as possible instead of just prohibiting them.

It's not about prohibiting them.

But I'd prefer we keep things simple in Buildroot, and since there is
already a way to do it, I don't mind that overlays do not provide this
solution. A post-build script is way more versatile when it comes to
changing the layout.

> Overlays are simpler to create and less error prone than scripts. They
> are also self-documented, since a "find" or "tree" command shows the
> resulting structure.

True, but they are less versatile...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Peter Korsgaard]

>>>>> "Carlos" == Carlos Santos <casantos@datacom.ind.br> writes:

 > Since commit 0db34529f48 we use rsync with the --keep-dirlinks option to
 > prevent overlays from accidentally overwriding /{usr,bin,sbin,lib} links
 > when BR2_ROOTFS_MERGED_USR option is enabled. Unfortunately this also
 > prevents replacing a symlink by a directory on purpose (e.g. /var/log,
 > to persist system logs).

 > Steps to reproduce:

 > - enable BR2_ROOTFS_MERGED_USR and BR2_PACKAGE_SKELETON_INIT_SYSV
 > - mkdir some_path/rootfs-overlay/var/log
 > - enable BR2_ROOTFS_OVERLAY="some_path/rootfs-overlay"
 > - run 'make'
 > - 'target/var/log' is still a symlink to '../tmp', not a directory

Funny enough I ran into the exact same issue (also with /var/log)
yesterday, and I agree that it isn't really obvious why it doesn't work
whereas overwriting symlinks to files does work (E.G. our
/etc/resolv.conf -> /tmp/resolv.conf can be overridden by an overlay
containing /etc/resolv.conf).


 > Fix the problem by adding a step in target-finalize that checks each
 > overlay, using the same criteria used in skeleton-custom.mk.

It would be good if we could refactor this so we wouldn't have two
copies of the same code, either by moving it to a variable or simply
checking TARGET_DIR after overlays and post-build scripts?


 > Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
 > ---
 >  Makefile | 33 ++++++++++++++++++++++++++++++++-
 >  1 file changed, 32 insertions(+), 1 deletion(-)

 > diff --git a/Makefile b/Makefile
 > index 957ba6634a..1b306f8556 100644
 > --- a/Makefile
 > +++ b/Makefile
 > @@ -751,9 +751,40 @@ endif
 >  	@$(call MESSAGE,"Sanitizing RPATH in target tree")
 >  	$(TOPDIR)/support/scripts/fix-rpath target
 
 > +# 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)
 > +
 > +	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
 > +		$(call MESSAGE,"Sanity check in overlay $(d)"); \
 > +		lib_inode=$$(stat -c '%i' $(d)/lib/. 2>/dev/null || true); \
 > +		bin_inode=$$(stat -c '%i' $(d)/bin/. 2>/dev/null || true); \
 > +		sbin_inode=$$(stat -c '%i' $(d)/sbin/. 2>/dev/null || true); \
 > +		usr_lib_inode=$$(stat -c '%i' $(d)/usr/lib/. 2>/dev/null || true); \
 > +		usr_bin_inode=$$(stat -c '%i' $(d)/usr/bin/. 2>/dev/null || true); \
 > +		usr_sbin_inode=$$(stat -c '%i' $(d)/usr/sbin/. 2>/dev/null || true); \
 > +		not_merged_dirs=""; \
 > +		test -z "$$lib_inode" || \
 > +			test "$$lib_inode" = "$$usr_lib_inode" || \
 > +				not_merged_dirs="$$not_merged_dirs /lib"; \
 > +		test -z "$$bin_inode" || \
 > +			test "$$bin_inode" = "$$usr_bin_inode" || \
 > +				not_merged_dirs="$$not_merged_dirs /bin"; \
 > +		test -z "$$sbin_inode" || \
 > +			test "$$sbin_inode" = "$$usr_sbin_inode" || \
 > +				not_merged_dirs="$$not_merged_dirs /sbin"; \
 > +		test -n "$$not_merged_dirs" && { \
 > +			echo "ERROR: The custom skeleton in $(d) is not" \

s/custom skeleton/overlay/

>  	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
 >  		$(call MESSAGE,"Copying overlay $(d)"); \
 > -		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
 > +		rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
 >  			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
 >  			$(d)/ $(TARGET_DIR)$(sep))

This is afaik now identical to the SYSTEM_RSYNC logic we have in
system/system.mk, so it would be nicer to use that variable here.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 >> > So, I am pretty much reluctant to see this patch go in.
 >> 
 >> This change does not prevent the user from using a post-build but I'd
 >> prefer to let Buildroot check the correctness of user changes as much
 >> as possible instead of just prohibiting them.

 > It's not about prohibiting them.

 > But I'd prefer we keep things simple in Buildroot, and since there is
 > already a way to do it, I don't mind that overlays do not provide this
 > solution. A post-build script is way more versatile when it comes to
 > changing the layout.

Yes, with a post-build script you have full flexibility, but we have
quite a lot of helper functionality to handle "common" fixups in a
simpler way than though a post-build script, E.G. stripping, removing
.py files, overlays, ..

This restriction for overlays isn't obvious and wasn't originally there
(before the merged /usr support), so it is arguably a regression (but
for an edge case as this afaik is the first report of it).

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Yann E. MORIN]

Peter, All,

On 2018-05-05 16:41 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  > Carlos, All,
>  > On 2018-05-03 09:19 -0300, Carlos Santos spake thusly:
>  >> Since commit 0db34529f48 we use rsync with the --keep-dirlinks option to
>  >> prevent overlays from accidentally overwriding /{usr,bin,sbin,lib} links
>  >> when BR2_ROOTFS_MERGED_USR option is enabled. Unfortunately this also
>  >> prevents replacing a symlink by a directory on purpose (e.g. /var/log,
>  >> to persist system logs).
> 
>  > In that case, I suggest your use a post-build script instead.
> 
>  > In facct, I think that any modifications that change the layout of the
>  > filesystem should be done as a post-build script rather than an overlay.
> 
> Why? Overlays is the the recommended way to add/override stuff, and the
> 'everything-you-put-in-the-overlay-will-override-the-buildroot-defaults'
> is a pretty nice and easy to understand behaviour.

Well, I disagree quite a bit: overlays are ugly and 99% of what users
do with an overlay can be done with a package (i.e. adding data blobs).
The remaining 1% can be done with a post-build script.

Besides, what you put in an overlay is never accoutnted for: no
graph-size, no legal-info, no nothing...

Repeat after me: overlays are ugly! ;-)

Two things that overlays are noce for:

  - provide /etc/-style config files to be written over the default ones,

  - provide test-data that is not supposed to go into production, and
    for which a package may be a bit overkill, and even then...

Otherwise, overlays are just a pita and, long term, are a liability...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Yann E. MORIN]

Peter, All,

On 2018-05-05 17:01 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>  >> > So, I am pretty much reluctant to see this patch go in.
>  > But I'd prefer we keep things simple in Buildroot, and since there is
>  > already a way to do it, I don't mind that overlays do not provide this
>  > solution. A post-build script is way more versatile when it comes to
>  > changing the layout.
> 
> Yes, with a post-build script you have full flexibility, but we have
> quite a lot of helper functionality to handle "common" fixups in a
> simpler way than though a post-build script, E.G. stripping, removing
> .py files, overlays, ..
> 
> This restriction for overlays isn't obvious and wasn't originally there
> (before the merged /usr support), so it is arguably a regression (but
> for an edge case as this afaik is the first report of it).

I still believe that such *modifications* to the layout be actually not
supported with overlays.

Today, someone wants to be able to overrids /var/log from a symlink to a
directory to be able to store remanent data there. But then, that will
require a modification in fstab to mount an actual filesystem there. And
such a modification would be done by a post-build script (if it comes
from an overlay, there is no way this will work as-is between buildroot
versions, becasue buildroot may add/remove stuff in there as well).

So, a post-build script is really needed anyway...

And then, someone will want to change something else, e.g. a file to a
symlink or a directory to a symlink or whatever...

Also, overlays are not copied under fakeroot (but OK, post-build script
neither, but we have fakeroot-scripts). So, if one wants special rights,
it's not possible with overlays either...

So, really, overlays are not as nice as one may think...

As I said, in the long run, they are more a liability than anything
else.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 >> Why? Overlays is the the recommended way to add/override stuff, and the
 >> 'everything-you-put-in-the-overlay-will-override-the-buildroot-defaults'
 >> is a pretty nice and easy to understand behaviour.

 > Well, I disagree quite a bit: overlays are ugly and 99% of what users
 > do with an overlay can be done with a package (i.e. adding data blobs).
 > The remaining 1% can be done with a post-build script.

 > Besides, what you put in an overlay is never accoutnted for: no
 > graph-size, no legal-info, no nothing...

 > Repeat after me: overlays are ugly! ;-)

 > Two things that overlays are noce for:

 >   - provide /etc/-style config files to be written over the default ones,

 >   - provide test-data that is not supposed to go into production, and
 >     for which a package may be a bit overkill, and even then...

 > Otherwise, overlays are just a pita and, long term, are a liability...

Do you feel better now? ;) I agree that they can be abused - But don't
forget, overlays were added to get people away from custom skeletons. I
still find overlays quite a bit nicer than custom skeletons.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Arnout Vandecappelle]

 TL;DR: I'm with Peter and Carlos on this one.

On 05-05-18 17:21, Yann E. MORIN wrote:
> Peter, All,
> 
> On 2018-05-05 17:01 +0200, Peter Korsgaard spake thusly:
>>>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>>  >> > So, I am pretty much reluctant to see this patch go in.
>>  > But I'd prefer we keep things simple in Buildroot, and since there is
>>  > already a way to do it, I don't mind that overlays do not provide this
>>  > solution. A post-build script is way more versatile when it comes to
>>  > changing the layout.
>>
>> Yes, with a post-build script you have full flexibility, but we have
>> quite a lot of helper functionality to handle "common" fixups in a
>> simpler way than though a post-build script, E.G. stripping, removing
>> .py files, overlays, ..
>>
>> This restriction for overlays isn't obvious and wasn't originally there
>> (before the merged /usr support), so it is arguably a regression (but
>> for an edge case as this afaik is the first report of it).
> 
> I still believe that such *modifications* to the layout be actually not
> supported with overlays.

 I too believe that overlays are a much simpler way to make modifications than
either post-build scripts or packages. In fact, I think post-build scripts are a
PITA :-)


> Today, someone wants to be able to overrids /var/log from a symlink to a
> directory to be able to store remanent data there. But then, that will
> require a modification in fstab to mount an actual filesystem there.

 Or an init script (or service file).

> And
> such a modification would be done by a post-build script (if it comes
> from an overlay, there is no way this will work as-is between buildroot
> versions, becasue buildroot may add/remove stuff in there as well).

 Well, Buildroot shouldn't do that, because it breaks top-level parallel build.
In fact, Buildroot will now issue a warning when two packages touch the same
file - fstab is part of skeleton so no other package should touch it. [Not that
it matters in this discussion, but currently only libselinux modifies fstab.]

 And anyway, if you overwrite a file with your overlay, you supposedly know what
you're doing.


> So, a post-build script is really needed anyway...
> 
> And then, someone will want to change something else, e.g. a file to a
> symlink or a directory to a symlink or whatever...
> 
> Also, overlays are not copied under fakeroot (but OK, post-build script
> neither, but we have fakeroot-scripts). So, if one wants special rights,
> it's not possible with overlays either...

 BR2_ROOTFS_DEVICE_TABLE?


 Regards,
 Arnout

> So, really, overlays are not as nice as one may think...
> 
> As I said, in the long run, they are more a liability than anything
> else.
> 
> Regards,
> Yann E. MORIN.
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF