Makefile.am: Fix underlinking #50

Merged
merged 1 commit into from Nov 29, 2016

Projects

None yet

3 participants

@sardemff7
Contributor

No description provided.

@achadwick achadwick and 2 others commented on an outdated diff Jun 9, 2016
tests/gegl/Makefile.am
$(top_builddir)/libmypaint.la \
- $(top_builddir)/gegl/libmypaint-gegl.la \
- $(top_builddir)/tests/libmypaint-tests.a
+ $(top_builddir)/gegl/libmypaint-gegl.la
@achadwick
achadwick Jun 9, 2016 Member

Does ordering matter here?

@sardemff7
sardemff7 Jun 9, 2016 Contributor

Yes. Try adding -Wl,--no-undefined to your `LDFLAGS’ and test with and without this patch. :-)

@QuLogic
QuLogic Jun 9, 2016 Member

Strictly speaking, shouldn't $(GEGL_LIBS) be last?

@sardemff7
sardemff7 Jun 9, 2016 Contributor

Oh, strictly speaking, yes.

@achadwick
Member

Hi there - what underlinking problem does this solve? We already use LIBS = @LIBS since 78ac562 and -no-undefined since bf3cd0c.

$ ldd -r .libs/libmypaint.so >/dev/null
[no output]

But that's only the "direct" case, of course.

@sardemff7
Contributor

Here are the logs:
without the patch
with the first part (without the reordering)
I use -Wl,--as-needed too, and note that -no-undefined is not very strong, libtool often ignore it.

You’re right about $(LIBS) but it is good practice to avoid putting everything in that an relying on the default values. *_LIBADD (and not *_LIBS, nor *_LDADD since you are building libs here) is cleaner.

Also, LIBS contains -lm as you use AC_SEARCH_LIBS.

I started an Autotools usage cleanup, but you do use a lot of shortcuts, so a simple fix will do for now. :-)

@sardemff7 sardemff7 Makefile.am: Fix underlinking
Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
00b0c38
@achadwick achadwick merged commit 3ed8f6a into mypaint:master Nov 29, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment