| Message ID | 1424792855-14278-2-git-send-email-stefan.sorensen@spectralink.com |
|---|---|
| State | Rejected |
| Headers | show |
Stefan, All, On 2015-02-24 16:47 +0100, Stefan Sørensen spake thusly: > 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... > 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... > 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). > 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--] > diff --git a/system/Config.in b/system/Config.in > index 95e10ab..b416377 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -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.
On Tue, 2015-02-24 at 22:39 +0100, Yann E. MORIN wrote: > > 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. > > 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). > > +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. > 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
Stefan, On Wed, 25 Feb 2015 12:02:41 +0000, Sørensen, Stefan wrote: > > 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
diff --git a/fs/common.mk b/fs/common.mk index 1d3926f..5c4cd6a 100644 --- a/fs/common.mk +++ b/fs/common.mk @@ -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) diff --git a/system/Config.in b/system/Config.in index 95e10ab..b416377 100644 --- a/system/Config.in +++ b/system/Config.in @@ -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 ""
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. Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com> --- fs/common.mk | 1 + system/Config.in | 16 ++++++++++++++++ 2 files changed, 17 insertions(+)