[Buildroot] [PATCH 1/1] package/e2fsprogs: disable the e2scrub stuff

[Buildroot] [PATCH 1/1] package/e2fsprogs: disable the e2scrub stuff

[Tian Yuanhao]

e2scrub_all depends on the readlink of coreutils. If you use the
readlink of busybox, an error [1] occurs.

Embedded systems usually do not use LVM, in which case e2scrub is
useless.

There is no single option to completely disable e2scrub, so use a method
similar to [2] to disable the e2scrub stuff.

[1]: https://github.com/tytso/e2fsprogs/issues/32
[2]: https://patchwork.ozlabs.org/project/buildroot/patch/20200717120654.548833-1-antoine.tenart at bootlin.com/

Signed-off-by: Tian Yuanhao <tianyuanhao@aliyun.com>
---
?package/e2fsprogs/e2fsprogs.mk | 5 ++++-
?1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
index eb82a55..cf6579d 100644
--- a/package/e2fsprogs/e2fsprogs.mk
+++ b/package/e2fsprogs/e2fsprogs.mk
@@ -52,7 +52,10 @@ E2FSPROGS_CONF_OPTS = \
?	--disable-e2initrd-helper \
?	--disable-testio-debug \
?	--disable-rpath \
-	--enable-symlink-install
+	--enable-symlink-install \
+	--with-crond-dir=no \
+	--with-systemd-unit-dir=no \
+	--with-udev-rules-dir=no
?
?ifeq ($(BR2_PACKAGE_E2FSPROGS_FUSE2FS),y)
?E2FSPROGS_CONF_OPTS += --enable-fuse2fs
--?
2.7.4

[Buildroot] [PATCH 1/1] package/e2fsprogs: disable the e2scrub stuff

[Thomas Petazzoni]

On Thu, 17 Sep 2020 17:03:45 +0800
Tian Yuanhao via buildroot <buildroot@busybox.net> wrote:

> e2scrub_all depends on the readlink of coreutils. If you use the
> readlink of busybox, an error [1] occurs.
> 
> Embedded systems usually do not use LVM, in which case e2scrub is
> useless.
> 
> There is no single option to completely disable e2scrub, so use a method
> similar to [2] to disable the e2scrub stuff.
> 
> [1]: https://github.com/tytso/e2fsprogs/issues/32
> [2]: https://patchwork.ozlabs.org/project/buildroot/patch/20200717120654.548833-1-antoine.tenart at bootlin.com/
> 
> Signed-off-by: Tian Yuanhao <tianyuanhao@aliyun.com>
> ---
> ?package/e2fsprogs/e2fsprogs.mk | 5 ++++-
> ?1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
> index eb82a55..cf6579d 100644
> --- a/package/e2fsprogs/e2fsprogs.mk
> +++ b/package/e2fsprogs/e2fsprogs.mk
> @@ -52,7 +52,10 @@ E2FSPROGS_CONF_OPTS = \
> ?	--disable-e2initrd-helper \
> ?	--disable-testio-debug \
> ?	--disable-rpath \
> -	--enable-symlink-install
> +	--enable-symlink-install \
> +	--with-crond-dir=no \
> +	--with-systemd-unit-dir=no \
> +	--with-udev-rules-dir=no

I'm not sure this is really a good solution. Indeed, we probably do
want to install udev rules or systemd services.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/e2fsprogs: disable the e2scrub stuff

[Tian Yuanhao]

On ?, 2020-09-17 at 21:39 +0200, Thomas Petazzoni wrote:
> On Thu, 17 Sep 2020 17:03:45 +0800
> Tian Yuanhao via buildroot <buildroot@busybox.net> wrote:
>?
> > 
> > diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
> > index eb82a55..cf6579d 100644
> > --- a/package/e2fsprogs/e2fsprogs.mk
> > +++ b/package/e2fsprogs/e2fsprogs.mk
> > @@ -52,7 +52,10 @@ E2FSPROGS_CONF_OPTS = \
> > ?	--disable-e2initrd-helper \
> > ?	--disable-testio-debug \
> > ?	--disable-rpath \
> > -	--enable-symlink-install
> > +	--enable-symlink-install \
> > +	--with-crond-dir=no \
> > +	--with-systemd-unit-dir=no \
> > +	--with-udev-rules-dir=no
> I'm not sure this is really a good solution. Indeed, we probably do
> want to install udev rules or systemd services.
> 
> Thomas

'--with-crond-dir=no --with-systemd-unit-dir=no --with-udev-rules-dir=no'
is approximately equal to '--disable-scrub' which does not exist.

Android [1], Yocto [2] and PTXdist [3] do not install e2scrub. Therefore,
I thought Buildroot should boldly use those options.

