Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2 0/4] Improve verification of custom rootfs skeletons and overlays
@ 2018-05-07 12:46 Carlos Santos
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 1/4] skeleton-custom: use a script to check merged usr structure Carlos Santos
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Carlos Santos @ 2018-05-07 12:46 UTC (permalink / raw)
  To: buildroot

This series makes some improvements in the verification of custom rootfs
skeletons and overlays, regarding mergerd /usr:

Patch 1 adds a script to check if a given path complies to the merged /usr
requirements and makes skeleton-custom.mk use it instead of a bunch of
variables filled by $(shell ...) macros.

Patch 2 ensures that /bin, /lib and /sbin are created for custom skeletons,
either as directories or symlinks, according to BR2_ROOTFS_MERGED_USR. 

Patch 3 uses the script added in patch 1 to check rootfs overlays, in
target-finalize.

Patch 3 removes the restriction of using merged /usr only with the default
skeleton or when systemd is selected.

Carlos Santos (4):
  skeleton-custom: use a script to check merged usr structure
  skeleton-custom: install /bin, /lib, and /sbin
  Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled
  system: allow selecting merged /usr along with custom rootfs skeleton

 Makefile                                   | 20 ++++++++++++---
 docs/manual/customize-rootfs.txt           | 17 +++++++++++++
 package/skeleton-custom/skeleton-custom.mk | 25 +++----------------
 support/scripts/check-merged-usr.sh        | 39 ++++++++++++++++++++++++++++++
 system/Config.in                           |  8 ++----
 5 files changed, 78 insertions(+), 31 deletions(-)
 create mode 100755 support/scripts/check-merged-usr.sh

-- 
2.14.3

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

* [Buildroot] [PATCH v2 1/4] skeleton-custom: use a script to check merged usr structure
  2018-05-07 12:46 [Buildroot] [PATCH v2 0/4] Improve verification of custom rootfs skeletons and overlays Carlos Santos
