search

ericmckean / snappy

Automatically exported from code.google.com/p/snappy
Other
0 stars 0 forks source link

configure.ac: EXTRA_LIBSNAPPY_LDFLAGS is possibly redundant #5

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
The dance with EXTRA_LIBSNAPPY_LDFLAGS seems odd. It'd be more conventional to 
do something like:

    SNAPPY_LTVERSION=1:0:0
    AC_SUBST([SNAPPY_LTVERSION])

and set this directly in libsnappy_la_LDFLAGS in Makefile.am. I'm not sure why 
it has been defined with m4_define.

If it's to allow the user to provide custom LDFLAGS, it's unnecessary: LDFLAGS 
is part of libsnappy_la_LINK. Here's the (generated) snippet from Makefile.in:

    libsnappy_la_LINK = $(LIBTOOL) --tag=CXX $(AM_LIBTOOLFLAGS) \
            $(LIBTOOLFLAGS) --mode=link $(CXXLD) $(AM_CXXFLAGS) \
            $(CXXFLAGS) $(libsnappy_la_LDFLAGS) $(LDFLAGS) -o $@

Original issue reported on code.google.com by endgame....@gmail.com on 22 Mar 2011 at 11:38

GoogleCodeExporter commented 9 years ago 
Hm. I tried your configure.ac entry, but how do I refer to it in Makefile.am? 
If I do

  libsnappy_la_LDFLAGS = -version-info $(SNAPPY_LTVERSION)

I seem to get an empty variable substituted (and the build fails).

Original comment by sgunder...@bigfoot.com on 22 Mar 2011 at 11:56

GoogleCodeExporter commented 9 years ago 
The call to AC_SUBST must appear after AC_INIT.

Also, because you don't use the shell variable for anything, you could just use 
AC_SUBST([SNAPPY_LTVERSION], [1:0:0]).

Original comment by endgame....@gmail.com on 23 Mar 2011 at 12:06

GoogleCodeExporter commented 9 years ago 
Yes, AC_SUBST was after AC_INIT. It's a bit strange it didn't work, but I made 
a workaround like this:

m4_define([snappy_ltversion], [1:0:0])

[...]

AC_SUBST([SNAPPY_LTVERSION], snappy_ltversion)

Original comment by se...@google.com on 23 Mar 2011 at 12:36

GoogleCodeExporter commented 9 years ago 
That's interesting, because with the attached patch, I do not see this problem.

The patch does not contain the changes to various autogenerated files. BTW:

$ autoconf --version
autoconf (GNU Autoconf) 2.67
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+/Autoconf: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>, <http://gnu.org/licenses/exceptions.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by David J. MacKenzie and Akim Demaille.
$ automake --version
automake (GNU automake) 1.11.1
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv2+: GNU GPL version 2 or later 
<http://gnu.org/licenses/gpl-2.0.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Tom Tromey <tromey@redhat.com>
       and Alexandre Duret-Lutz <adl@gnu.org>.
$ libtool --version
ltmain.sh (GNU libtool) 2.2.6b
Written by Gordon Matzigkeit <gord@gnu.ai.mit.edu>, 1996

Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Original comment by endgame....@gmail.com on 23 Mar 2011 at 12:41

Attachments:

GoogleCodeExporter commented 9 years ago 
Yes, with AC_SUBST directly to a value, it works for me too. The problem 
happens if I set the variable before AC_INIT (since I want all the versioning 
together at the top) and then try to AC_SUBST it at the bottom.

In any case, the patch should be forthcoming.

Original comment by se...@google.com on 23 Mar 2011 at 1:02

GoogleCodeExporter commented 9 years ago 
Fixed in r8.

Original comment by se...@google.com on 23 Mar 2011 at 11:13