[1]: https://android.googlesource.com/platform/external/e2fsprogs/+/refs/heads/master/Android.bp
[2]: http://cgit.openembedded.org/openembedded-core/tree/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.45.6.bb
[3]: https://git.pengutronix.de/cgit/ptxdist/tree/rules/e2fsprogs.make

[Buildroot] [PATCH 1/1] package/e2fsprogs: disable the e2scrub stuff

[Yann E. MORIN]

Tian, All,

On 2020-09-18 09:57 +0800, Tian Yuanhao via buildroot spake thusly:
> On ?, 2020-09-17 at 21:39 +0200, Thomas Petazzoni wrote:
> > On Thu, 17 Sep 2020 17:03:45 +0800
> > Tian Yuanhao via buildroot <buildroot@busybox.net> wrote:
> >?
> > > 
> > > diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
> > > index eb82a55..cf6579d 100644
> > > --- a/package/e2fsprogs/e2fsprogs.mk
> > > +++ b/package/e2fsprogs/e2fsprogs.mk
> > > @@ -52,7 +52,10 @@ E2FSPROGS_CONF_OPTS = \
> > > ?	--disable-e2initrd-helper \
> > > ?	--disable-testio-debug \
> > > ?	--disable-rpath \
> > > -	--enable-symlink-install
> > > +	--enable-symlink-install \
> > > +	--with-crond-dir=no \
> > > +	--with-systemd-unit-dir=no \
> > > +	--with-udev-rules-dir=no
> > I'm not sure this is really a good solution. Indeed, we probably do
> > want to install udev rules or systemd services.
> > 
> > Thomas
> 
> '--with-crond-dir=no --with-systemd-unit-dir=no --with-udev-rules-dir=no'
> is approximately equal to '--disable-scrub' which does not exist.

But this is not very obvious. Also, even if one does not want e2scrub,
ont may still want the systemd units files and udev ruels to be
installed, sa Thomas already explained.

Furthermore, if one look at the e2fsprogs Mafiles, e2scrub is onlt build
on linux;

    $ cat configure.ac
    [...]
      1464 dnl e2scrub only builds on linux
      1465 dnl
      1466 E2SCRUB_CMT="$LINUX_CMT"
      1467 AC_SUBST(E2SCRUB_CMT)
    [...]

And so, following the E2SCRUB_CMT definition leads to:

    $ cat -n Makefile.in
    [...]
      16 @E2SCRUB_CMT at E2SCRUB_DIR= scrub
    [...]

So, if E2SCRUB_CMT is set (to a sharp sign), E2SCRUB_DIR is not set.
E2SCRUB_DIR is then used later on:

    $ cat -n Makefile.in
    [...]
      24 PROG_SUBDIRS=e2fsck $(DEBUGFS_DIR) misc $(RESIZE_DIR) tests/progs po \
      25         $(E2SCRUB_DIR)
    [...]

So, if we really want to be able to disable e2scrub, can we try to use:

    E2FSPROGS_MAKE_OPTS = E2SCRUB_DIR=
    E2FSPROGS_INSTALL_TARGET_OPTS = E2SCRUB_DIR= DESTDIR=$(TARGET_DIR) install

Could you give it a try, and respin a new patch, please?

> Android [1], Yocto [2] and PTXdist [3] do not install e2scrub. Therefore,
> I thought Buildroot should boldly use those options.
> [1]: https://android.googlesource.com/platform/external/e2fsprogs/+/refs/heads/master/Android.bp
> [2]: http://cgit.openembedded.org/openembedded-core/tree/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.45.6.bb
> [3]: https://git.pengutronix.de/cgit/ptxdist/tree/rules/e2fsprogs.make

As far as I can tell, they all use different ways to disable e2scrub:

  - Android and PTXDist seem to do selective install, by only installing
    in select sub-dirs

  - the Yocto one seem to cherry-pick individual files from the
    'staging' areau, when assembling the package archive.

I.e. none of those three do explicitly disable e2scrub, they just don;t
install it.

But Buildroot does no selective install; we just rely on the package's
own 'make install' rule.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 1/1] package/e2fsprogs: disable e2scrub

[Tian Yuanhao]

e2scrub_all depends on coreutils' readlink. If you use busybox's
readlink, you will see the following error:

	readlink: invalid option -- 'e'

Embedded systems usually don't install e2scrub, which is useless without
LVM.

Signed-off-by: Tian Yuanhao <tianyuanhao@aliyun.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
?package/e2fsprogs/e2fsprogs.mk | 4 ++++
?1 file changed, 4 insertions(+)

diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
index eb82a55..44413ba 100644
--- a/package/e2fsprogs/e2fsprogs.mk
+++ b/package/e2fsprogs/e2fsprogs.mk
@@ -73,6 +73,10 @@ E2FSPROGS_INSTALL_STAGING_OPTS = \
?	DESTDIR=$(STAGING_DIR) \
?	install-libs
?
+# Disable e2scrub as we don't need it.
+E2FSPROGS_MAKE_OPTS = E2SCRUB_DIR=
+E2FSPROGS_INSTALL_TARGET_OPTS = E2SCRUB_DIR= DESTDIR=$(TARGET_DIR) install
+
?# Package does not build in parallel due to improper make rules
?define HOST_E2FSPROGS_INSTALL_CMDS
?	$(HOST_MAKE_ENV) $(MAKE1) -C $(@D) install install-libs
--?
2.7.4

[Buildroot] [PATCH v2 1/1] package/e2fsprogs: disable e2scrub

[James Hilliard]

On Sun, Sep 20, 2020 at 8:49 PM Tian Yuanhao via buildroot
<buildroot@busybox.net> wrote:
>
> e2scrub_all depends on coreutils' readlink. If you use busybox's
> readlink, you will see the following error:
>
>         readlink: invalid option -- 'e'
>
> Embedded systems usually don't install e2scrub, which is useless without
> LVM.
>
> Signed-off-by: Tian Yuanhao <tianyuanhao@aliyun.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
>  package/e2fsprogs/e2fsprogs.mk | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
> index eb82a55..44413ba 100644
> --- a/package/e2fsprogs/e2fsprogs.mk
> +++ b/package/e2fsprogs/e2fsprogs.mk
> @@ -73,6 +73,10 @@ E2FSPROGS_INSTALL_STAGING_OPTS = \
>         DESTDIR=$(STAGING_DIR) \
>         install-libs
>
> +# Disable e2scrub as we don't need it.
> +E2FSPROGS_MAKE_OPTS = E2SCRUB_DIR=
> +E2FSPROGS_INSTALL_TARGET_OPTS = E2SCRUB_DIR= DESTDIR=$(TARGET_DIR) install
> +
We want to disable e2scrub from running when it will fail but I'm not sure
we should unconditionally disable it for cases when it would function.
Can we make the cron/udev/service installation conditional on e2scrub
dependencies being present.

I think you can disable it from running with these configure options, these
should probably also be conditional and depend on systemd crond and udev
support:
--with-crond-dir=no
--with-udev-rules-dir=no
--with-systemd-unit-dir=no
>  # Package does not build in parallel due to improper make rules
>  define HOST_E2FSPROGS_INSTALL_CMDS
>         $(HOST_MAKE_ENV) $(MAKE1) -C $(@D) install install-libs
> --
> 2.7.4
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH v3 1/1] package/e2fsprogs: disable e2scrub conditionally

[Tian Yuanhao]

e2scrub_all depends on coreutils' readlink. If you use busybox's
readlink, you will see the following error:

  readlink: invalid option -- 'e'

Disable e2scrub if its dependencies are not met.

Signed-off-by: Tian Yuanhao <tianyuanhao@aliyun.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: James Hilliard <james.hilliard1@gmail.com>

---
Changes v2 -> v3:
  - check dependencies (suggested by James)

Changes v1 -> v2:
  - use make flags instead of configure options (suggested by Yann)
---
 package/e2fsprogs/e2fsprogs.mk | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
index 4420d16aff..db4b849143 100644
--- a/package/e2fsprogs/e2fsprogs.mk
+++ b/package/e2fsprogs/e2fsprogs.mk
@@ -74,6 +74,16 @@ E2FSPROGS_INSTALL_STAGING_OPTS = \
 	DESTDIR=$(STAGING_DIR) \
 	install-libs
 
+# e2scrub support needs bash, coreutils, lvm2 and util-linux
+ifneq ($(BR2_PACKAGE_BASH)$(BR2_PACKAGE_COREUTILS)$(BR2_PACKAGE_LVM2)$(BR2_PACKAGE_UTIL_LINUX),yyyy)
+E2FSPROGS_MAKE_OPTS += E2SCRUB_DIR=
+endif
+
+E2FSPROGS_INSTALL_TARGET_OPTS = \
+	$(E2FSPROGS_MAKE_OPTS) \
+	DESTDIR=$(TARGET_DIR) \
+	install
+
 # Package does not build in parallel due to improper make rules
 define HOST_E2FSPROGS_INSTALL_CMDS
 	$(HOST_MAKE_ENV) $(MAKE1) -C $(@D) install install-libs
-- 
2.25.1

[Buildroot] [PATCH v3 1/1] package/e2fsprogs: disable e2scrub conditionally

[Yann E. MORIN]

Tian, All,

On 2021-04-30 17:16 +0800, Tian Yuanhao via buildroot spake thusly:
> e2scrub_all depends on coreutils' readlink. If you use busybox's
> readlink, you will see the following error:
> 
>   readlink: invalid option -- 'e'
> 
> Disable e2scrub if its dependencies are not met.
> 
> Signed-off-by: Tian Yuanhao <tianyuanhao@aliyun.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: James Hilliard <james.hilliard1@gmail.com>

I've done a few changes:

  - add a kconfig option like for other tools
  - move the conditions to that new option
  - reword the commit log accordingly

With the above: applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
> Changes v2 -> v3:
>   - check dependencies (suggested by James)
> 
> Changes v1 -> v2:
>   - use make flags instead of configure options (suggested by Yann)
> ---
>  package/e2fsprogs/e2fsprogs.mk | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
> index 4420d16aff..db4b849143 100644
> --- a/package/e2fsprogs/e2fsprogs.mk
> +++ b/package/e2fsprogs/e2fsprogs.mk
> @@ -74,6 +74,16 @@ E2FSPROGS_INSTALL_STAGING_OPTS = \
>  	DESTDIR=$(STAGING_DIR) \
>  	install-libs
>  
> +# e2scrub support needs bash, coreutils, lvm2 and util-linux
> +ifneq ($(BR2_PACKAGE_BASH)$(BR2_PACKAGE_COREUTILS)$(BR2_PACKAGE_LVM2)$(BR2_PACKAGE_UTIL_LINUX),yyyy)
> +E2FSPROGS_MAKE_OPTS += E2SCRUB_DIR=
> +endif
> +
> +E2FSPROGS_INSTALL_TARGET_OPTS = \
> +	$(E2FSPROGS_MAKE_OPTS) \
> +	DESTDIR=$(TARGET_DIR) \
> +	install
> +
>  # Package does not build in parallel due to improper make rules
>  define HOST_E2FSPROGS_INSTALL_CMDS
>  	$(HOST_MAKE_ENV) $(MAKE1) -C $(@D) install install-libs
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v3 1/1] package/e2fsprogs: disable e2scrub conditionally

[Alexander Dahl]

Hei hei,

On Sat, May 01, 2021 at 12:03:25AM +0200, Yann E. MORIN wrote:
> Tian, All,
> 
> On 2021-04-30 17:16 +0800, Tian Yuanhao via buildroot spake thusly:
> > e2scrub_all depends on coreutils' readlink. If you use busybox's
> > readlink, you will see the following error:
> > 
> >   readlink: invalid option -- 'e'
> > 
> > Disable e2scrub if its dependencies are not met.
> > 
> > Signed-off-by: Tian Yuanhao <tianyuanhao@aliyun.com>
> > Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> > Cc: James Hilliard <james.hilliard1@gmail.com>
> 
> I've done a few changes:
> 
>   - add a kconfig option like for other tools
>   - move the conditions to that new option
>   - reword the commit log accordingly
> 
> With the above: applied to master, thanks.

That sounds like quite invasive changes.  What about juts providing
feedback and wait for a new iteration? o.O

Greets
Alex

> 
> Regards,
> Yann E. MORIN.
> 
> > ---
> > Changes v2 -> v3:
> >   - check dependencies (suggested by James)
> > 
> > Changes v1 -> v2:
> >   - use make flags instead of configure options (suggested by Yann)
> > ---
> >  package/e2fsprogs/e2fsprogs.mk | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
> > index 4420d16aff..db4b849143 100644
> > --- a/package/e2fsprogs/e2fsprogs.mk
> > +++ b/package/e2fsprogs/e2fsprogs.mk
> > @@ -74,6 +74,16 @@ E2FSPROGS_INSTALL_STAGING_OPTS = \
> >  	DESTDIR=$(STAGING_DIR) \
> >  	install-libs
> >  
> > +# e2scrub support needs bash, coreutils, lvm2 and util-linux
> > +ifneq ($(BR2_PACKAGE_BASH)$(BR2_PACKAGE_COREUTILS)$(BR2_PACKAGE_LVM2)$(BR2_PACKAGE_UTIL_LINUX),yyyy)
> > +E2FSPROGS_MAKE_OPTS += E2SCRUB_DIR=
> > +endif
> > +
> > +E2FSPROGS_INSTALL_TARGET_OPTS = \
> > +	$(E2FSPROGS_MAKE_OPTS) \
> > +	DESTDIR=$(TARGET_DIR) \
> > +	install
> > +
> >  # Package does not build in parallel due to improper make rules
> >  define HOST_E2FSPROGS_INSTALL_CMDS
> >  	$(HOST_MAKE_ENV) $(MAKE1) -C $(@D) install install-libs
> > -- 
> > 2.25.1
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot at busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
/"\ ASCII RIBBON | ?With the first link, the chain is forged. The first
\ / CAMPAIGN     | speech censured, the first thought forbidden, the
 X  AGAINST      | first freedom denied, chains us all irrevocably.?
/ \ HTML MAIL    | (Jean-Luc Picard, quoting Judge Aaron Satie)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20210501/9830f14a/attachment.asc>