@ 2018-05-07 12:46 ` Carlos Santos
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 2/4] skeleton-custom: install /bin, /lib, and /sbin Carlos Santos
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Carlos Santos @ 2018-05-07 12:46 UTC (permalink / raw)
  To: buildroot

Introduce support/scripts/check-merged-usr.sh, a script that check if a
given path complies to the merged /usr requirements:

    /
    /bin -> usr/bin
    /lib -> usr/lib
    /sbin -> usr/sbin
    /usr/bin/
    /usr/lib/
    /usr/sbin/

Use this script in skeleton-custom.mk instead of a bunch of variables
filled by $(shell ...) macros. The same script will be used to check
rootfs overlays, in a forthcoming change.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v1->v2:

- Rebase series to HEAD of master branch
---
 package/skeleton-custom/skeleton-custom.mk | 23 +-----------------
 support/scripts/check-merged-usr.sh        | 39 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 22 deletions(-)
 create mode 100755 support/scripts/check-merged-usr.sh

diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
index 8c57531782..b1cddd9146 100644
--- a/package/skeleton-custom/skeleton-custom.mk
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -23,32 +23,11 @@ $(error No path specified for the custom skeleton)
 endif
 endif
 
-# Extract the inode numbers for all of those directories. In case any is
-# a symlink, we want to get the inode of the pointed-to directory, so we
-# append '/.' to be sure we get the target directory. Since the symlinks
-# can be anyway (/bin -> /usr/bin or /usr/bin -> /bin), we do that for
-# all of them.
-#
-SKELETON_CUSTOM_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/lib/. 2>/dev/null)
-SKELETON_CUSTOM_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/bin/. 2>/dev/null)
-SKELETON_CUSTOM_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/sbin/. 2>/dev/null)
-SKELETON_CUSTOM_USR_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/lib/. 2>/dev/null)
-SKELETON_CUSTOM_USR_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/bin/. 2>/dev/null)
-SKELETON_CUSTOM_USR_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/sbin/. 2>/dev/null)
-
 # 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)
 
-ifneq ($(SKELETON_CUSTOM_LIB_INODE),$(SKELETON_CUSTOM_USR_LIB_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR_DIRS += /lib
-endif
-ifneq ($(SKELETON_CUSTOM_BIN_INODE),$(SKELETON_CUSTOM_USR_BIN_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR_DIRS += /bin
-endif
-ifneq ($(SKELETON_CUSTOM_SBIN_INODE),$(SKELETON_CUSTOM_USR_SBIN_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR_DIRS += /sbin
-endif
+SKELETON_CUSTOM_NOT_MERGED_USR_DIRS = $(shell support/scripts/check-merged-usr.sh $(SKELETON_CUSTOM_PATH))
 
 endif # merged /usr
 
diff --git a/support/scripts/check-merged-usr.sh b/support/scripts/check-merged-usr.sh
new file mode 100755
index 0000000000..74c43c89fd
--- /dev/null
+++ b/support/scripts/check-merged-usr.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+#
+# Check if a given custom skeleton or overlay complies to the merged /usr
+# requirements:
+# /
+# /bin -> usr/bin
+# /lib -> usr/lib
+# /sbin -> usr/sbin
+# /usr/bin/
+# /usr/lib/
+# /usr/sbin/
+#
+# Output: the list non-compliant paths (empty if compliant).
+#
+
+# Extract the inode numbers for all of those directories. In case any is
+# a symlink, we want to get the inode of the pointed-to directory, so we
+# append '/.' to be sure we get the target directory. Since the symlinks
+# can be anyway (/bin -> /usr/bin or /usr/bin -> /bin), we do that for
+# all of them.
+#
+lib_inode=$(stat -c '%i' "${1}/lib/." 2>/dev/null)
+bin_inode=$(stat -c '%i' "${1}/bin/." 2>/dev/null)
+sbin_inode=$(stat -c '%i' "${1}/sbin/." 2>/dev/null)
+usr_lib_inode=$(stat -c '%i' "${1}/usr/lib/." 2>/dev/null)
+usr_bin_inode=$(stat -c '%i' "${1}/usr/bin/." 2>/dev/null)
+usr_sbin_inode=$(stat -c '%i' "${1}/usr/sbin/." 2>/dev/null)
+
+not_merged_dirs=""
+test -z "$lib_inode" || \
+	test "$lib_inode" = "$usr_lib_inode" || \
+		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"
+echo "${not_merged_dirs# }"
-- 
2.14.3

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

* [Buildroot] [PATCH v2 2/4] skeleton-custom: install /bin, /lib, and /sbin
  2018-05-07 12:46 [Buildroot] [PATCH v2 0/4] Improve verification of custom rootfs skeletons and overlays Carlos Santos
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 1/4] skeleton-custom: use a script to check merged usr structure Carlos Santos
@ 2018-05-07 12:46 ` Carlos Santos
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled Carlos Santos
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 4/4] system: allow selecting merged /usr along with custom rootfs skeleton Carlos Santos
  3 siblings, 0 replies; 10+ messages in thread
From: Carlos Santos @ 2018-05-07 12:46 UTC (permalink / raw)
  To: buildroot

skeleton-custom does not install the required /bin, /lib and /sbin
directories (or symlinks), which may result in an imcomplete tree, The
user could add the required directories/symlinks to the skeleton but
they may be invalid, depending on the state of BR2_ROOTFS_MERGED_USR.

Steps to reproduce:

- Enable BR2_ROOTFS_MERGED_USR and BR2_INIT_SYSTEMD
- Set BR2_ROOTFS_SKELETON_CUSTOM_PATH to "system/skeleton"
- Run "make skeleton"
- target/{bin.lib,sbin} will not exist

Add calls to SYSTEM_USR_SYMLINKS_OR_DIRS to INSTALL_TARGET_CMDS and
INSTALL_STAGING_CMDS, so the required directories or symlinks are
created.

Add a paragraph to the documentation clarifying that custom skeletons
don't need to contain /bin, /lib or /sbin and must not contain them when
BR2_ROOTFS_MERGED_USR is enabled.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v1->v2:

- Rebase series to HEAD of master branch
- Rework commit message and documentation, as suggested by Thomas
  Petazzoni
