[Buildroot] [PATCH v4 0/9] target permissions

[Buildroot] [PATCH v4 0/9] target permissions

[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

 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(-)

-- 
2.1.3

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

[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.

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Makefile b/Makefile
index 3d8283b..eeb1412 100644
--- a/Makefile
+++ b/Makefile
@@ -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
-- 
2.1.3

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

[Arnout Vandecappelle]

On 11/21/14 17:19, Guido Mart?nez wrote:
> 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.
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 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

> ---
>  Makefile | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 3d8283b..eeb1412 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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
> 


-- 
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

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

[Thomas Petazzoni]

Dear Guido Mart?nez,

On Fri, 21 Nov 2014 13:19:00 -0300, Guido Mart?nez wrote:
> 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.
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> ---
>  Makefile | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)

Applied with some additional changes:

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

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[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.

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index eeb1412..5880bbb 100644
--- a/Makefile
+++ b/Makefile
@@ -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)), \
-- 
2.1.3

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

[Arnout Vandecappelle]

On 11/21/14 17:19, Guido Mart?nez wrote:
> 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.
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 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

> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index eeb1412..5880bbb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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)), \
> 


-- 
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

[Buildroot] [PATCH] CHANGES: add release note for the permissions

[Arnout Vandecappelle]

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.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 CHANGES | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/CHANGES b/CHANGES
index 3f5162e..1e94929 100644
--- a/CHANGES
+++ b/CHANGES
@@ -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.
-- 
2.1.4

[Buildroot] [PATCH] CHANGES: add release note for the permissions

[Baruch Siach]

Hi Arnout,

On Mon, Jul 13, 2015 at 01:36:28PM +0200, Arnout Vandecappelle (Essensium/Mind) wrote:
> 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

> how it should be done.

baruch

-- 
     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 -

[Buildroot] [PATCH] CHANGES: add release note for the permissions

[Thomas Petazzoni]

Dear Arnout Vandecappelle (Essensium/Mind),

On Mon, 13 Jul 2015 13:36:28 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> 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.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  CHANGES | 11 +++++++++++
>  1 file changed, 11 insertions(+)

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

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[Thomas Petazzoni]

Dear Guido Mart?nez,

On Fri, 21 Nov 2014 13:19:01 -0300, Guido Mart?nez wrote:
> 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.
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[Guido Martínez]

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

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 toolchain/helpers.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
index 3121da4..46f5f22 100644
--- a/toolchain/helpers.mk
+++ b/toolchain/helpers.mk
@@ -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 ; \
-- 
2.1.3

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

[Arnout Vandecappelle]

On 11/21/14 17:19, Guido Mart?nez wrote:
> This makes sure we don't have any weird permissions on the staging dir,
> which could affect the target.
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  toolchain/helpers.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> index 3121da4..46f5f22 100644
> --- a/toolchain/helpers.mk
> +++ b/toolchain/helpers.mk
> @@ -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 ; \
> 


-- 
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

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

[Thomas Petazzoni]

Dear Guido Mart?nez,

On Fri, 21 Nov 2014 13:19:02 -0300, Guido Mart?nez wrote:
> This makes sure we don't have any weird permissions on the staging dir,
> which could affect the target.
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> ---
>  toolchain/helpers.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[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.

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 9643a30..ec2989f 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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 $@
 
-- 
2.1.3

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

[Thomas Petazzoni]

Dear Guido Mart?nez,

On Fri, 21 Nov 2014 13:19:03 -0300, Guido Mart?nez wrote:
> 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.
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> ---
>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[Guido Martínez]

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

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 package/matchbox/matchbox-keyboard/matchbox-keyboard.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/matchbox/matchbox-keyboard/matchbox-keyboard.mk b/package/matchbox/matchbox-keyboard/matchbox-keyboard.mk
index ebf23e4..bdcc32f 100644
--- a/package/matchbox/matchbox-keyboard/matchbox-keyboard.mk
+++ b/package/matchbox/matchbox-keyboard/matchbox-keyboard.mk
@@ -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
-- 
2.1.3

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

[Thomas Petazzoni]

Dear Guido Mart?nez,

On Fri, 21 Nov 2014 13:19:04 -0300, Guido Mart?nez wrote:
> in order to not depend on the previous permissions of the file
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> ---
>  package/matchbox/matchbox-keyboard/matchbox-keyboard.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patch applied to the next branch, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[Guido Martínez]

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

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 support/scripts/mkmakefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/scripts/mkmakefile b/support/scripts/mkmakefile
index 27b1507..974e2c0 100755
--- a/support/scripts/mkmakefile
+++ b/support/scripts/mkmakefile
@@ -35,7 +35,7 @@ MAKEFLAGS += --no-print-directory
 all	:= \$(filter-out Makefile,\$(MAKECMDGOALS))
 
 _all:
-	\$(MAKE) \$(MAKEARGS) \$(all)
+	umask 0022 && \$(MAKE) \$(MAKEARGS) \$(all)
 
 Makefile:;
 
-- 
2.1.3

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

[Thomas Petazzoni]

Dear Guido Mart?nez,

On Fri, 21 Nov 2014 13:19:05 -0300, Guido Mart?nez wrote:
> Small optimization so we don't have another 'make' level (caused by the
> umask fix) when running the generated makefile.
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> ---
>  support/scripts/mkmakefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

[Guido Martínez]

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

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 board/boundarydevices/nitrogen6x/post-build.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/board/boundarydevices/nitrogen6x/post-build.sh b/board/boundarydevices/nitrogen6x/post-build.sh
index cc6e66a..2b43aa3 100755
--- a/board/boundarydevices/nitrogen6x/post-build.sh
+++ b/board/boundarydevices/nitrogen6x/post-build.sh
@@ -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
-- 
2.1.3

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

[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.

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 boot/grub/grub.mk | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/boot/grub/grub.mk b/boot/grub/grub.mk
index 535ada6..ae687d6 100644
--- a/boot/grub/grub.mk
+++ b/boot/grub/grub.mk
@@ -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
 
-- 
2.1.3

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

[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.

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 fs/iso9660/iso9660.mk | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
index 908f0e9..5b44ba4 100644
--- a/fs/iso9660/iso9660.mk
+++ b/fs/iso9660/iso9660.mk
@@ -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)
-- 
2.1.3

[Buildroot] [PATCH v4 0/9] target permissions

[Thomas Petazzoni]

Dear Guido Mart?nez,

On Fri, 21 Nov 2014 13:18:59 -0300, Guido Mart?nez wrote:

> Guido Mart?nez (9):
[...]
>   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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v4 0/9] target permissions

[Guido Martínez]

On Sat, Nov 22, 2014 at 07:02:33PM +0100, Thomas Petazzoni wrote:
> Dear Guido Mart?nez,
> 
> On Fri, 21 Nov 2014 13:18:59 -0300, Guido Mart?nez wrote:
> 
> > Guido Mart?nez (9):
> [...]
> >   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.

-- 
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar

[Buildroot] [PATCH v4 0/9] target permissions

[Ezequiel Garcia]

On 11/21/2014 01:18 PM, Guido Mart?nez wrote:
> 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(-)
> 

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

Unless this is considered just an implementation detail.
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

[Buildroot] [PATCH v4 0/9] target permissions

[Guido Martínez]

Hi everyone,

On Fri, Nov 21, 2014 at 01:18:59PM -0300, Guido Mart?nez wrote:
>[...]
> 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(-)
> 

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

Cheers!

-- 
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar