fs: Bail out on errors in fakeroot script
PATCH 1/2 fs: Bail out on errors in fakeroot script
Currently, the generated fakeroot script has no error checking causing make to continue building even if some of the fakeroot script commands have failed. This can cause e.g. using an invalid device tables to go unnoticed.
So add a "set -e" to the start of the fakeroot script so it will exit with a failure code as soon as one of the script commands fails.
--- fs/common.mk | 1 + 1 file changed, 1 insertion(+)
fs/common.mk +1 -0
@@ -72,6 +72,7 @@ $$(BINARIES_DIR)/rootfs.$(1): target-finalize $$(ROOTFS_$(2)_DEPENDENCIES) rm -f $$(FAKEROOT_SCRIPT) rm -f $$(TARGET_DIR_WARNING_FILE) rm -f $$(USERS_TABLE) + echo "set -e" >> $$(FAKEROOT_SCRIPT) echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) ifneq ($$(ROOTFS_DEVICE_TABLES),) cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
Signature · 1 line
1.9.3
PATCH 2/2 fs: add custom fakeroot script support
This patch add a new config option, BR2_ROOTFS_FAKEROOT_SCRIPT, a list of scripts that are executed in the fakeroot environment as the last step before creating the filesystem image.
The scripts can for example be used to invoke a tool that operates on file ownerships, device nodes or any other custom action that requires root privileges.
--- fs/common.mk | 1 + system/Config.in | 16 ++++++++++++++++ 2 files changed, 17 insertions(+)
fs/common.mk +1 -0
@@ -87,6 +87,7 @@ ifneq ($$(ROOTFS_USERS_TABLES),) endif printf '$$(subst $$(sep),\n,$$(PACKAGES_USERS))' >> $$(USERS_TABLE) PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT) + $$(foreach s,$$(call qstrip,$$(BR2_ROOTFS_FAKEROOT_SCRIPT)),echo $$(s) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT)$$(sep)) echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT) chmod a+x $$(FAKEROOT_SCRIPT) PATH=$$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(FAKEROOT_SCRIPT)
system/Config.in +16 -0
@@ -421,6 +421,22 @@ config BR2_ROOTFS_POST_BUILD_SCRIPT argument. Make sure the exit code of those scripts are 0, otherwise make will stop after calling them. +config BR2_ROOTFS_FAKEROOT_SCRIPT + string "Custom scripts to run as fake root" + default "" + help + Specify a space-separated list of scripts to be run as part + of the fakeroot script, just before Buildroot packs the + files into selected filesystem images. + + This can for example be used to invoke a tool that operates on + file ownerships, device nodes or any other custom action that + requires root privileges. + + These scripts are called with the target directory name as + first argument. The script is executed from the main Buildroot + source directory as the current directory. + config BR2_ROOTFS_POST_IMAGE_SCRIPT string "Custom scripts to run after creating filesystem images" default ""
Signature · 1 line
1.9.3
PATCH 2/2 fs: add custom fakeroot script support
Stefan, All,
On 2015-02-24 16:47 +0100, Stefan S?rensen spake thusly:
Quoted 3 lines
This patch add a new config option,
BR2_ROOTFS_FAKEROOT_SCRIPT, a list of scripts that are executed in the fakeroot environment as the last step before creating the filesystem image.
I'm not specially opposed to that feature (and in fact I think this would be nice to have!), but parts of the justification are not completely right, see below...
Quoted 2 lines
The scripts can for example be used to invoke a tool that operates on file ownerships,
For file ownership, we already have BR2_ROOTFS_DEVICE_TABLE: "Path to the permission tables". The naming of the option may be a bit misleading, indeed, but that's historical...
Quoted 1 line
device nodes
Ditto, we also already have BR2_ROOTFS_STATIC_DEVICE_TABLE: "Path to the device tables". That can indeed only be used when using static /dev management, but when /dev is managed dynamically (devtmpfs, eudev, mdev or systemd), having custom device tables is completely useless (as /dev is a mountpoint, and existing /dev entries would become invisible).
Quoted 2 lines
or any other custom action that requires
root privileges.
What would be those kind of "custom actions" that are not covered by the above?
One thing I can see as problematic is assigning ownership to a whole directory tree, of which the content might be unpredictable at configure time (e.g. so that it is not possible to provide a permission table for all the entries); something one might trivially do (with your solution):
#!/bin/sh
TARGET_DIR="${1}"
chown -R uid:gid ${TARGET_DIR}/foo/bar/But even with what we currently have, this is already doable (albeit this is a bit of a hack). One would set:
BR2_ROOTFS_DEVICE_TABLE="system/device_table.txt $(BUILD_DIR)/tmp-perms"
BR2_ROOTFS_POST_BUILD_SCRIPT="/path/to/post-build.script"
BR2_ROOTFS_POST_SCRIPT_ARGS"$(BUILD_DIR)"and then have /path/to/post-build.script contain something like:
#!/bin/sh
TARGET_DIR="${1}"
BUILD_DIR="${2}"
uid=42
gid=31415
find ${TARGET_DIR}/foo/bar/ \( -type f -o -type d \) \
|while read entry; do
[ -f "${entry}" ] && t=f
[ -d "${entry}" ] && t=d
m="$( stat -c '%a' "${entry}" )"
m=$((m&0760)) # Whatever...
printf "%s %s %s %s %s - - - - -\n" \
"${entry#${TARGET_DIR}}" ${t} ${m} ${uid} ${gid}
done >"${BUILD_DIR}/tmp-perms"Granted, that's neither trivial nor elegant, but that's doable.
[--SNIP--]
system/Config.in config BR2_ROOTFS_POST_BUILD_SCRIPT · line 425
Quoted 10 lines
system/Config.in +2
-0@@ -421,6 +421,22 @@ config BR2_ROOTFS_POST_BUILD_SCRIPT argument. Make sure the exit code of those scripts are 0, otherwise make will stop after calling them. +config BR2_ROOTFS_FAKEROOT_SCRIPT + string "Custom scripts to run as fake root"
I'm not very happy with talking about 'fake root' (which is really 'fakeroot'). I'd prefer we avoid mentionning fakeroot, which is internal implementation details.
Note also that those scripts would be run once for each filesystem type the user has enabled; if tar, squashfs, cramfs and ext2/3/4 are enabled, the scripts would be run four times. Thus, those scripts must be idempotent (like for the post-build scripts, mind you).
In the end (and after quite some thinking!), I stil fail to see a compelling reason. Do you have an example?
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 2/2 fs: add custom fakeroot script support
Quoted 6 lines · Yann E. MORIN
Quoted 2 lines
The scripts can for example be used to invoke a tool that operates on file ownerships,
For file ownership, we already have
BR2_ROOTFS_DEVICE_TABLE: "Path to the permission tables". The naming of the option may be a bit misleading, indeed, but that's historical...
As you mention yourself, bulk ownership changes are problematic with device tables. Generating the device table in the post-build script doesn't seem very elegant to me.
Quoted 5 lines
Quoted 2 lines
or any other custom action that requires
root privileges.What would be those kind of "custom actions" that are not covered by the above?
An example could be changes to files/device nodes that are created earlier in the script. We use it to remove /dev/console created by /fs/cpio.mk (our image format doesn't support device nodes).
Creating a manifest file of the whole filesystem, including stuff added by the fakeroot script, would also need to be done just before image creation (although here there is no need for being root).
Quoted 6 lines
system/Config.in config BR2_ROOTFS_POST_BUILD_SCRIPT · line 425
Quoted 2 lines
system/Config.in +2
-0+config BR2_ROOTFS_FAKEROOT_SCRIPT + string "Custom scripts to run as fake root"I'm not very happy with talking about 'fake root' (which is really 'fakeroot'). I'd prefer we avoid mentionning fakeroot, which is internal implementation details.
I will rewrite it to not mention fakeroot.
Quoted 4 lines
Note also that those scripts would be run once for each filesystem type the user has enabled; if tar, squashfs, cramfs and ext2/3/4 are enabled, the scripts would be run four times. Thus, those scripts must be idempotent (like for the post-build scripts, mind you).
I will add a comment to the help text.
Stefan
PATCH 2/2 fs: add custom fakeroot script support
Stefan,
Quoted 7 lines · Stefan
Quoted 3 lines
For file ownership, we already have
BR2_ROOTFS_DEVICE_TABLE: "Path to the permission tables". The naming of the option may be a bit misleading, indeed, but that's historical...As you mention yourself, bulk ownership changes are problematic with device tables. Generating the device table in the post-build script doesn't seem very elegant to me.
Since commit
http://git.buildroot.net/buildroot/commit/package/makedevs?id=410f9b60137820143caf51a2a37da6f8fa16679f,
you can use the permission table to recursively set the owernship/rights of a directory and all its subdirectories/files.
Therefore, I don't think this patch is needed, and I've marked it as Rejected in our patch tracking system. Do not hesitate to repost an updated version with a new justification if you don't agree, of course.
Thanks a lot!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/2 fs: Bail out on errors in fakeroot script
In reply to Stefan SørensenStefan, All,
On 2015-02-24 16:47 +0100, Stefan S?rensen spake thusly:
Quoted 9 lines
Currently, the generated fakeroot script has no error checking causing make to continue building even if some of the fakeroot script commands have failed. This can cause e.g. using an invalid device tables to go unnoticed.
So add a "set -e" to the start of the fakeroot script so it will exit with a failure code as soon as one of the script commands fails.
Signed-off-by Stefan S?rensen <[email protected]>
Regards,
Yann E. MORIN.
Quoted 23 lines
--- fs/common.mk | 1 + 1 file changed, 1 insertion(+)fs/common.mk +1
-0@@ -72,6 +72,7 @@ $$(BINARIES_DIR)/rootfs.$(1): target-finalize $$(ROOTFS_$(2)_DEPENDENCIES) rm -f $$(FAKEROOT_SCRIPT) rm -f $$(TARGET_DIR_WARNING_FILE) rm -f $$(USERS_TABLE) + echo "set -e" >> $$(FAKEROOT_SCRIPT) echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) ifneq ($$(ROOTFS_DEVICE_TABLES),) cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)Signature · 6 lines
1.9.3 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
Thomas Petazzoni
AppliedPATCH 1/2 fs: Bail out on errors in fakeroot script
In reply to Stefan SørensenDear Stefan S?rensen,
Quoted 9 lines · Stefan S?rensen
Currently, the generated fakeroot script has no error checking causing make to continue building even if some of the fakeroot script commands have failed. This can cause e.g. using an invalid device tables to go unnoticed.
So add a "set -e" to the start of the fakeroot script so it will exit with a failure code as soon as one of the script commands fails.
Signed-off-by Stefan S?rensen <[email protected]>
Applied, thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com