---
 docs/manual/customize-rootfs.txt           | 9 +++++++++
 package/skeleton-custom/skeleton-custom.mk | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/docs/manual/customize-rootfs.txt b/docs/manual/customize-rootfs.txt
index 44fc460670..9d3a62ddaf 100644
--- a/docs/manual/customize-rootfs.txt
+++ b/docs/manual/customize-rootfs.txt
@@ -100,6 +100,15 @@ To enable this feature, enable config option
   +System configuration+ menu. If you specify a relative path, it will
   be relative to the root of the Buildroot tree.
 +
+Custom skeletons don't need to contain the '/bin', '/lib' or '/sbin'
+  directories, since they are created automatically during the build.
+  When +BR2_ROOTFS_MERGED_USR+ is enabled, then the custom skeleton must
+  not contain the '/bin', '/lib' or '/sbin' directories, as Buildroot
+  will create them as symbolic links to the relevant folders in '/usr'.
+  In such a situation, should the skeleton have any programs or
+  libraries, they should be placed in '/usr/bin', '/usr/sbin' and
+  '/usr/lib'.
++
 This method is not recommended because it duplicates the entire
   skeleton, which prevents taking advantage of the fixes or improvements
   brought to the default skeleton in later Buildroot releases.
diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
index b1cddd9146..01cd62794d 100644
--- a/package/skeleton-custom/skeleton-custom.mk
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -43,6 +43,7 @@ endif
 # things we customise in the custom skeleton.
 define SKELETON_CUSTOM_INSTALL_TARGET_CMDS
 	$(call SYSTEM_RSYNC,$(SKELETON_CUSTOM_PATH),$(TARGET_DIR))
+	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(TARGET_DIR))
 	$(call SYSTEM_LIB_SYMLINK,$(TARGET_DIR))
 	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
 		$(TARGET_DIR_WARNING_FILE)
@@ -54,6 +55,7 @@ endef
 # skeleton to staging.
 define SKELETON_CUSTOM_INSTALL_STAGING_CMDS
 	$(call SYSTEM_RSYNC,$(SKELETON_CUSTOM_PATH),$(STAGING_DIR))
+	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(STAGING_DIR))
 	$(call SYSTEM_LIB_SYMLINK,$(STAGING_DIR))
 endef
 
-- 
2.14.3

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

* [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled
  2018-05-07 12:46 [Buildroot] [PATCH v2 0/4] Improve verification of custom rootfs skeletons and overlays Carlos Santos
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 1/4] skeleton-custom: use a script to check merged usr structure Carlos Santos
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 2/4] skeleton-custom: install /bin, /lib, and /sbin Carlos Santos
@ 2018-05-07 12:46 ` Carlos Santos
  2018-05-07 12:54   ` Thomas Petazzoni
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 4/4] system: allow selecting merged /usr along with custom rootfs skeleton Carlos Santos
  3 siblings, 1 reply; 10+ messages in thread
From: Carlos Santos @ 2018-05-07 12:46 UTC (permalink / raw)
  To: buildroot

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.

Add a paragraph to the documentation clarifying that rootfs overlays
don't need to contain /bin, /lib or /sbin and must not contain them when
BR2_ROOTFS_MERGED_USR is enabled.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v1->v2:

- Rebase series to HEAD of master branch
- Rework commit message and documentation, as suggested by Thomas
  Petazzoni
---
 Makefile                         | 20 +++++++++++++++++---
 docs/manual/customize-rootfs.txt |  8 ++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index c024c65f78..dc51269143 100644
--- a/Makefile
+++ b/Makefile
@@ -746,11 +746,25 @@ 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)"); \
+		not_merged_dirs="$$(support/scripts/check-merged-usr.sh $(d))"; \
+		test -n "$$not_merged_dirs" && { \
+			echo "ERROR: The overlay in $(d) is not" \
+				"using a merged /usr for the following directories:" \
+				$$not_merged_dirs; \
+			exit 1; \
+		} || true$(sep))
+
+endif # merged /usr
+
 	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
 		$(call MESSAGE,"Copying overlay $(d)"); \
-		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
-			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
-			$(d)/ $(TARGET_DIR)$(sep))
+		$(call SYSTEM_RSYNC,$(d),$(TARGET_DIR))$(sep))
 
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
 		$(call MESSAGE,"Executing post-build script $(s)"); \
