[xfstests-bld] build-all: fix acl tools build
diff mbox series

Message ID 20190920184846.18251-1-amir73il@gmail.com
State New
Headers show
Series
  • [xfstests-bld] build-all: fix acl tools build
Related show

Commit Message

Amir Goldstein Sept. 20, 2019, 6:48 p.m. UTC
On a clean do-all buster image getfacl is dynamically
linked with libacl.so.1, but the built library is not installed.
The installed libacl.so.1 in the image is incompatible with the
version of acl tools that we build:

root@kvm-xfstests:~# getfacl -n -p xfstests/
getfacl: symbol lookup error: getfacl: undefined symbol: walk_tree

Fix this by linking acl tools with the static libacl library.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 build-all | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Amir Goldstein Sept. 20, 2019, 6:51 p.m. UTC | #1
CC: Eryu's correct email address

On Fri, Sep 20, 2019 at 9:48 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On a clean do-all buster image getfacl is dynamically
> linked with libacl.so.1, but the built library is not installed.
> The installed libacl.so.1 in the image is incompatible with the
> version of acl tools that we build:
>
> root@kvm-xfstests:~# getfacl -n -p xfstests/
> getfacl: symbol lookup error: getfacl: undefined symbol: walk_tree
>
> Fix this by linking acl tools with the static libacl library.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  build-all | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/build-all b/build-all
> index 217ae62..83a1394 100755
> --- a/build-all
> +++ b/build-all
> @@ -233,12 +233,15 @@ fi
>
>  if test -z "$SKIP_ACL" ; then
>      build_start "ACL library"
> +    # Specify NO_SOLIB=1 so that libacl is only built as a static
> +    # library.  Then acl tools will be statically linked to it, and we won't
> +    # have to install libacl1.so.
>      (cd acl; \
>       CPPFLAGS="-I$DESTDIR/include" \
>       CFLAGS="$LCF -I$DESTDIR/include" \
>       LDFLAGS="$LLF $EXEC_LDFLAGS -L$DESTDIR/lib" \
>       ./configure $cross --prefix=$DESTDIR --disable-nls; $MAKE_CLEAN ; \
> -     make $J LDFLAGS="$LLF $EXEC_LLDFLAGS -static -L$DESTDIR/lib" ; \
> +     make $J NO_SOLIB=1 LDFLAGS="$LLF $EXEC_LLDFLAGS -static -L$DESTDIR/lib" ; \
>       make $J install)
>  fi
>
> --
> 2.17.1
>
Theodore Y. Ts'o Sept. 20, 2019, 11:41 p.m. UTC | #2
On Fri, Sep 20, 2019 at 09:51:18PM +0300, Amir Goldstein wrote:
> CC: Eryu's correct email address

You probably don't need to explicitly cc Eryu, since he's reviewing
xfstests-bld changes.

For that matter, if people are getting annoyed because there are too
many xfstests-bld/{kvm,gce,android}-xfstests changes getting sent to
fstests, please let me know and we can use some other list for these
reviews.

				- Ted
Theodore Y. Ts'o Sept. 21, 2019, 2:13 p.m. UTC | #3
On Fri, Sep 20, 2019 at 09:48:46PM +0300, Amir Goldstein wrote:
> On a clean do-all buster image getfacl is dynamically
> linked with libacl.so.1, but the built library is not installed.
> The installed libacl.so.1 in the image is incompatible with the
> version of acl tools that we build:

So I did a clean build of a4df7d7b3112: "Update the default version of
quota and xfsprogs" in buster, and the resulting getfacl executable
does not depend on libacl.so.1:

<tytso@stretch-amd64-CHROOT.lambda> {/tmp/xfstests-bld}
1002% ldd xfstests/bin/getfacl 
	linux-vdso.so.1 (0x00007ffeafbc4000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fd8cf9a0000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fd8cff47000)

Is there something in your config.custom which is influencing the
reseult?  I'm also not seeing any file in the acl directory, after the
build, which contains NO_SOLIB:

<tytso@stretch-amd64-CHROOT.lambda> {/tmp/xfstests-bld/acl}
1004% find . -type f -print | xargs grep NO_SOLIB

The git repository for keyutils does respect NO_SOLIB, but that's
because it's hard-coded into keyutils's Makefile.  So it doesn't
appear to me how your change would make any difference to how the acl
library is built.

Can you take a closer look?

					- Ted
Amir Goldstein Sept. 21, 2019, 3:37 p.m. UTC | #4
On Sat, Sep 21, 2019 at 5:13 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Fri, Sep 20, 2019 at 09:48:46PM +0300, Amir Goldstein wrote:
> > On a clean do-all buster image getfacl is dynamically
> > linked with libacl.so.1, but the built library is not installed.
> > The installed libacl.so.1 in the image is incompatible with the
> > version of acl tools that we build:
>
> So I did a clean build of a4df7d7b3112: "Update the default version of
> quota and xfsprogs" in buster, and the resulting getfacl executable
> does not depend on libacl.so.1:
>
> <tytso@stretch-amd64-CHROOT.lambda> {/tmp/xfstests-bld}
> 1002% ldd xfstests/bin/getfacl
>         linux-vdso.so.1 (0x00007ffeafbc4000)
>         libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fd8cf9a0000)
>         /lib64/ld-linux-x86-64.so.2 (0x00007fd8cff47000)
>
> Is there something in your config.custom which is influencing the
> reseult?  I'm also not seeing any file in the acl directory, after the
> build, which contains NO_SOLIB:
>
> <tytso@stretch-amd64-CHROOT.lambda> {/tmp/xfstests-bld/acl}
> 1004% find . -type f -print | xargs grep NO_SOLIB
>
> The git repository for keyutils does respect NO_SOLIB, but that's
> because it's hard-coded into keyutils's Makefile.  So it doesn't
> appear to me how your change would make any difference to how the acl
> library is built.
>
> Can you take a closer look?
>

OK. You are right of course, that the change is wrong and not needed.
Sorry for the noise.
I now wonder how did libacl.so.1 end up in bld/lib the first time I built...

I did try a do-update build after do-all, which may have caused it.

There is one issue with update builds that has been bothering me forever
and has to do with xfstests and xfsprogs build, so while we are on fstests
list I will mention that and maybe you can advise.

Unlike do-all, do-update does 'make' without 'configure', but when
configure.ac changes, and that happens every xfsprogs release,
xfsprogs as well as xfstests Makefile runs an unsolicited ./configure
without all the config parameters that were used for the first
xfstests-bld build and then build fails.

Is there a standard way to run ./configure in "olddefconfig" mode, rather
than "defconfig" mode, for the case that configure.ac changes?

Thanks,
Amir.
Theodore Y. Ts'o Sept. 21, 2019, 9:35 p.m. UTC | #5
On Sat, Sep 21, 2019 at 06:37:05PM +0300, Amir Goldstein wrote:
> There is one issue with update builds that has been bothering me forever
> and has to do with xfstests and xfsprogs build, so while we are on fstests
> list I will mention that and maybe you can advise.
> 
> Unlike do-all, do-update does 'make' without 'configure', but when
> configure.ac changes, and that happens every xfsprogs release,
> xfsprogs as well as xfstests Makefile runs an unsolicited ./configure
> without all the config parameters that were used for the first
> xfstests-bld build and then build fails.

I rarely use do-update these days, now that have ccache configured for
my build chroots.  The primary case where I use it is when making
minor changes to xfstests (e.g., changing some source file in
xfstests/src which requires a recompile).  I used to use it when
updating or creating a new test, but now we have "{kvm,gce}-xfstests
--update-xfstests-tar" which is even faster.

If anything happens which requires configure to be run, I just use
"./do-all" and be done with it.  After all, i'm not doing all that
often.

The reason why the Makefiles in xfstests and xfsprogs are running an
unsolicity ./configure without all the config parameter can be fixed.
An example of how to do this can be found in e2fsprogs, where in the
MCONFIG.in file we have these lines:

$(top_srcdir)/configure: $(top_srcdir)/configure.ac
	cd $(top_srcdir) && autoheader && autoconf

$(top_builddir)/config.status: $(top_srcdir)/configure
	cd $(top_builddir); ./config.status --recheck

$(top_builddir)/MCONFIG: $(top_srcdir)/MCONFIG.in $(top_builddir)/config.status
	cd $(top_builddir); CONFIG_FILES=MCONFIG ./config.status

and we have similar lines for all files generated by configure, for
example:

Makefile: $(srcdir)/Makefile.in $(top_builddir)/MCONFIG \
		$(DEP_MAKEFILE) $(top_builddir)/config.status
	cd $(top_builddir); CONFIG_FILES=$(my_dir)/Makefile ./config.status

The basic idea is that when configure.ac changes, we build configure,
and then we have config.status in the build tree depend on configure
--- and we use config.status --recheck to rerun the configure script
with all of the existing configure script options.  We then similarly
use dependencies on config.status to rebuild all of the generated
files.

Whether or not it's worth it to set up a similar system for xfsprogs
and xfstests, and whether their respective maintainers would accept a
change is a different question.

I suppose you could try to teach the build-all script in xfstests-bld
when xfsprogs and xfstests might do an automatic run of ./configure in
build-all --update mode, and in that case (and only that case, since
the goal is to make --update be fast), have it run "./config.status
--recheck ; ./config.status".

It's not clear to me it's worth it, since I don't pull updates of
xfstests from upstream more than every week or two, and a full build
only takes around 12 minutes.  And for the much more common case where
I'm tweaking a test or creating a new test, "--update-xfstests-tar" is
sufficient, and even faster than rebuilding a new KVM image using
./do-update.

Cheers,

						- Ted

Patch
diff mbox series

diff --git a/build-all b/build-all
index 217ae62..83a1394 100755
--- a/build-all
+++ b/build-all
@@ -233,12 +233,15 @@  fi
 
 if test -z "$SKIP_ACL" ; then
     build_start "ACL library"
+    # Specify NO_SOLIB=1 so that libacl is only built as a static
+    # library.  Then acl tools will be statically linked to it, and we won't
+    # have to install libacl1.so.
     (cd acl; \
      CPPFLAGS="-I$DESTDIR/include" \
      CFLAGS="$LCF -I$DESTDIR/include" \
      LDFLAGS="$LLF $EXEC_LDFLAGS -L$DESTDIR/lib" \
      ./configure $cross --prefix=$DESTDIR --disable-nls; $MAKE_CLEAN ; \
-     make $J LDFLAGS="$LLF $EXEC_LLDFLAGS -static -L$DESTDIR/lib" ; \
+     make $J NO_SOLIB=1 LDFLAGS="$LLF $EXEC_LLDFLAGS -static -L$DESTDIR/lib" ; \
      make $J install)
 fi