Re: AM_LDFLAGS vs. maude_la_LDFLAGS

[Alexandre Duret-Lutz]

>>> "Akim" == Akim Demaille <address@hidden> writes:

 Akim> It is my understanding (after having re-read the doc ;) that these
 Akim> flags are exclusive: either one of the two is used.  

It was mine too (after having written that doc, and some bits of
code that do assume this).  Turns out I was wrong.

 Akim> So I have in a Makefile.am of mine:

 >> AM_LDFLAGS = -avoid-version -module -shared
 >> ...
 >> # Override AM_LDFLAGS: this guy must not be a module.
 >> libtcswigpy_la_LDFLAGS =

 Akim> and Makefile.in does contain:

 Akim> libtcswigpy.la: $(libtcswigpy_la_OBJECTS) $(libtcswigpy_la_DEPENDENCIES) 
 Akim> $(CXXLINK) -rpath $(pyexecdir) $(libtcswigpy_la_LDFLAGS) 
$(libtcswigpy_la_OBJECTS) $(libtcswigpy_la_LIBADD) $(LIBS)

 Akim> but CXXLINKS is:

 Akim> CXXLINK = $(LIBTOOL) --tag=CXX $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) \
 Akim> --mode=link $(CXXLD) $(AM_CXXFLAGS) $(CXXFLAGS) $(AM_LDFLAGS) \
 Akim> $(LDFLAGS) -o $@

 Akim> so I'm stuck with AM_LDFLAGS although I meant to escape from it.  Of
 Akim> course I'll get rid of AM_LDFLAGS, but I'm not sure I was an outlaw
 Akim> according to the documentation.

Also, if you were using libtcswigpy_la_CXXFLAGS, the above code
would simply ignore it and use AM_CXXFLAGS instead.  This
undoubtedly is an error.

Whether using libtcswigpy_la_LDFLAGS in addition to AM_LDFLAGS
is an error I don't know.  Apparently the two variable were
always used this way since the introduction of AM_LDFLAGS in
Automake 1.5.  So from that point of view what is wrong is the
documentation that I have added.

OTOH, the current behavior of AM_LDFLAGS vs. maude_LDFLAGS
sounds very counterintuitive to me.  Even if it breaks some
package, I'd rather fix this to match the handling of the other
flag variables than document and carry over this exception.

The appended proposed patch (of which only the FCFLAGS chunk
would go on branch-1-9) summarizes what breaks in the NEWS
chunk.  Does anybody feel strongly against this?

 Akim> And shouldn't the LDFLAGS be last?  It is in CXXLINKS, but in the
 Akim> command line.

Fixed in the patch below as a side-effect of using maude_LDFLAGS
instead of AM_LDFLAGS.


2004-12-26  Alexandre Duret-Lutz  <address@hidden>

        Fix handling of per-target flags in link rules.
        * automake.in (define_per_target_linker_variable): New function.
        (handle_programs, handle_ltlibraries): Use it.
        (%link_languages): New map.
        (register_language): Fill it.
        (Preprocessed Fortran): Fix the definition of FCLINK.
        * lib/am/ltlibrary.am (%LTLIBRARY%): Do not append
        $(%XLTLIBRARY%_LDFLAGS) to the command, this is now done by
        define_per_target_linker_variable if needed.
        * lib/am/program.am (%PROGRAM%%EXEEXT%): Likewise with
        $(%XPROGRAM%_LDFLAGS).
        * doc/automake.texi (Program and Library Variables): Mention
        AM_LDFLAGS and AM_LIBTOOLFLAGS in the definition of maude_LDFLAGS
        and maude_LIBTOOLFLAGS.
        * tests/libtool9.test: New file.
        * tests/Makefile.am (TESTS): Add it.
        * NEWS: Explain the backward incompatibility.
        Report from Akim Demaille.

Index: NEWS
===================================================================
RCS file: /cvs/automake/automake/NEWS,v
retrieving revision 1.291
diff -u -r1.291 NEWS
--- NEWS        12 Dec 2004 23:39:01 -0000      1.291
+++ NEWS        26 Dec 2004 23:20:57 -0000
@@ -18,6 +18,37 @@
     can be specified using AM_LIBTOOLFLAGS and target_LIBTOOLFLAGS.
 
   - aclocal now also supports -Wmumble and -Wno-mumble options.
+
+  - Per-target flags are now correctly handled in link rules.
+
+    For instance maude_CFLAGS correctly overrides AM_CFLAGS; likewise
+    for maude_LDFLAGS and AM_LDFLAGS.  Previous versions bogusly
+    preferred AM_CFLAGS over maude_CFLAGS while linking, and they
+    used both AM_LDFLAGS and maude_LDFLAGS on the same link command.
+
+    The fix for compiler flags (i.e., using maude_CFLAGS instead of
+    AM_CFLAGS) should not hurt any package since that is how _CFLAGS
+    is expected to work (and actually works during compilation).
+
+    However using maude_LDFLAGS "instead of" AM_LDFLAGS rather than
+    "in addition to" breaks backward compatibility with older versions.
+    If your package used both variables, as in
+
+      AM_LDFLAGS = common flags
+      bin_PROGRAMS = a b c
+      a_LDFLAGS = more flags
+      ...
+
+    and assumed *_LDFLAGS would sum up, you should rewrite it as
+
+      AM_LDFLAGS = common flags
+      bin_PROGRAMS = a b c
+      a_LDFLAGS = $(AM_LDFLAGS) more flags
+      ...
+
+    This new behavior of *_LDFLAGS is more coherent with other
+    per-target variables, and the way *_LDFLAGS variables were
+    considered internally.
 
 New in 1.9:
 
Index: automake.in
===================================================================
RCS file: /cvs/automake/automake/automake.in,v
retrieving revision 1.1591
diff -u -r1.1591 automake.in
--- automake.in 12 Dec 2004 23:39:01 -0000      1.1591
+++ automake.in 26 Dec 2004 23:20:58 -0000
@@ -403,6 +403,8 @@
 
 # This maps languages names onto objects.
 my %languages = ();
+# Maps each linker variable onto a language object.
+my %link_languages = ();
 
 # List of targets we must always output.
 # FIXME: Complete, and remove falsely required targets.
@@ -864,7 +866,7 @@
                   'Name' => 'Preprocessed Fortran',
                   'config_vars' => ['FC'],
                   'linker' => 'FCLINK',
-                  'link' => '$(FCLD) $(AM_FFLAGS) $(FCFLAGS) $(AM_LDFLAGS) 
$(LDFLAGS) -o $@',
+                  'link' => '$(FCLD) $(AM_FCFLAGS) $(FCFLAGS) $(AM_LDFLAGS) 
$(LDFLAGS) -o $@',
                   'lder' => 'FCLD',
                   'ld' => '$(FC)',
                   'flags' => ['FCFLAGS', 'CPPFLAGS'],
@@ -1923,7 +1925,7 @@
     # object extension.
     my ($one_file, $unxformed, $obj, $where, %transform) = @_;
 
-    my ($linker) = '';
+    my $linker = '';
 
     # No point in continuing if _OBJECTS is defined.
     return if reject_var ($one_file . '_OBJECTS',
@@ -2353,15 +2355,7 @@
       set_seen ($xname . '_LDFLAGS');
 
       # Determine program to use for link.
-      my $xlink;
-      if (var ($xname . '_LINK'))
-       {
-         $xlink = $xname . '_LINK';
-       }
-      else
-       {
-         $xlink = $linker ? $linker : 'LINK';
-       }
+      my $xlink = &define_per_target_linker_variable ($linker, $xname);
 
       # If the resulting program lies into a subdirectory,
       # make sure this directory will exist.
@@ -2648,15 +2642,7 @@
                                             NONLIBTOOL => 0, LIBTOOL => 1);
 
       # Determine program to use for link.
-      my $xlink;
-      if (var ($xlib . '_LINK'))
-       {
-         $xlink = $xlib . '_LINK';
-       }
-      else
-       {
-         $xlink = $linker ? $linker : 'LINK';
-       }
+      my $xlink = &define_per_target_linker_variable ($linker, $xlib);
 
       my $rpathvar = "am_${xlib}_rpath";
       my $rpath = "\$($rpathvar)";
@@ -5539,6 +5525,20 @@
   # Fill indexes.
   $extension_map{$_} = $lang->name foreach @{$lang->extensions};
   $languages{$lang->name} = $lang;
+  my $link = $lang->linker;
+  if ($link)
+    {
+      if (exists $link_languages{$link})
+       {
+         prog_error ("`$link' has different definitions in "
+                     . $lang->name . " and " . $link_languages{$link}->name)
+           if $lang->link ne $link_languages{$link}->link;
+       }
+      else
+       {
+         $link_languages{$link} = $lang;
+       }
+    }
 
   # Update the pattern of known extensions.
   accept_extensions (@{$lang->extensions});
@@ -5786,7 +5786,6 @@
 {
     my ($lang) = @_;
 
-    my ($var, $value) = ($lang->lder, $lang->ld);
     my $libtool_tag = '';
     $libtool_tag = '--tag=' . $lang->libtool_tag . ' '
       if $lang->libtool_tag && exists $libtool_tags{$lang->libtool_tag};
@@ -5801,6 +5800,53 @@
                      INTERNAL);
 }
 
+sub define_per_target_linker_variable ($$)
+{
+  my ($linker, $target) = @_;
+
+  # If the user wrote a custom link command, we don't define ours.
+  return "${target}_LINK"
+    if set_seen "${target}_LINK";
+
+  my $xlink = $linker ? $linker : 'LINK';
+
+  my $lang = $link_languages{$xlink};
+  prog_error "Unknown language for linker variable `$xlink'"
+    unless $lang;
+
+  my $link_command = $lang->link;
+  if (var 'LIBTOOL')
+    {
+      my $libtool_tag = '';
+      $libtool_tag = '--tag=' . $lang->libtool_tag . ' '
+       if $lang->libtool_tag && exists $libtool_tags{$lang->libtool_tag};
+
+      $link_command =
+       "\$(LIBTOOL) $libtool_tag\$(AM_LIBTOOLFLAGS) \$(LIBTOOLFLAGS) "
+       . "--mode=link " . $link_command;
+    }
+
+  # Rewrite each occurrence of `AM_$flag' in the link
+  # command into `${derived}_$flag' if it exists.
+  my $orig_command = $link_command;
+  my @flags = (@{$lang->flags}, 'LDFLAGS');
+  push @flags, 'LIBTOOLFLAGS' if var 'LIBTOOL';
+  for my $flag (@flags)
+    {
+      my $val = "${target}_$flag";
+      $link_command =~ s/\(AM_$flag\)/\($val\)/
+       if set_seen ($val);
+    }
+
+  # If the computed command is the same as the generic command, use
+  # the command linker variable.
+  return $lang->linker
+    if $link_command eq $orig_command;
+
+  &define_variable ("${target}_LINK", $link_command, INTERNAL);
+  return "${target}_LINK";
+}
+
 ################################################################
 
 # &check_trailing_slash ($WHERE, $LINE)
Index: doc/automake.texi
===================================================================
RCS file: /cvs/automake/automake/doc/automake.texi,v
retrieving revision 1.81
diff -u -r1.81 automake.texi
--- doc/automake.texi   18 Dec 2004 17:54:28 -0000      1.81
+++ doc/automake.texi   26 Dec 2004 23:21:00 -0000
@@ -3600,10 +3600,11 @@
 
 @item maude_LDFLAGS
 This variable is used to pass extra flags to the link step of a program
-or a shared library.
+or a shared library.  It overrides the global @code{AM_LDFLAGS} variable.
 
 @item maude_LIBTOOLFLAGS
 This variable is used to pass extra options to @command{libtool}.
+It overrides the global @code{AM_LIBTOOLFLAGS} variable.
 These options are output before @command{libtool}'s @code{--mode=MODE}
 option, so they should not be mode-specific options (those belong to
 the compiler or linker flags).  @xref{Libtool Flags}.
Index: lib/am/ltlibrary.am
===================================================================
RCS file: /cvs/automake/automake/lib/am/ltlibrary.am,v
retrieving revision 1.6
diff -u -r1.6 ltlibrary.am
--- lib/am/ltlibrary.am 2 Jun 2003 07:08:40 -0000       1.6
+++ lib/am/ltlibrary.am 26 Dec 2004 23:21:00 -0000
@@ -1,5 +1,5 @@
 ## automake - create Makefile.in from Makefile.am
-## Copyright (C) 1994, 1995, 1996, 2003 Free Software Foundation, Inc.
+## Copyright (C) 1994, 1995, 1996, 2003, 2004 Free Software Foundation, Inc.
 
 ## This program is free software; you can redistribute it and/or modify
 ## it under the terms of the GNU General Public License as published by
@@ -16,4 +16,4 @@
 ## Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
 ## 02111-1307, USA.
 %LTLIBRARY%: $(%XLTLIBRARY%_OBJECTS) $(%XLTLIBRARY%_DEPENDENCIES) %DIRSTAMP%
-       $(%XLINK%) %RPATH% $(%XLTLIBRARY%_LDFLAGS) $(%XLTLIBRARY%_OBJECTS) 
$(%XLTLIBRARY%_LIBADD) $(LIBS)
+       $(%XLINK%) %RPATH% $(%XLTLIBRARY%_OBJECTS) $(%XLTLIBRARY%_LIBADD) 
$(LIBS)
Index: lib/am/program.am
===================================================================
RCS file: /cvs/automake/automake/lib/am/program.am,v
retrieving revision 1.24
diff -u -r1.24 program.am
--- lib/am/program.am   2 Jun 2003 07:08:40 -0000       1.24
+++ lib/am/program.am   26 Dec 2004 23:21:00 -0000
@@ -1,5 +1,5 @@
 ## automake - create Makefile.in from Makefile.am
-## Copyright (C) 1994, 1995, 1996, 1997, 2001, 2003
+## Copyright (C) 1994, 1995, 1996, 1997, 2001, 2003, 2004
 ## Free Software Foundation, Inc.
 
 ## This program is free software; you can redistribute it and/or modify
@@ -23,4 +23,4 @@
 ## Or maybe not... sadly, incremental linkers are rarer than losing
 ## systems.
        @rm -f %PROGRAM%%EXEEXT%
-       $(%XLINK%) $(%XPROGRAM%_LDFLAGS) $(%XPROGRAM%_OBJECTS) 
$(%XPROGRAM%_LDADD) $(LIBS)
+       $(%XLINK%) $(%XPROGRAM%_OBJECTS) $(%XPROGRAM%_LDADD) $(LIBS)
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.576
diff -u -r1.576 Makefile.am
--- tests/Makefile.am   9 Dec 2004 22:07:45 -0000       1.576
+++ tests/Makefile.am   26 Dec 2004 23:21:00 -0000
@@ -302,6 +302,7 @@
 libtool6.test \
 libtool7.test \
 libtool8.test \
+libtool9.test \
 license.test \
 link_c_cxx.test        \
 link_dist.test \
Index: tests/Makefile.in
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.in,v
retrieving revision 1.747
diff -u -r1.747 Makefile.in
--- tests/Makefile.in   9 Dec 2004 22:07:45 -0000       1.747
+++ tests/Makefile.in   26 Dec 2004 23:21:00 -0000
@@ -421,6 +421,7 @@
 libtool6.test \
 libtool7.test \
 libtool8.test \
+libtool9.test \
 license.test \
 link_c_cxx.test        \
 link_dist.test \
Index: tests/libtool9.test
===================================================================
RCS file: tests/libtool9.test
diff -N tests/libtool9.test
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/libtool9.test 26 Dec 2004 23:21:00 -0000
@@ -0,0 +1,107 @@
+#! /bin/sh
+# Copyright (C) 2004  Free Software Foundation, Inc.
+#
+# This file is part of GNU Automake.
+#
+# GNU Automake is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# GNU Automake is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Automake; see the file COPYING.  If not, write to
+# the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+
+# Make sure xxx_LINK is defined for each target that requires specific
+# flags.
+# Quite similar to libtool7.test, using AM_LDFLAGS in addition to xxx_LDFLAGS.
+
+required='libtoolize gcc'
+. ./defs || exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_PROG_CC
+AM_PROG_CC_C_O
+AC_LIBTOOL_DLOPEN
+AM_PROG_LIBTOOL
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+AM_LDFLAGS = -module
+lib_LTLIBRARIES = libmod1.la mod2.la
+libmod1_la_SOURCES = mod1.c
+libmod1_la_LDFLAGS =
+libmod1_la_LIBADD = -dlopen mod2.la
+mod2_la_SOURCES = mod2.c
+
+bin_PROGRAMS = prg prg2
+prg_SOURCES = prg.c
+prg_LDADD = -dlopen libmod1.la -dlpreopen mod2.la
+prg_CPPFLAGS = -DXYZ=1
+prg2_SOURCES = prg.c
+prg2_CFLAGS =
+
+print:
+       @echo 1BEG: $(prg_DEPENDENCIES) :END1
+       @echo 2BEG: $(libmod1_la_DEPENDENCIES) :END2
+       @echo 3BEG: $(libmod1_la_LINK) :END3
+       @echo 4BEG: $(mod2_la_LINK) :END4
+       @echo 5BEG: $(prg_LINK) :END5
+       @echo 6BEG: $(prg2_LINK) :END6
+
+END
+
+mkdir liba
+
+cat > mod1.c << 'END'
+int
+mod1 ()
+{
+   return 1;
+}
+END
+
+cat > mod2.c << 'END'
+int
+mod2 ()
+{
+   return 2;
+}
+END
+
+cat > prg.c << 'END'
+int
+main ()
+{
+   return 0;
+}
+END
+
+libtoolize --force --copy
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE --add-missing --copy
+
+./configure
+env LDFLAGS=ldflags AM_LDFLAGS=am_ldflags libmod1_la_LDFLAGS=lm1_la_ldflags \
+    CFLAGS=cflags AM_CFLAGS=am_cflags prg2_CFLAGS=prg2_cflags \
+    $MAKE -e print >output 2>&1
+cat output
+grep '1BEG: libmod1.la mod2.la :END1' output
+grep '2BEG: mod2.la :END2' output
+grep '3BEG:.* am_cflags cflags .*lm1_la_ldflags ldflags.* :END3' output
+grep '3BEG: .*am_ldflags.* :END3' output && exit 1
+grep '4BEG: :END4' output
+grep '5BEG: :END5' output
+grep '6BEG:.* prg2_cflags cflags .*am_ldflags ldflags.* :END6' output
+grep '6BEG: .*am_cflags.* :END6' output && exit 1
+$MAKE

-- 
Alexandre Duret-Lutz