All of 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 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.