diff --git a/docs/manual/customize-rootfs.txt b/docs/manual/customize-rootfs.txt
index 9d3a62ddaf..bb6d8da6bf 100644
--- a/docs/manual/customize-rootfs.txt
+++ b/docs/manual/customize-rootfs.txt
@@ -22,6 +22,14 @@ A filesystem overlay is a tree of files that is copied directly
   etc., files called +.empty+ and files ending in +~+ are excluded from
   the copy.
 +
+Filesystem overlays don't need to contain the '/bin', '/lib' or '/sbin'
+  directories, since they are created automatically during the build.
+  When +BR2_ROOTFS_MERGED_USR+ is enabled, then the overlay must not
+  contain the '/bin', '/lib' or '/sbin' directories, as Buildroot will
+  create them as symbolic links to the relevant folders in '/usr'.  In
+  such a situation, should the overlay have any programs or libraries,
+  they should be placed in '/usr/bin', '/usr/sbin' and '/usr/lib'.
++
 As shown in xref:customize-dir-structure[], the recommended path for
   this overlay is +board/<company>/<boardname>/rootfs-overlay+.
 
-- 
2.14.3

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

* [Buildroot] [PATCH v2 4/4] system: allow selecting merged /usr along with custom rootfs skeleton
  2018-05-07 12:46 [Buildroot] [PATCH v2 0/4] Improve verification of custom rootfs skeletons and overlays Carlos Santos
                   ` (2 preceding siblings ...)
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled Carlos Santos
@ 2018-05-07 12:46 ` Carlos Santos
  3 siblings, 0 replies; 10+ messages in thread
From: Carlos Santos @ 2018-05-07 12:46 UTC (permalink / raw)
  To: buildroot

If the user is brave enough to use a custom rootfs skeleton then we must
not prevent using merged /usr too. Actually it is already possible to do
this, although indirectly, by selecting BR2_INIT_SYSTEMD.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v1->v2:

- Rebase series to HEAD of master branch
---
 system/Config.in | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/system/Config.in b/system/Config.in
index d14a864ca5..911cd424ba 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -31,10 +31,6 @@ config BR2_ROOTFS_SKELETON_CUSTOM_PATH
 	help
 	  Path to custom target skeleton.
 
-# dummy config so merged /usr workarounds can also be activated for
-# custom rootfs skeleton
-config BR2_ROOTFS_MERGED_USR
-
 endif
 
 if BR2_ROOTFS_SKELETON_DEFAULT
@@ -209,8 +205,6 @@ config BR2_ROOTFS_DEVICE_TABLE_SUPPORTS_EXTENDED_ATTRIBUTES
 	help
 	  Support extended attributes handling in device tables
 
-if BR2_ROOTFS_SKELETON_DEFAULT
-
 config BR2_ROOTFS_MERGED_USR
 	bool "Use symlinks to /usr for /bin, /sbin and /lib"
 	help
@@ -223,6 +217,8 @@ config BR2_ROOTFS_MERGED_USR
 	  symlinks to their counterparts in /usr. In this case, /usr can
 	  not be a separate filesystem.
 
+if BR2_ROOTFS_SKELETON_DEFAULT
+
 config BR2_TARGET_ENABLE_ROOT_LOGIN
 	bool "Enable root login with password"
 	default y
-- 
2.14.3

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

