Skip to content

Commit

Permalink
Cleanup linking
Browse files Browse the repository at this point in the history
I noticed during code review of openzfs#4385 that the author of a
commit had peppered the various Makefile.am files with `$(TIRPC_LIBS)`
when putting it into `lib/libspl/Makefile.am` should have sufficed. Upon
further examination, it seems that he had copied what we do with
`$(ZLIB)`. Unfortunately, what we do is wrong, so lets clean it up so
that we encourage future contributors to write patches that specify the
dependencies only on the things pulling them into the dependency graph.

In an ideal world, this would translate into improvements in ELF's
DT_NEEDED entries, but that is not the case because of a couple of bugs
in libtool.

The first bug is in libtool, where it overlinks by using static link
dependencies for dynamic linking:

https://wiki.mageia.org/en/Overlinking_issues_in_packaging

The workaround for this should be to pass `-Wl,--as-needed` in
`LDFLAGS`. That leads us to the second bug, where libtool passes
`LDFLAGS` after the libraries are specified and `ld` will only honor
`--as-needed` on libraries specified before it:

https://sigquit.wordpress.com/2011/02/16/why-asneeded-doesnt-work-as-expected-for-your-libraries-on-your-autotools-project/

There are a few possible workarounds for the second bug. One is to
either patch the compiler spec file to specify `-Wl,--as-needed` or pass
`-Wl,--as-needed` via `CC` like `CC='gcc -Wl,--as-needed'` so that it is
specified early. Another is to patch ltmain.sh like Gentoo does:

https://gitweb.gentoo.org/repo/gentoo.git/tree/eclass/ELT-patches/as-needed

Without one of those workarounds, this cleanup provides no benefit in
terms of DT_NEEDED entry generation. It should still be an improvement
because it nicely simplifies the code while encouraging good habits when
patching autotools scripts.

Additionally, we have multiple `-lz` and `-lblkid` passed to the
compiler because each `AC_CHECK_LIB` adds it to `$LIBS`. That is
somewhat annoying to see, so we switch to `AC_SEARCH_LIBS` to avoid it.
This is consistent with the recommendation to use `AC_SEARCH_LIBS` over
`AC_CHECK_LIB` by autotools upstream:

https://www.gnu.org/software/autoconf/manual/autoconf-2.66/html_node/Libraries.html

Signed-off-by: Richard Yao <[email protected]>
  • Loading branch information
ryao committed Mar 17, 2016
1 parent 6bb24f4 commit 47a86a4
Show file tree
Hide file tree
Showing 9 changed files with 7 additions and 16 deletions.
2 changes: 0 additions & 2 deletions cmd/zdb/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,3 @@ zdb_LDADD = \
$(top_builddir)/lib/libzpool/libzpool.la \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la

zdb_LDADD += $(ZLIB)
1 change: 0 additions & 1 deletion cmd/zfs/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@ zfs_LDADD = \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la

zfs_LDADD += $(ZLIB)
zfs_LDFLAGS = -pthread
2 changes: 0 additions & 2 deletions cmd/zhack/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,3 @@ zhack_LDADD = \
$(top_builddir)/lib/libzpool/libzpool.la \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la

zhack_LDADD += $(ZLIB)
2 changes: 0 additions & 2 deletions cmd/zstreamdump/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,3 @@ zstreamdump_LDADD = \
$(top_builddir)/lib/libzpool/libzpool.la \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la

zstreamdump_LDADD += $(ZLIB)
2 changes: 0 additions & 2 deletions cmd/ztest/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,3 @@ ztest_LDADD = \
$(top_builddir)/lib/libzpool/libzpool.la \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la

ztest_LDADD += -lm -ldl
4 changes: 2 additions & 2 deletions config/user-libuuid.m4
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ AC_DEFUN([ZFS_AC_CONFIG_USER_LIBUUID], [
AC_CHECK_HEADER([uuid/uuid.h], [], [AC_MSG_FAILURE([
*** uuid/uuid.h missing, libuuid-devel package required])])
AC_CHECK_LIB([uuid], [uuid_generate], [], [AC_MSG_FAILURE([
AC_SEARCH_LIBS([uuid_generate], [uuid], [], [AC_MSG_FAILURE([
*** uuid_generate() missing, libuuid-devel package required])])
AC_CHECK_LIB([uuid], [uuid_is_null], [], [AC_MSG_FAILURE([
AC_SEARCH_LIBS([uuid_is_null], [uuid], [], [AC_MSG_FAILURE([
*** uuid_is_null() missing, libuuid-devel package required])])
AC_SUBST([LIBUUID], ["-luuid"])
Expand Down
6 changes: 3 additions & 3 deletions config/user-zlib.m4
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ AC_DEFUN([ZFS_AC_CONFIG_USER_ZLIB], [
AC_CHECK_HEADER([zlib.h], [], [AC_MSG_FAILURE([
*** zlib.h missing, zlib-devel package required])])
AC_CHECK_LIB([z], [compress2], [], [AC_MSG_FAILURE([
AC_SEARCH_LIBS([compress2], [z], [], [AC_MSG_FAILURE([
*** compress2() missing, zlib-devel package required])])
AC_CHECK_LIB([z], [uncompress], [], [AC_MSG_FAILURE([
AC_SEARCH_LIBS([uncompress], [z], [], [AC_MSG_FAILURE([
*** uncompress() missing, zlib-devel package required])])
AC_CHECK_LIB([z], [crc32], [], [AC_MSG_FAILURE([
AC_SEARCH_LIBS([crc32], [z], [], [AC_MSG_FAILURE([
*** crc32() missing, zlib-devel package required])])
AC_SUBST([ZLIB], ["-lz"])
Expand Down
2 changes: 1 addition & 1 deletion lib/libefi/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ nodist_libefi_la_SOURCES = \
$(USER_C) \
$(KERNEL_C)

libefi_la_LIBADD = $(LIBUUID) $(ZLIB)
libefi_la_LIBADD = $(LIBUUID)

EXTRA_DIST = $(USER_C)
2 changes: 1 addition & 1 deletion lib/libzfs/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ libzfs_la_LIBADD = \
$(top_builddir)/lib/libnvpair/libnvpair.la \
$(top_builddir)/lib/libzpool/libzpool.la

libzfs_la_LIBADD += -lm -ldl $(LIBBLKID)
libzfs_la_LIBADD += -lm $(LIBBLKID)
libzfs_la_LDFLAGS = -version-info 2:0:0

EXTRA_DIST = $(libzfs_pc_DATA) $(USER_C)

0 comments on commit 47a86a4

Please sign in to comment.