Skip to contentSkip to conversation

target permissions

PATCH Superseded Guido Martínez 5 participants 26 messages Nov 21, 2014 – Jul 13, 2015 Kbuild

lore
Share & tools
lore
#0 lore raw

v4 cover target permissions

Hey guys,

fourth patchset to fix the target permissions variability.

I checked (to the best of my knowledge) every 'cp' and 'rsync' call under package/. There are two bigs cases:

(1) Copies from the package or download directory to the build directory (example: cache-calibrator, fan-ctrl, ramspeed, samba4). They are OK, since they copy source code, a Makefile or some other build files. These files _shouldn't_ affect the result, so I left them the way they are. We can change them if you agree, but I thought it'd be too much noise for now.

(2) Copies from anything under output/ to the target. These should all be fine. Since we now build with a controlled umask and reset any permissions that could vary (tar files don't vary), we shouldn't observe any variability under output/ (modulo (1)-kind copies).

I tested parallel builds with this, as Yann suggested, and they seem to work the same, which is expected since we just wrap everything at the top level.

Differences from v3 are:

- Recursive running of make is done with $(MAKE), and not using --no-print-directory. Also, change the "%" rule in "$(MAKECMDGOALS)" to properly detect phony rules.

- Use --chmod on the external toolchain's rsync, to prevent any weird permissions coming from there. This isn't stricly necessary, since the toolchain libs are copied to the target via 'install', which won't depend on any existing permissions. But this way we're safer in case anything changes in the future. Also we make sure everything under output/ is controlled.

- Fix some cp's to the target from non-controlled dirs on boot/grub/grub.mk, fs/iso9660/iso9660.mk, and board/boundarydevices/nitrogen6x/post-build.sh

- Fifth patch dropped as it was accepted.

- Added the umask setting in the Makefile generated in the $(O) dir. This is just an optimization so we don't have another level of 'make' running.

- Fixed 'install' call on matchbox-keyboard patch (#4)

Thanks!

---

Guido Mart?nez (9):
Makefile: don't depend on the umask
Makefile: don't depend on current skeleton/overlay permissions toolchain/helpers.mk: use --chmod on rsync pkg-generic.mk: don't depend on external package permissions
package: matchbox-keyboard: use install instead of cp
scripts: mkmakefile: set umask before calling BR's makefile
nitrogen6x: use install instead of cp
boot/grub: use install instead of cp
fs/iso9660: use install instead of cp

 Makefile                                              | 19 +++++++++++++++++--
 board/boundarydevices/nitrogen6x/post-build.sh        |  6 +++---
 boot/grub/grub.mk                                     | 14 +++++++-------
 fs/iso9660/iso9660.mk                                 | 16 +++++++++-------
 .../matchbox/matchbox-keyboard/matchbox-keyboard.mk   |  2 +-
 package/pkg-generic.mk                                |  2 +-
 support/scripts/mkmakefile                            |  2 +-
 toolchain/helpers.mk                                  |  2 +-
 8 files changed, 40 insertions(+), 23 deletions(-)
Signature · 1 line
2.1.3
#1 lore raw

PATCH v4 1/9 Makefile: don't depend on the umask

Some packages and BR itself create files and directories on the target with cp/mkdir/etc which depend on the umask at the time of building.

To fix this, use a trick inside the Makefile which wraps all rules when the umask is not 0022. This sets the umask at the top level, and then the building process continues as usual.

---
 Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
Makefile +15 -0
index 3d8283b..eeb1412 100644
@@ -24,6 +24,19 @@
 # You shouldn't need to mess with anything beyond this point...
 #--------------------------------------------------------------
 
+# Trick for always running with a fixed umask
+UMASK=0022
+ifneq ($(shell umask),$(UMASK))
+.PHONY: all $(MAKECMDGOALS)
+
+all:
+	@umask $(UMASK) && $(MAKE)
+
+$(MAKECMDGOALS):
+	@umask $(UMASK) && $(MAKE) $@
+
+else # umask
+
 # This is our default rule, so must come first
 all:
 
@@ -932,3 +945,5 @@ include docs/manual/manual.mk
 -include $(BR2_EXTERNAL)/docs/*/*.mk
 
 .PHONY: $(noconfig_targets)
+
+endif #umask
Signature · 1 line
2.1.3
#2 lore raw

PATCH v4 1/9 Makefile: don't depend on the umask

Quoted 8 lines · Guido Mart?nez

Some packages and BR itself create files and directories on the target with cp/mkdir/etc which depend on the umask at the time of building.

To fix this, use a trick inside the Makefile which wraps all rules when the umask is not 0022. This sets the umask at the top level, and then the building process continues as usual.

Without this patch, building with umask 077 yields the following in rootfs.tar:

-rw------- 0/0 122 2015-07-13 11:28 ./etc/os-release

After the patch, we get:

-rw-r--r-- 0/0 122 2015-07-13 11:31 ./etc/os-release

Also verified that it has virtually no impact on the time of 'make -qp'.

Regards,
Arnout

Quoted 35 lines
---
 Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
Makefile +15 -0
index 3d8283b..eeb1412 100644
@@ -24,6 +24,19 @@
 # You shouldn't need to mess with anything beyond this point...
 #--------------------------------------------------------------
 
+# Trick for always running with a fixed umask
+UMASK=0022
+ifneq ($(shell umask),$(UMASK))
+.PHONY: all $(MAKECMDGOALS)
+
+all:
+	@umask $(UMASK) && $(MAKE)
+
+$(MAKECMDGOALS):
+	@umask $(UMASK) && $(MAKE) $@
+
+else # umask
+
 # This is our default rule, so must come first
 all:
 
@@ -932,3 +945,5 @@ include docs/manual/manual.mk
 -include $(BR2_EXTERNAL)/docs/*/*.mk
 
 .PHONY: $(noconfig_targets)
+
+endif #umask
Signature · 6 lines
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
#3 lore raw

PATCH v4 1/9 Makefile: don't depend on the umask

In reply to Guido Martínez

Dear Guido Mart?nez,

Quoted 11 lines · Guido Mart?nez

Some packages and BR itself create files and directories on the target with cp/mkdir/etc which depend on the umask at the time of building.

To fix this, use a trick inside the Makefile which wraps all rules when the umask is not 0022. This sets the umask at the top level, and then the building process continues as usual.

---
 Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Applied with some additional changes:

    [Thomas: add --no-print-directory, as suggested by Arnout.]

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH v4 2/9 Makefile: don't depend on current skeleton/overlay permissions

In reply to Guido Martínez

We use 'rsync -a' to copy the skeleton and overlays, so the target ends up with the exact same permissions as on the repo. The problem is we don't track these permissions, since Git doesn't allow for that (except for the exec bit). This means users with different umasks at the time of cloning could end up with different target permissions.

Fix this by using --chmod on rsync calls so we don't depend on the current permission set for the skeleton and overlays. We do depend on the exec bit, but that's fine since that one is tracked by Git.

---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Makefile +2 -2
index eeb1412..5880bbb 100644
@@ -489,7 +489,7 @@ RSYNC_VCS_EXCLUSIONS = \
 $(BUILD_DIR)/.root:
 	mkdir -p $(TARGET_DIR)
 	rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
-		--chmod=Du+w --exclude .empty --exclude '*~' \
+		--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
 		$(TARGET_SKELETON)/ $(TARGET_DIR)/
 	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $(TARGET_DIR_WARNING_FILE)
 	@ln -snf lib $(TARGET_DIR)/$(LIB_SYMLINK)
@@ -625,7 +625,7 @@ endif
 	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
 		$(call MESSAGE,"Copying overlay $(d)"); \
 		rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
-			--chmod=Du+w --exclude .empty --exclude '*~' \
+			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
 			$(d)/ $(TARGET_DIR)$(sep))
 
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
Signature · 1 line
2.1.3
#5 lore raw

PATCH v4 2/9 Makefile: don't depend on current skeleton/overlay permissions

Quoted 11 lines · Guido Mart?nez

We use 'rsync -a' to copy the skeleton and overlays, so the target ends up with the exact same permissions as on the repo. The problem is we don't track these permissions, since Git doesn't allow for that (except for the exec bit). This means users with different umasks at the time of cloning could end up with different target permissions.

Fix this by using --chmod on rsync calls so we don't depend on the current permission set for the skeleton and overlays. We do depend on the exec bit, but that's fine since that one is tracked by Git.

It may break existings setup that rely on these permissions to be kept, but
since that's anyway not a good idea, let's keep it this way. However, I'll follow up with a release not that explains the issue.

Regards,
Arnout

Quoted 27 lines
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Makefile +2 -2
index eeb1412..5880bbb 100644
@@ -489,7 +489,7 @@ RSYNC_VCS_EXCLUSIONS = \
 $(BUILD_DIR)/.root:
 	mkdir -p $(TARGET_DIR)
 	rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
-		--chmod=Du+w --exclude .empty --exclude '*~' \
+		--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
 		$(TARGET_SKELETON)/ $(TARGET_DIR)/
 	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $(TARGET_DIR_WARNING_FILE)
 	@ln -snf lib $(TARGET_DIR)/$(LIB_SYMLINK)
@@ -625,7 +625,7 @@ endif
 	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
 		$(call MESSAGE,"Copying overlay $(d)"); \
 		rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
-			--chmod=Du+w --exclude .empty --exclude '*~' \
+			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
 			$(d)/ $(TARGET_DIR)$(sep))
 
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
Signature · 6 lines
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
#6 lore raw

PATCH 1/1 CHANGES: add release note for the permissions

In reply to Guido Martínez

Since the rsync of skeleton or rootfs now changes the permissions of the copied files and directories, this may break existing setups that rely on these permissions being kept. So add a release not explaining how it should be done.

---
 CHANGES | 11 +++++++++++
 1 file changed, 11 insertions(+)
CHANGES +11 -0
index 3f5162e..1e94929 100644
@@ -1,3 +1,14 @@
+2015.08-rc1, not yet released
+
+	Fixes all over the tree and new features.
+
+	Infrastructure: Buildroot takes better care now of generating
+	predictable permissions in the target filesystem. However,
+	existing permissions on a custom skeleton or rootfs overlay
+	will no longer be preserved. Therefore, it is necessary to
+	add a permission table (BR2_ROOTFS_DEVICE_TABLE) to set
+	the required permissions.
+
 2015.05, Released May 31st, 2015
 
 	Minor fixes.
Signature · 1 line
2.1.4
#7 lore raw

PATCH 1/1 CHANGES: add release note for the permissions

Hi Arnout,

Quoted 3 lines · Arnout Vandecappelle (Essensium/Mind)

Since the rsync of skeleton or rootfs now changes the permissions of the copied files and directories, this may break existing setups that rely on these permissions being kept. So add a release not explaining

note

Quoted 1 line

how it should be done.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#8 lore raw

PATCH 1/1 CHANGES: add release note for the permissions

In reply to Arnout Vandecappelle (Essensium/Mind)

Dear Arnout Vandecappelle (Essensium/Mind),

Quoted 9 lines · Arnout Vandecappelle (Essensium/Mind)

Since the rsync of skeleton or rootfs now changes the permissions of the copied files and directories, this may break existing setups that rely on these permissions being kept. So add a release not explaining how it should be done.

---
 CHANGES | 11 +++++++++++
 1 file changed, 11 insertions(+)

I've fixed the commit log as suggested by Baruch and applied, thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#9 lore raw

PATCH v4 2/9 Makefile: don't depend on current skeleton/overlay permissions

In reply to Guido Martínez

Dear Guido Mart?nez,

Quoted 6 lines · Guido Mart?nez

We use 'rsync -a' to copy the skeleton and overlays, so the target ends up with the exact same permissions as on the repo. The problem is we don't track these permissions, since Git doesn't allow for that (except for the exec bit). This means users with different umasks at the time of cloning could end up with different target permissions.

Quoted 8 lines

Fix this by using --chmod on rsync calls so we don't depend on the current permission set for the skeleton and overlays. We do depend on the exec bit, but that's fine since that one is tracked by Git.