* [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled
  2018-05-07 12:46 ` [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled Carlos Santos
@ 2018-05-07 12:54   ` Thomas Petazzoni
  2018-05-07 13:05     ` Carlos Santos
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Petazzoni @ 2018-05-07 12:54 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon,  7 May 2018 09:46:39 -0300, Carlos Santos wrote:
> 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.
> 
> Add a paragraph to the documentation clarifying that rootfs overlays
> don't need to contain /bin, /lib or /sbin and must not contain them when
> BR2_ROOTFS_MERGED_USR is enabled.
> 
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
> Changes v1->v2:
> 
> - Rebase series to HEAD of master branch
> - Rework commit message and documentation, as suggested by Thomas
>   Petazzoni
> ---
>  Makefile                         | 20 +++++++++++++++++---
>  docs/manual/customize-rootfs.txt |  8 ++++++++
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c024c65f78..dc51269143 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -746,11 +746,25 @@ 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)"); \
> +		not_merged_dirs="$$(support/scripts/check-merged-usr.sh $(d))"; \
> +		test -n "$$not_merged_dirs" && { \
> +			echo "ERROR: The overlay in $(d) is not" \
> +				"using a merged /usr for the following directories:" \
> +				$$not_merged_dirs; \
> +			exit 1; \
> +		} || true$(sep))
> +
> +endif # merged /usr
> +
>  	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
>  		$(call MESSAGE,"Copying overlay $(d)"); \
> -		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
> -			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
> -			$(d)/ $(TARGET_DIR)$(sep))
> +		$(call SYSTEM_RSYNC,$(d),$(TARGET_DIR))$(sep))

This specific chunk is not directly related to checking that overlays
comply with the merged /usr rule, so shouldn't this be in a separate
commit ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

* [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled
  2018-05-07 12:54   ` Thomas Petazzoni
@ 2018-05-07 13:05     ` Carlos Santos
  2018-05-07 13:10       ` Thomas Petazzoni
  0 siblings, 1 reply; 10+ messages in thread
From: Carlos Santos @ 2018-05-07 13:05 UTC (permalink / raw)
  To: buildroot

> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "buildroot" <buildroot@buildroot.org>, "Yann Morin" <yann.morin.1998@free.fr>, "Thomas De Schampheleire"
> <thomas.de_schampheleire@nokia.com>
> Sent: Monday, May 7, 2018 9:54:43 AM
> Subject: Re: [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

> Hello,
> 
> On Mon,  7 May 2018 09:46:39 -0300, Carlos Santos wrote:
>> 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.
>> 
>> Add a paragraph to the documentation clarifying that rootfs overlays
>> don't need to contain /bin, /lib or /sbin and must not contain them when
>> BR2_ROOTFS_MERGED_USR is enabled.
>> 
>> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
>> ---
>> Changes v1->v2:
>> 
>> - Rebase series to HEAD of master branch
>> - Rework commit message and documentation, as suggested by Thomas
>>   Petazzoni
>> ---
>>  Makefile                         | 20 +++++++++++++++++---
>>  docs/manual/customize-rootfs.txt |  8 ++++++++
>>  2 files changed, 25 insertions(+), 3 deletions(-)
>> 
>> diff --git a/Makefile b/Makefile
>> index c024c65f78..dc51269143 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -746,11 +746,25 @@ 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)"); \
>> +		not_merged_dirs="$$(support/scripts/check-merged-usr.sh $(d))"; \
>> +		test -n "$$not_merged_dirs" && { \
>> +			echo "ERROR: The overlay in $(d) is not" \
>> +				"using a merged /usr for the following directories:" \
>> +				$$not_merged_dirs; \
>> +			exit 1; \
>> +		} || true$(sep))
>> +
>> +endif # merged /usr
>> +
>>  	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
>>  		$(call MESSAGE,"Copying overlay $(d)"); \
>> -		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
>> -			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
>> -			$(d)/ $(TARGET_DIR)$(sep))
>> +		$(call SYSTEM_RSYNC,$(d),$(TARGET_DIR))$(sep))
> 
> This specific chunk is not directly related to checking that overlays
> comply with the merged /usr rule, so shouldn't this be in a separate
> commit ?

It removes ?--keep-dirlinks?, so becomes identical to the SYSTEM_RSYNC
logic, as observed by Peter Korsgaard in a previous message.

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens

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

