Skip to contentSkip to conversation

e2fsprogs: support findfs

RFC New James Knight 2 participants 3 messages Feb 2, 2016

lore
Share & tools
lore
#0 lore raw

RFC 1/1 e2fsprogs: support findfs

The following adjusts e2fsprogs's configure to force the enablement of CONFIG_BUILD_FINDFS. The current support for e2fsprogs's findfs is not complete; while the findfs symbolic link to e2label is made, e2label does not handle the findfs argument. By ensuring the CONFIG_BUILD_FINDFS flag is set, e2label is built with findfs support. Since e2fsprogs already requires libblkid, the build should not be an issue; however, using this option prevent a developer from restricting findfs-related code in e2label (see alternative below).

Note that the --disable-libblkid configuration argument must remain to prevent conflicts with util-linux's libblkid and an e2fsprogs- generated variant (see e1ffc2f791b336339909c90559b7db40b455f172).

An alternative to this approach would be to remove any e2fsprogs's findfs references from Buildroot and add the findfs utility to util-linux (since the package provides a findfs utility as well).

---
 package/e2fsprogs/0002-support-findfs.patch | 11 +++++++++++
 package/e2fsprogs/e2fsprogs.mk              |  3 +++
 2 files changed, 14 insertions(+)
 create mode 100644 package/e2fsprogs/0002-support-findfs.patch
package/e2fsprogs/0002-support-findfs.patch new file +11 -0
new file mode 100644
index 0000000..4e125fa
@@ -0,0 +1,11 @@
+diff -Naur a/configure.in b/configure.in
+--- a/configure.in	2016-02-02 15:03:02.440059366 -0500
++++ b/configure.in	2016-02-02 15:03:38.265404484 -0500
+@@ -533,6 +533,7 @@
+ 		[AC_MSG_ERROR([external blkid library not found])], -luuid)
+ 	BLKID_CMT=#
+ 	PROFILED_LIBBLKID=$LIBBLKID
++	AC_DEFINE(CONFIG_BUILD_FINDFS, 1)
+ 	AC_MSG_RESULT([Disabling private blkid library])
+ else
+ 	LIBBLKID='$(LIB)/libblkid'$LIB_EXT
package/e2fsprogs/e2fsprogs.mk +3 -0
index 4b81b44..d16edde 100644
@@ -12,6 +12,9 @@ E2FSPROGS_LICENSE_FILES = COPYING lib/uuid/COPYING lib/ss/mit-sipb-copyright.h l
 E2FSPROGS_INSTALL_STAGING = YES
 E2FSPROGS_INSTALL_STAGING_OPTS = DESTDIR=$(STAGING_DIR) install-libs
 
+# 0002-support-findfs.patch
+E2FSPROGS_AUTORECONF = YES
+
 # e4defrag doesn't build on older systems like RHEL5.x, and we don't
 # need it on the host anyway.
 HOST_E2FSPROGS_CONF_OPTS += --disable-defrag
Signature · 1 line
2.6.4.windows.1
#1 lore raw

RFC 1/1 e2fsprogs: support findfs

Quoted 9 lines · James Knight

The following adjusts e2fsprogs's configure to force the enablement of CONFIG_BUILD_FINDFS. The current support for e2fsprogs's findfs is not complete; while the findfs symbolic link to e2label is made, e2label does not handle the findfs argument. By ensuring the CONFIG_BUILD_FINDFS flag is set, e2label is built with findfs support. Since e2fsprogs already requires libblkid, the build should not be an issue; however, using this option prevent a developer from restricting findfs-related code in e2label (see alternative below).

Quoted 7 lines

Note that the --disable-libblkid configuration argument must remain to prevent conflicts with util-linux's libblkid and an e2fsprogs- generated variant (see e1ffc2f791b336339909c90559b7db40b455f172).

An alternative to this approach would be to remove any e2fsprogs's findfs references from Buildroot and add the findfs utility to util-linux (since the package provides a findfs utility as well).

Ideally, both should be done :-)

package/e2fsprogs/0002-support-findfs.patch

Quoted 15 lines
---
 package/e2fsprogs/0002-support-findfs.patch | 11 +++++++++++
 package/e2fsprogs/e2fsprogs.mk              |  3 +++
 2 files changed, 14 insertions(+)
 create mode 100644 package/e2fsprogs/0002-support-findfs.patch
package/e2fsprogs/0002-support-findfs.patch new file +0 -0
new file mode 100644
index 0000000..4e125fa
@@ -0,0 +1,11 @@

The patch is missing a commit message.

Did you send it upstream?

package/e2fsprogs/0002-support-findfs.patch line 8

Quoted 8 lines
package/e2fsprogs/0002-support-findfs.patch new file +8 -0
+diff -Naur a/configure.in b/configure.in
+--- a/configure.in	2016-02-02 15:03:02.440059366 -0500
++++ b/configure.in	2016-02-02 15:03:38.265404484 -0500
+@@ -533,6 +533,7 @@
+ 		[AC_MSG_ERROR([external blkid library not found])], -luuid)
+ 	BLKID_CMT=#
+ 	PROFILED_LIBBLKID=$LIBBLKID
++	AC_DEFINE(CONFIG_BUILD_FINDFS, 1)

Hm, it would be weird if this were allowed... If you add this,
CONFIG_BUILD_FINDFS will be defined if --disable-libblkid was given. if --enable-libblkid was given, and if nothing is given. So the whole point of the option is moot. That doesn't sound right...

Can you check that with upstream, or consult their logs when the option was
introduced?

Regards,
Arnout

Quoted 18 lines
package/e2fsprogs/0002-support-findfs.patch new file +3 -0
+ 	AC_MSG_RESULT([Disabling private blkid library])
+ else
+ 	LIBBLKID='$(LIB)/libblkid'$LIB_EXT
package/e2fsprogs/e2fsprogs.mk +3 -0
index 4b81b44..d16edde 100644
@@ -12,6 +12,9 @@ E2FSPROGS_LICENSE_FILES = COPYING lib/uuid/COPYING lib/ss/mit-sipb-copyright.h l
 E2FSPROGS_INSTALL_STAGING = YES
 E2FSPROGS_INSTALL_STAGING_OPTS = DESTDIR=$(STAGING_DIR) install-libs
 
+# 0002-support-findfs.patch
+E2FSPROGS_AUTORECONF = YES
+
 # e4defrag doesn't build on older systems like RHEL5.x, and we don't
 # need it on the host anyway.
 HOST_E2FSPROGS_CONF_OPTS += --disable-defrag
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#2 lore raw

RFC 1/1 e2fsprogs: support findfs

Arnout,

Thanks for the comments.

Quoted 6 lines · Arnout Vandecappelle

...

Quoted 3 lines

An alternative to this approach would be to remove any e2fsprogs's findfs references from Buildroot and add the findfs utility to util-linux (since the package provides a findfs utility as well).

Ideally, both should be done :-)

I thought this should be done as well. I initially made changes that would include util-linux's findfs and you can toggle between both. However, after I implemented a chunk of the work, I saw e2fsprog's dependency (at least in the Config.in) for util-linux/libblkid. Since util-linux build's libblkid, e2fsprog should never attempt to build the library itself (thus, cannot never support findfs). Hence why maybe findfs should be removed from Buildroot's e2fsprog package (unless I'm missing something).

Quoted 3 lines

The patch is missing a commit message.

Did you send it upstream?

Did not provide a commit message or send anything upstream. Just looking for input first before making a complete package (if any will be made).

Quoted 9 lines

package/e2fsprogs/0002-support-findfs.patch line 8

Quoted 8 lines
package/e2fsprogs/0002-support-findfs.patch new file +8 -0
+diff -Naur a/configure.in b/configure.in
+--- a/configure.in   2016-02-02 15:03:02.440059366 -0500
++++ b/configure.in   2016-02-02 15:03:38.265404484 -0500
+@@ -533,6 +533,7 @@
+             [AC_MSG_ERROR([external blkid library not found])], -luuid)
+     BLKID_CMT=#
+     PROFILED_LIBBLKID=$LIBBLKID
++    AC_DEFINE(CONFIG_BUILD_FINDFS, 1)
Quoted 7 lines

Hm, it would be weird if this were allowed... If you add this,
CONFIG_BUILD_FINDFS will be defined if --disable-libblkid was given. if --enable-libblkid was given, and if nothing is given. So the whole point of the option is moot. That doesn't sound right...

Can you check that with upstream, or consult their logs when the option was
introduced?

It doesn't; I completely agree. I feel there should be an option in there that:

1) Builds without libblkid with a limited subset.
2) Builds with an externally provided libblkid with blkid-related features.
3) Builds its "private" libblkid library with blkid-related features.

I'm no master at autoconf work, but how I viewed its current configuration file is that option 1 (partially) and 3 are only available. Although looking throughout the source code, I see a bunch of code requiring blkid-support. Maybe there is some magic that its internal implementation of findfs requires its internal variant of blkid-related code (although forcing to build with this patch, it seems to just work (limited testing)). I try to find more via old commit logs.

That all being said, I don't necessarily like the patch. It's modifying the package to get a non-Busybox findfs to work in Buildroot. I'd prefer making the alternative change to using util-linux's findfs and removing e2fsprog's findfs (which seemed to work in my limited testing). Just doing a shout out before I submit a patch request for something that might not be ideal (maybe I should have submitted an RFC for the alternative approach first).