| Message ID | 20200612133249.4127906-1-heiko@sntech.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | fs: squashfs: Add config option to pass mksquashfs cmdln paramters. | expand |
Heiko, All, On 2020-06-12 15:32 +0200, Heiko Stuebner spake thusly: > From: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > This can be use to specify additional command line argument to > mksquashfs (e.g. -nopad). The implementation is inspired by > a similar mechanism for ext file system images. What kind of options do you expect to need to pass? Usually, I'd prefer we have common options directly mapped to entries in the menuconfig, and keep this kind of "pass-through" options for more exotic options. For example, if we are looking at the compression options, I'd like we add a config option like config BR2_TARGET_ROOTFS_SQUASHFS_EXTRA_COMPRESS bool "high-compression" depends on !BR2_TARGET_ROOTFS_SQUASHFS4_LZMA # No option help Say 'y' to compress even further, at the expense of compression time. For example: - for gzip, this enables all the -Xstrategy - for lz4, this enables -Xhc - for lzo, this enables -Xcompression-level 9 - for xz, this chooses appropriate -Xbcj filters - for zstd, this enables -Xcompression-level 22 And maybe a few other such options, eventually adding the BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS for corner-cases only. > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> > Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com> > --- > fs/squashfs/Config.in | 11 +++++++++++ > fs/squashfs/squashfs.mk | 8 +++++++- > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/fs/squashfs/Config.in b/fs/squashfs/Config.in > index dde2097cb7..789b2cd24a 100644 > --- a/fs/squashfs/Config.in > +++ b/fs/squashfs/Config.in > @@ -5,6 +5,16 @@ config BR2_TARGET_ROOTFS_SQUASHFS > > if BR2_TARGET_ROOTFS_SQUASHFS > > +config BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS > + string "additional mksquashfs options" > + default "" > + help > + Specify a space-separated list of mksquashfs options, including > + any squashfs filesystem features. > + > + For more information about the mke2fs options, see the manual s/mke2fs/mksquashfs/ > + page mksquashfs(1). > + > choice > prompt "Compression algorithm" > default BR2_TARGET_ROOTFS_SQUASHFS4_GZIP > @@ -31,4 +41,5 @@ config BR2_TARGET_ROOTFS_SQUASHFS4_ZSTD > bool "zstd" > > endchoice > + > endif > diff --git a/fs/squashfs/squashfs.mk b/fs/squashfs/squashfs.mk > index 34ab048603..4bc6b338b0 100644 > --- a/fs/squashfs/squashfs.mk > +++ b/fs/squashfs/squashfs.mk > @@ -6,7 +6,13 @@ > > ROOTFS_SQUASHFS_DEPENDENCIES = host-squashfs > > -ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS) > +SQUASHFS_MKFS_OPTS = $(call qstrip,$(BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS)) Keep the vairalbes prefixed with ROOTFS_ here, to avoid clashing with the package namespace: SQUASHFS_MKFS_OPTS by name would be a variable of the "squashfs" package. > +ROOTFS_SQUASHFS_ARGS = \ > + -noappend \ > + -processors \ > + $(PARALLEL_JOBS) \ $(PARALLEL_JOBS) is a parameter to the --processor option, so it should be on the same line. However, I don;t think you need to introduce an intermediate variable: ROOTFS_SQUASHFS_ARGS = \ -noappend \ -processors $(PARALLEL_JOBS) \ $(call qstrip,$(BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS)) Regards, Yann E. MORIN. > + $(SQUASHFS_MKFS_OPTS) > > ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_LZ4),y) > ROOTFS_SQUASHFS_ARGS += -comp lz4 -Xhc > -- > 2.26.2 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hi, Am Sonntag, 14. Juni 2020, 09:57:25 CEST schrieb Yann E. MORIN: > Heiko, All, > > On 2020-06-12 15:32 +0200, Heiko Stuebner spake thusly: > > From: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > > > This can be use to specify additional command line argument to > > mksquashfs (e.g. -nopad). The implementation is inspired by > > a similar mechanism for ext file system images. > > What kind of options do you expect to need to pass? In our case it is "-nopad" to omit padding to the next 4k boundary. > Usually, I'd prefer we have common options directly mapped to entries in > the menuconfig, and keep this kind of "pass-through" options for more > exotic options. > > For example, if we are looking at the compression options, I'd like we > add a config option like > > config BR2_TARGET_ROOTFS_SQUASHFS_EXTRA_COMPRESS > bool "high-compression" > depends on !BR2_TARGET_ROOTFS_SQUASHFS4_LZMA # No option > help > Say 'y' to compress even further, at the expense of > compression time. > > For example: > - for gzip, this enables all the -Xstrategy > - for lz4, this enables -Xhc > - for lzo, this enables -Xcompression-level 9 > - for xz, this chooses appropriate -Xbcj filters > - for zstd, this enables -Xcompression-level 22 > > And maybe a few other such options, eventually adding the > BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS for corner-cases only. ok, so if I'm reading you correctly, I should probably just introduce a config BR2_TARGET_ROOTFS_SQUASHFS_NOPAD option, right? > > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com> > > --- > > fs/squashfs/Config.in | 11 +++++++++++ > > fs/squashfs/squashfs.mk | 8 +++++++- > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/fs/squashfs/Config.in b/fs/squashfs/Config.in > > index dde2097cb7..789b2cd24a 100644 > > --- a/fs/squashfs/Config.in > > +++ b/fs/squashfs/Config.in > > @@ -5,6 +5,16 @@ config BR2_TARGET_ROOTFS_SQUASHFS > > > > if BR2_TARGET_ROOTFS_SQUASHFS > > > > +config BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS > > + string "additional mksquashfs options" > > + default "" > > + help > > + Specify a space-separated list of mksquashfs options, including > > + any squashfs filesystem features. > > + > > + For more information about the mke2fs options, see the manual > > s/mke2fs/mksquashfs/ ok > > + page mksquashfs(1). > > + > > choice > > prompt "Compression algorithm" > > default BR2_TARGET_ROOTFS_SQUASHFS4_GZIP > > @@ -31,4 +41,5 @@ config BR2_TARGET_ROOTFS_SQUASHFS4_ZSTD > > bool "zstd" > > > > endchoice > > + > > endif > > diff --git a/fs/squashfs/squashfs.mk b/fs/squashfs/squashfs.mk > > index 34ab048603..4bc6b338b0 100644 > > --- a/fs/squashfs/squashfs.mk > > +++ b/fs/squashfs/squashfs.mk > > @@ -6,7 +6,13 @@ > > > > ROOTFS_SQUASHFS_DEPENDENCIES = host-squashfs > > > > -ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS) > > +SQUASHFS_MKFS_OPTS = $(call qstrip,$(BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS)) > > Keep the vairalbes prefixed with ROOTFS_ here, to avoid clashing with > the package namespace: SQUASHFS_MKFS_OPTS by name would be a variable of > the "squashfs" package. ok, I wasn't sure here what the correct naming is, a lot of file-systems seem to use their options without ROOTFS_* So I'll change this to ROOTFS_* then. > > +ROOTFS_SQUASHFS_ARGS = \ > > + -noappend \ > > + -processors \ > > + $(PARALLEL_JOBS) \ > > $(PARALLEL_JOBS) is a parameter to the --processor option, so it should > be on the same line. > > However, I don;t think you need to introduce an intermediate variable: > > ROOTFS_SQUASHFS_ARGS = \ > -noappend \ > -processors $(PARALLEL_JOBS) \ > $(call qstrip,$(BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS)) this was a bit "modelled" after (aka stolen from) how the extfs does this ;-) But yeah, we can make this nicer . Heiko
Heiko, All, On 2020-06-14 12:20 +0200, Heiko Stübner spake thusly: > Am Sonntag, 14. Juni 2020, 09:57:25 CEST schrieb Yann E. MORIN: > > Heiko, All, > > > > On 2020-06-12 15:32 +0200, Heiko Stuebner spake thusly: > > > From: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > > > > > This can be use to specify additional command line argument to > > > mksquashfs (e.g. -nopad). The implementation is inspired by > > > a similar mechanism for ext file system images. > > > > What kind of options do you expect to need to pass? > > In our case it is "-nopad" to omit padding to the > next 4k boundary. [--SNIP--] > ok, so if I'm reading you correctly, I should probably > just introduce a > config BR2_TARGET_ROOTFS_SQUASHFS_NOPAD > option, right? Basically, right. But usually, we go with positive logic (at least user-facing), so: config BR2_TARGET_ROOTFS_SQUASHFS_PAD bool "pad to a 4K boundary" default y # legacy was always ON help Say 'y' here (the default) to pad the the filesystem image to a 4K boundary. Say 'n' to disable padding. And then in the .mk : ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS_PAD),) ROOTFS_SQUASHFS_ARGS += -nopad endif [--SNIP--] > > > -ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS) > > > +SQUASHFS_MKFS_OPTS = $(call qstrip,$(BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS)) > > > > Keep the vairalbes prefixed with ROOTFS_ here, to avoid clashing with > > the package namespace: SQUASHFS_MKFS_OPTS by name would be a variable of > > the "squashfs" package. > > ok, I wasn't sure here what the correct naming is, a lot of file-systems > seem to use their options without ROOTFS_* Yes, this is historical, and so far it works: we have no package named 'ext', so there are not actual clashing. But we do have a pakcage named 'squashfs', so there can be clashing. > > $(PARALLEL_JOBS) is a parameter to the --processor option, so it should > > be on the same line. > > However, I don;t think you need to introduce an intermediate variable: > > ROOTFS_SQUASHFS_ARGS = \ > > -noappend \ > > -processors $(PARALLEL_JOBS) \ > > $(call qstrip,$(BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS)) > this was a bit "modelled" after (aka stolen from) how the extfs does this ;-) I see, but I don't see the point there either. > But yeah, we can make this nicer . Please! :-) Regards, Yann E. MORIN.
diff --git a/fs/squashfs/Config.in b/fs/squashfs/Config.in index dde2097cb7..789b2cd24a 100644 --- a/fs/squashfs/Config.in +++ b/fs/squashfs/Config.in @@ -5,6 +5,16 @@ config BR2_TARGET_ROOTFS_SQUASHFS if BR2_TARGET_ROOTFS_SQUASHFS +config BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS + string "additional mksquashfs options" + default "" + help + Specify a space-separated list of mksquashfs options, including + any squashfs filesystem features. + + For more information about the mke2fs options, see the manual + page mksquashfs(1). + choice prompt "Compression algorithm" default BR2_TARGET_ROOTFS_SQUASHFS4_GZIP @@ -31,4 +41,5 @@ config BR2_TARGET_ROOTFS_SQUASHFS4_ZSTD bool "zstd" endchoice + endif diff --git a/fs/squashfs/squashfs.mk b/fs/squashfs/squashfs.mk index 34ab048603..4bc6b338b0 100644 --- a/fs/squashfs/squashfs.mk +++ b/fs/squashfs/squashfs.mk @@ -6,7 +6,13 @@ ROOTFS_SQUASHFS_DEPENDENCIES = host-squashfs -ROOTFS_SQUASHFS_ARGS = -noappend -processors $(PARALLEL_JOBS) +SQUASHFS_MKFS_OPTS = $(call qstrip,$(BR2_TARGET_ROOTFS_SQUASHFS_MKFS_OPTIONS)) + +ROOTFS_SQUASHFS_ARGS = \ + -noappend \ + -processors \ + $(PARALLEL_JOBS) \ + $(SQUASHFS_MKFS_OPTS) ifeq ($(BR2_TARGET_ROOTFS_SQUASHFS4_LZ4),y) ROOTFS_SQUASHFS_ARGS += -comp lz4 -Xhc