* [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled
  2018-05-07 13:05     ` Carlos Santos
@ 2018-05-07 13:10       ` Thomas Petazzoni
  2018-05-07 13:28         ` Carlos Santos
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Petazzoni @ 2018-05-07 13:10 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, 7 May 2018 10:05:46 -0300 (BRT), Carlos Santos wrote:

> >>  	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
> >>  		$(call MESSAGE,"Copying overlay $(d)"); \
> >> -		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
> >> -			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
> >> -			$(d)/ $(TARGET_DIR)$(sep))
> >> +		$(call SYSTEM_RSYNC,$(d),$(TARGET_DIR))$(sep))  
> > 
> > This specific chunk is not directly related to checking that overlays
> > comply with the merged /usr rule, so shouldn't this be in a separate
> > commit ?  
> 
> It removes ?--keep-dirlinks?, so becomes identical to the SYSTEM_RSYNC
> logic, as observed by Peter Korsgaard in a previous message.

Agreed, but I don't see what it has to do with "check rootfs overlays
with BR2_ROOTFS_MERGED_USR enabled".

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

* [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled
  2018-05-07 13:10       ` Thomas Petazzoni
@ 2018-05-07 13:28         ` Carlos Santos
  2018-05-07 13:51           ` Thomas Petazzoni
  0 siblings, 1 reply; 10+ messages in thread
From: Carlos Santos @ 2018-05-07 13:28 UTC (permalink / raw)
  To: buildroot

> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "buildroot" <buildroot@buildroot.org>, "Yann Morin" <yann.morin.1998@free.fr>, "Thomas De Schampheleire"
> <thomas.de_schampheleire@nokia.com>, "Peter Korsgaard" <peter@korsgaard.com>
> Sent: Monday, May 7, 2018 10:10:02 AM
> Subject: Re: [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

> Hello,
> 
> On Mon, 7 May 2018 10:05:46 -0300 (BRT), Carlos Santos wrote:
> 
>> >>  	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
>> >>  		$(call MESSAGE,"Copying overlay $(d)"); \
>> >> -		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
>> >> -			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
>> >> -			$(d)/ $(TARGET_DIR)$(sep))
>> >> +		$(call SYSTEM_RSYNC,$(d),$(TARGET_DIR))$(sep))
>> > 
>> > This specific chunk is not directly related to checking that overlays
>> > comply with the merged /usr rule, so shouldn't this be in a separate
>> > commit ?
>> 
>> It removes ?--keep-dirlinks?, so becomes identical to the SYSTEM_RSYNC
>> logic, as observed by Peter Korsgaard in a previous message.
> 
> Agreed, but I don't see what it has to do with "check rootfs overlays
> with BR2_ROOTFS_MERGED_USR enabled".

Hum, well, agree.

[grumpy mode on]

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens

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

* [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled
  2018-05-07 13:28         ` Carlos Santos
@ 2018-05-07 13:51           ` Thomas Petazzoni
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Petazzoni @ 2018-05-07 13:51 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, 7 May 2018 10:28:38 -0300 (BRT), Carlos Santos wrote:

> > Agreed, but I don't see what it has to do with "check rootfs overlays
> > with BR2_ROOTFS_MERGED_USR enabled".  
> 
> Hum, well, agree.
> 
> [grumpy mode on]

Sorry about the nitpicking. It's not so much about being picky, but
mainly to make sure I (and perhaps others?) understand correctly the
changes you've made. I'm sorry if that makes you grumpy, it's
definitely not my intention. I do appreciate a lot the contributions
you make and their quality.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

end of thread, other threads:[~2018-05-07 13:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-07 12:46 [Buildroot] [PATCH v2 0/4] Improve verification of custom rootfs skeletons and overlays Carlos Santos
2018-05-07 12:46 ` [Buildroot] [PATCH v2 1/4] skeleton-custom: use a script to check merged usr structure Carlos Santos
2018-05-07 12:46 ` [Buildroot] [PATCH v2 2/4] skeleton-custom: install /bin, /lib, and /sbin Carlos Santos
2018-05-07 12:46 ` [Buildroot] [PATCH v2 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled Carlos Santos
2018-05-07 12:54   ` Thomas Petazzoni
2018-05-07 13:05     ` Carlos Santos
2018-05-07 13:10       ` Thomas Petazzoni
2018-05-07 13:28         ` Carlos Santos
2018-05-07 13:51           ` Thomas Petazzoni
2018-05-07 12:46 ` [Buildroot] [PATCH v2 4/4] system: allow selecting merged /usr along with custom rootfs skeleton Carlos Santos

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