---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#10 lore raw

PATCH v4 3/9 toolchain/helpers.mk: use --chmod on rsync

In reply to Guido Martínez

This makes sure we don't have any weird permissions on the staging dir, which could affect the target.

---
 toolchain/helpers.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
toolchain/helpers.mk +1 -1
index 3121da4..46f5f22 100644
@@ -139,7 +139,7 @@ copy_toolchain_sysroot = \
 	SUPPORT_LIB_DIR="$(strip $5)" ; \
 	for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
 		if [ -d $${ARCH_SYSROOT_DIR}/$$i ] ; then \
-			rsync -au --chmod=Du+w --exclude 'usr/lib/locale' \
+			rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
 				--exclude lib --exclude lib32 --exclude lib64 \
 				$${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
 		fi ; \
Signature · 1 line
2.1.3
#11 lore raw

PATCH v4 3/9 toolchain/helpers.mk: use --chmod on rsync

Quoted 4 lines · Guido Mart?nez

This makes sure we don't have any weird permissions on the staging dir, which could affect the target.

Regards,
Arnout

Quoted 18 lines
---
 toolchain/helpers.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
toolchain/helpers.mk +1 -1
index 3121da4..46f5f22 100644
@@ -139,7 +139,7 @@ copy_toolchain_sysroot = \
 	SUPPORT_LIB_DIR="$(strip $5)" ; \
 	for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
 		if [ -d $${ARCH_SYSROOT_DIR}/$$i ] ; then \
-			rsync -au --chmod=Du+w --exclude 'usr/lib/locale' \
+			rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
 				--exclude lib --exclude lib32 --exclude lib64 \
 				$${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
 		fi ; \
Signature · 6 lines
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
#12 lore raw

PATCH v4 3/9 toolchain/helpers.mk: use --chmod on rsync

In reply to Guido Martínez

Dear Guido Mart?nez,

Quoted 7 lines · Guido Mart?nez

This makes sure we don't have any weird permissions on the staging dir, which could affect the target.

---
 toolchain/helpers.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#13 lore raw

PATCH v4 4/9 pkg-generic.mk: don't depend on external package permissions

In reply to Guido Martínez

Reset permissions for rsynced packages (when using OVERRIDE_SRCDIR) to 755/644. We do this under the assumption that source files shouldn't care about their permissions, except possibly for the exec bit.

This guarantees that if a package uses 'rsync -a' or 'cp -p' to copy a file from its build dir to the target, it'll end up with the same permissions on the target every time.

---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/pkg-generic.mk +1 -1
index 9643a30..ec2989f 100644
@@ -117,7 +117,7 @@ $(BUILD_DIR)/%/.stamp_rsynced:
 	@$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
 	@test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
 	$(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep))
-	rsync -au $(RSYNC_VCS_EXCLUSIONS) $(SRCDIR)/ $(@D)
+	rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $(SRCDIR)/ $(@D)
 	$(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
 
Signature · 1 line
2.1.3
#14 lore raw

PATCH v4 4/9 pkg-generic.mk: don't depend on external package permissions

Dear Guido Mart?nez,

Quoted 4 lines · Guido Mart?nez

Reset permissions for rsynced packages (when using OVERRIDE_SRCDIR) to 755/644. We do this under the assumption that source files shouldn't care about their permissions, except possibly for the exec bit.

Quoted 8 lines

This guarantees that if a package uses 'rsync -a' or 'cp -p' to copy a file from its build dir to the target, it'll end up with the same permissions on the target every time.

---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#15 lore raw

PATCH v4 5/9 package: matchbox-keyboard: use install instead of cp

In reply to Guido Martínez

in order to not depend on the previous permissions of the file

---
 package/matchbox/matchbox-keyboard/matchbox-keyboard.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/matchbox/matchbox-keyboard/matchbox-keyboard.mk +1 -1
index ebf23e4..bdcc32f 100644
@@ -15,7 +15,7 @@ MATCHBOX_KEYBOARD_DEPENDENCIES = host-pkgconf matchbox-lib matchbox-fakekey expa
 MATCHBOX_KEYBOARD_CONF_ENV = expat=yes
 
 define MATCHBOX_KEYBOARD_POST_INSTALL_FIXES
-	cp -dpf ./package/matchbox/matchbox-keyboard/mb-applet-kbd-wrapper.sh $(TARGET_DIR)/usr/bin/
+	$(INSTALL) -D -m 0755 package/matchbox/matchbox-keyboard/mb-applet-kbd-wrapper.sh $(TARGET_DIR)/usr/bin/mb-applet-kbd-wrapper.sh
 endef
 
 MATCHBOX_KEYBOARD_POST_INSTALL_TARGET_HOOKS += MATCHBOX_KEYBOARD_POST_INSTALL_FIXES
Signature · 1 line
2.1.3
#16 lore raw

PATCH v4 5/9 package: matchbox-keyboard: use install instead of cp

Dear Guido Mart?nez,

Quoted 6 lines · Guido Mart?nez

in order to not depend on the previous permissions of the file

---
 package/matchbox/matchbox-keyboard/matchbox-keyboard.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch applied to the next branch, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#17 lore raw

PATCH v4 6/9 scripts: mkmakefile: set umask before calling BR's makefile

In reply to Guido Martínez

Small optimization so we don't have another 'make' level (caused by the umask fix) when running the generated makefile.

---
 support/scripts/mkmakefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
support/scripts/mkmakefile +1 -1
index 27b1507..974e2c0 100755
@@ -35,7 +35,7 @@ MAKEFLAGS += --no-print-directory
 all	:= \$(filter-out Makefile,\$(MAKECMDGOALS))
 
 _all:
-	\$(MAKE) \$(MAKEARGS) \$(all)
+	umask 0022 && \$(MAKE) \$(MAKEARGS) \$(all)
 
 Makefile:;
 
Signature · 1 line
2.1.3
#18 lore raw

PATCH v4 6/9 scripts: mkmakefile: set umask before calling BR's makefile

Dear Guido Mart?nez,

Quoted 7 lines · Guido Mart?nez

Small optimization so we don't have another 'make' level (caused by the umask fix) when running the generated makefile.

---
 support/scripts/mkmakefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#19 lore raw

PATCH v4 7/9 nitrogen6x: use install instead of cp

In reply to Guido Martínez

in order to not depend on the previous permissions of each file

---
 board/boundarydevices/nitrogen6x/post-build.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
board/boundarydevices/nitrogen6x/post-build.sh +3 -3
index cc6e66a..2b43aa3 100755
@@ -8,11 +8,11 @@
 BOARD_DIR="$(dirname $0)"
 
 # bd u-boot looks for bootscript here
-cp $BOARD_DIR/6x_bootscript $TARGET_DIR
+install -D -m 0644 $BOARD_DIR/6x_bootscript $TARGET_DIR/6x_bootscript
 
 # u-boot / update script for bd upgradeu command
 if [ -e $BINARIES_DIR/u-boot.imx ];
 then
-    cp $BINARIES_DIR/u-boot.imx $TARGET_DIR
-    cp $BOARD_DIR/6x_upgrade $TARGET_DIR
+    install -D -m 0644 $BINARIES_DIR/u-boot.imx $TARGET_DIR/u-boot.imx
+    install -D -m 0644 $BOARD_DIR/6x_upgrade $TARGET_DIR/6x_upgrade
 fi
Signature · 1 line
2.1.3
#20 lore raw

PATCH v4 8/9 boot/grub: use install instead of cp

In reply to Guido Martínez

This way we don't depend on the permissions of files under boot/grub/, which aren't tracked.

While we're at it, change all 'cp' usages into 'install' with the correct mode for each file.

---
 boot/grub/grub.mk | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
boot/grub/grub.mk +7 -7
index 535ada6..ae687d6 100644
@@ -81,7 +81,7 @@ GRUB_CONF_OPTS = \
 
 ifeq ($(BR2_TARGET_GRUB_SPLASH),y)
 define GRUB_INSTALL_SPLASH
-	cp boot/grub/splash.xpm.gz $(TARGET_DIR)/boot/grub/
+	$(INSTALL) -D -m 0644 boot/grub/splash.xpm.gz $(TARGET_DIR)/boot/grub/splash.xpm.gz
 endef
 else
 define GRUB_INSTALL_SPLASH
@@ -95,14 +95,14 @@ endif
 # of a disk or disk image.
 
 define GRUB_INSTALL_TARGET_CMDS
-	install -m 0755 -D $(@D)/grub/grub $(HOST_DIR)/sbin/grub
-	mkdir -p $(TARGET_DIR)/boot/grub
-	cp $(@D)/stage1/stage1 $(TARGET_DIR)/boot/grub
+	$(INSTALL) -D -m 0755 $(@D)/grub/grub $(HOST_DIR)/sbin/grub
+	$(INSTALL) -D -m 0755 $(@D)/stage1/stage1 $(TARGET_DIR)/boot/grub/stage1
 	for f in $(GRUB_STAGE_1_5_TO_INSTALL) ; do \
-		cp $(@D)/stage2/$${f}_stage1_5 $(TARGET_DIR)/boot/grub ; \
+		$(INSTALL) -D -m 0755 $(@D)/stage2/$${f}_stage1_5 \
+			$(TARGET_DIR)/boot/grub/$${f}_stage1_5 ; \
 	done
-	cp $(@D)/stage2/stage2 $(TARGET_DIR)/boot/grub
-	cp boot/grub/menu.lst $(TARGET_DIR)/boot/grub
+	$(INSTALL) -D -m 0644 $(@D)/stage2/stage2 $(TARGET_DIR)/boot/grub/stage2
+	$(INSTALL) -D -m 0644 boot/grub/menu.lst $(TARGET_DIR)/boot/grub/menu.lst
 	$(GRUB_INSTALL_SPLASH)
 endef
 
Signature · 1 line
2.1.3
#21 lore raw

PATCH v4 9/9 fs/iso9660: use install instead of cp

In reply to Guido Martínez

This way we don't depend on the permissions of files under boot/ or fs/, which aren't tracked.

While we're at it, change all 'cp' usages into 'install' with the correct mode for each file.

---
 fs/iso9660/iso9660.mk | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
fs/iso9660/iso9660.mk +9 -7
index 908f0e9..5b44ba4 100644
@@ -14,20 +14,22 @@ ROOTFS_ISO9660_DEPENDENCIES = grub host-cdrkit host-fakeroot linux rootfs-cpio
 
 $(BINARIES_DIR)/rootfs.iso9660: $(ROOTFS_ISO9660_DEPENDENCIES)
 	@$(call MESSAGE,"Generating root filesystem image rootfs.iso9660")
-	mkdir -p $(ISO9660_TARGET_DIR)
-	mkdir -p $(ISO9660_TARGET_DIR)/boot/grub
-	cp $(GRUB_DIR)/stage2/stage2_eltorito $(ISO9660_TARGET_DIR)/boot/grub/
-	cp $(ISO9660_BOOT_MENU) $(ISO9660_TARGET_DIR)/boot/grub/menu.lst
+	$(INSTALL) -D -m 0644 $(GRUB_DIR)/stage2/stage2_eltorito \
+		$(ISO9660_TARGET_DIR)/boot/grub/stage2_eltorito
+	$(INSTALL) -D -m 0644 $(ISO9660_BOOT_MENU) \
+		$(ISO9660_TARGET_DIR)/boot/grub/menu.lst
 ifeq ($(BR2_TARGET_GRUB_SPLASH),)
 	$(SED) '/^splashimage/d' $(ISO9660_TARGET_DIR)/boot/grub/menu.lst
 else
-	cp boot/grub/splash.xpm.gz $(ISO9660_TARGET_DIR)/
+	$(INSTALL) -D -m 0644 boot/grub/splash.xpm.gz \
+		$(ISO9660_TARGET_DIR)/splash.xpm.gz
 endif
-	cp $(LINUX_IMAGE_PATH) $(ISO9660_TARGET_DIR)/kernel
+	$(INSTALL) -D -m 0644 $(LINUX_IMAGE_PATH) $(ISO9660_TARGET_DIR)/kernel
 ifeq ($(BR2_TARGET_ROOTFS_INITRAMFS),y)
 	$(SED) '/initrd/d'  $(ISO9660_TARGET_DIR)/boot/grub/menu.lst
 else
-	cp $(BINARIES_DIR)/rootfs.cpio$(ROOTFS_CPIO_COMPRESS_EXT) $(ISO9660_TARGET_DIR)/initrd
+	$(INSTALL) -D -m 0644 $(BINARIES_DIR)/rootfs.cpio$(ROOTFS_CPIO_COMPRESS_EXT) \
+		$(ISO9660_TARGET_DIR)/initrd
 endif
 	# Use fakeroot to pretend all target binaries are owned by root
 	rm -f $(FAKEROOT_SCRIPT)
Signature · 1 line
2.1.3
#22 lore raw

v4 cover target permissions

In reply to Guido Martínez

Dear Guido Mart?nez,

Quoted 1 line · Guido Mart?nez

Guido Mart?nez (9):

[...]

Quoted 3 lines

nitrogen6x: use install instead of cp
boot/grub: use install instead of cp
fs/iso9660: use install instead of cp

I've applied those three patches to the next branch. The other ones are more sensitive, so I'll wait to get more feedback/review, but applying those three patches means you'll have less patches to carry in your branch :)

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#23 lore raw

v4 cover target permissions

Quoted 6 lines · Thomas Petazzoni

Dear Guido Mart?nez,

Quoted 1 line · Guido Mart?nez

Guido Mart?nez (9):

[...]

Quoted 8 lines
Quoted 3 lines

nitrogen6x: use install instead of cp
boot/grub: use install instead of cp
fs/iso9660: use install instead of cp

I've applied those three patches to the next branch. The other ones are more sensitive, so I'll wait to get more feedback/review, but applying those three patches means you'll have less patches to carry in your branch :)

Thanks Thomas!

I agree the other require a bit more feedback. Even though I have been using them for a while with no problems, one can't be sure enough when touching this kind of stuff :)

BTW, patch #2 was Reviewed-by Yann on the previous patchset. It's the exact same patch. Same for #4 (was #3), except for a minor change in the commit log.

Signature · 2 lines
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar
#24 lore raw

v4 cover target permissions

In reply to Guido Martínez
Quoted 63 lines · Guido Mart?nez

Hey guys,

fourth patchset to fix the target permissions variability.

I checked (to the best of my knowledge) every 'cp' and 'rsync' call under package/. There are two bigs cases:

(1) Copies from the package or download directory to the build directory (example: cache-calibrator, fan-ctrl, ramspeed, samba4). They are OK, since they copy source code, a Makefile or some other build files. These files _shouldn't_ affect the result, so I left them the way they are. We can change them if you agree, but I thought it'd be too much noise for now.

(2) Copies from anything under output/ to the target. These should all be fine. Since we now build with a controlled umask and reset any permissions that could vary (tar files don't vary), we shouldn't observe any variability under output/ (modulo (1)-kind copies).

I tested parallel builds with this, as Yann suggested, and they seem to work the same, which is expected since we just wrap everything at the top level.

Differences from v3 are:

- Recursive running of make is done with $(MAKE), and not using --no-print-directory. Also, change the "%" rule in "$(MAKECMDGOALS)" to properly detect phony rules.

- Use --chmod on the external toolchain's rsync, to prevent any weird permissions coming from there. This isn't stricly necessary, since the toolchain libs are copied to the target via 'install', which won't depend on any existing permissions. But this way we're safer in case anything changes in the future. Also we make sure everything under output/ is controlled.

- Fix some cp's to the target from non-controlled dirs on boot/grub/grub.mk, fs/iso9660/iso9660.mk, and board/boundarydevices/nitrogen6x/post-build.sh

- Fifth patch dropped as it was accepted.

- Added the umask setting in the Makefile generated in the $(O) dir. This is just an optimization so we don't have another level of 'make' running.

- Fixed 'install' call on matchbox-keyboard patch (#4)

Thanks!

---

Guido Mart?nez (9):
Makefile: don't depend on the umask
Makefile: don't depend on current skeleton/overlay permissions toolchain/helpers.mk: use --chmod on rsync pkg-generic.mk: don't depend on external package permissions
package: matchbox-keyboard: use install instead of cp
scripts: mkmakefile: set umask before calling BR's makefile
nitrogen6x: use install instead of cp
boot/grub: use install instead of cp
fs/iso9660: use install instead of cp

Quoted 10 lines
 Makefile                                              | 19 +++++++++++++++++--
 board/boundarydevices/nitrogen6x/post-build.sh        |  6 +++---
 boot/grub/grub.mk                                     | 14 +++++++-------
 fs/iso9660/iso9660.mk                                 | 16 +++++++++-------
 .../matchbox/matchbox-keyboard/matchbox-keyboard.mk   |  2 +-
 package/pkg-generic.mk                                |  2 +-
 support/scripts/mkmakefile                            |  2 +-
 toolchain/helpers.mk                                  |  2 +-
 8 files changed, 40 insertions(+), 23 deletions(-)

Wouldn't it be nice to document this behavior in the manual?

Unless this is considered just an implementation detail.

Signature · 2 lines
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
#25 lore raw

v4 cover target permissions

In reply to Guido Martínez

Hi everyone,

Quoted 12 lines · Guido Mart?nez

[...]
Guido Mart?nez (9):
Makefile: don't depend on the umask
Makefile: don't depend on current skeleton/overlay permissions toolchain/helpers.mk: use --chmod on rsync pkg-generic.mk: don't depend on external package permissions
package: matchbox-keyboard: use install instead of cp
scripts: mkmakefile: set umask before calling BR's makefile
nitrogen6x: use install instead of cp
boot/grub: use install instead of cp
fs/iso9660: use install instead of cp

Quoted 10 lines
 Makefile                                              | 19 +++++++++++++++++--
 board/boundarydevices/nitrogen6x/post-build.sh        |  6 +++---
 boot/grub/grub.mk                                     | 14 +++++++-------
 fs/iso9660/iso9660.mk                                 | 16 +++++++++-------
 .../matchbox/matchbox-keyboard/matchbox-keyboard.mk   |  2 +-
 package/pkg-generic.mk                                |  2 +-
 support/scripts/mkmakefile                            |  2 +-
 toolchain/helpers.mk                                  |  2 +-
 8 files changed, 40 insertions(+), 23 deletions(-)

Any comments on this? Has anyone tested/reviewed the new set?

Cheers!

Signature · 2 lines
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar