diff mbox series

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

Message ID 1600333425.32426.1.camel@aliyun.com
State Changes Requested
Headers show
Series [1/1] package/e2fsprogs: disable the e2scrub stuff | expand

Commit Message

Tian Yuanhao Sept. 17, 2020, 9:03 a.m. UTC
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@bootlin.com/

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

-- 
2.7.4

Comments

Thomas Petazzoni Sept. 17, 2020, 7:39 p.m. UTC | #1
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@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
Tian Yuanhao Sept. 18, 2020, 1:57 a.m. UTC | #2
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
Yann E. MORIN Sept. 20, 2020, 9:12 p.m. UTC | #3
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@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.
diff mbox series

Patch

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