Hello Matieu,
good news: I found a solution.
Am 19.04.2017 um 12:54 schrieb Mathieu Lirzin:
Hello,
Thomas Martitz <ku...@rockbox.org> writes:
Am 13.04.2017 um 14:59 schrieb Mathieu Lirzin:
I have not spent a long time digging through the mailing list archive,
There seem to have a lot of mail related to subdir-objects past bugs
which makes it hard to find the information I am looking for. However I
have found some questions which relates to the change you are proposing:
https://lists.gnu.org/archive/html/automake/2013-02/msg00051.html
https://lists.gnu.org/archive/html/automake/2003-11/msg00104.html
Thanks for taking your time and digging through the archives. It looks
like there is a clear demand for the change (see also Peter Rosin's
reply).
I agree that the way long object names appears can be confusing for
Automake users.
Thomas Martitz <ku...@rockbox.org> writes:
[...]
But with subdir-objects this is unecessary, since uniqueness of the
object file names
is already achieved by placing them next to the unique source files.
Unfornately, I have found an example which seems to contradict that
assumption.
Makefile.am:
AUTOMAKE_OPTIONS = subdir-objects foreign
noinst_PROGRAMS = foo
foo_SOURCES = src/foo.c
foo_CPPFLAGS = -DVAL=0
include src/local.mk
src/local.mk:
noinst_PROGRAMS += src/foo
src_foo_CPPFLAGS = -DVAL=1
src_foo_SOURCES = src/foo.c
src/foo.c:
int
main ()
{
return VAL;
}
With the current behavior both "src/foo-foo.o" and "src/src_foo-foo.o"
are produced which allows the two executables to refer to the correct
VAL. However with the change you are proposing, only "src/foo-foo.o" is
produced which is then used for both executables and make them return
the same VAL.
See attached patch for more details. It adds a test which passes on
current 'minor' branch but fails when applied on top of your patch. The
intent is to allow you to reproduce the issue.
The case you have brought up pretty much does the same as using %C%
inside local.mk. It's just that you manually applied src_ and src/
prefixes manually.
Therefore, yes, this is affected by my change. The code where my
change is located doesn't really know whether %reldir%-feature was
used, or if prefixes have been manually assigned. That code only sees
the already-expanded variables.
I have to say that your example seems pretty academic to me. I doubt
that anyone builds two equally named binaries in practice. Normally
you would build programs with different names, in which case this
effect doesn't occur, wouldn't you?
But please be sure that understand that you take this seriously.
Out of context a lot of things can be seen as weird, but since having
programs with the same name in different directories has been considered
valid by Automake for ages, we should expect some users to rely on that
behavior, and we don't want to break it.
I don't know if this is feasible to only use long names when there is an
executable or library basename clash. I suppose this would not be trivial to
implement. WDYT?
I will certainly have a look at your suggestion.
I have implemented a solution. Basically the file name truncation is
disabled if the are programs or libs that have identical names. Please
tell me if that's sufficient.
Best ragerds.
>From 8cd0a71abac70bbdcaac804dc7964fa9f34753a0 Mon Sep 17 00:00:00 2001
From: Thomas Martitz <ku...@rockbox.org>
Date: Mon, 13 Mar 2017 12:41:59 +0100
Subject: [PATCH 1/3] Shorter object file names under subdir-objects
With the %reldir% feature, object file names can become very long, because the
file names are prefixed with the %canon_reldir% substitution. The reason is to
achieve unique object file names when target-specific CFLAGS or similar are
used. When subdir-objects is also in effect, these long file names are also
placed in potentially deep subdirectories.
But with subdir-objects this is unnecessary, since uniqueness of the object
file names is already achieved by placing them next to the unique source files.
Therefore, this changes strips paths components, that are caused by
%canon_reldir% or otherwise, from the object file names. The object file name
is prefixed by the target in case of target-specific CFLAGS. As a result, the
build tree looks less scary and many cases where $var_SHORTNAME was necessary
can now be avoided. Remember that the use of $var_SHORTNAME is discouraged (and
is not always an option since it does not work inside conditionals).
There is one exception to the above: if the same source file is linked to
separate programs/libraries with per-executable flags and the
programs/libraries have identical names, then uniqueness of truncated object
file names is broken. Therefore the truncation is prevented for these targets
if there happens to be identically named progams or libraries,
Example:
previously:
sub/Makefile.am:
AUTOMAKE_OPTIONS = subdir-objects
bin_PROGRAMS += %D%/foo
%C%_foo_CFLAGS = $(AM_CFLAGS) -g
resulted in objects:
sub/sub_foo-foo.o
now object file name is:
sub/foo-foo.o
---
NEWS | 6 +++
bin/automake.in | 90 ++++++++++++++++++++++++++++++++--
t/list-of-tests.mk | 1 +
t/subdir-objects-objname.sh | 115 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 207 insertions(+), 5 deletions(-)
create mode 100644 t/subdir-objects-objname.sh
diff --git a/NEWS b/NEWS
index af904d442..a1f4f37c1 100644
--- a/NEWS
+++ b/NEWS
@@ -64,6 +64,12 @@
New in 1.16:
+* Miscellaneous changes
+
+ - When subdir-objects is in effect, Automake will now construct shorter
+ object file names. This should make the use of $var_SHORTNAME is unnecessary
+ in many cases. $var_SHORTNAME is discouraged anyway.
+
* Bugs fixed:
- Automatic dependency tracking has been fixed to work also when the
diff --git a/bin/automake.in b/bin/automake.in
index 09a1c956b..aaca1321b 100644
--- a/bin/automake.in
+++ b/bin/automake.in
@@ -1562,6 +1562,29 @@ sub check_libobjs_sources
}
}
+# http://linux.seindal.dk/2005/09/09/longest-common-prefix-in-perl/
+sub longest_common_prefix {
+ my $prefix = shift;
+
+ for (@_)
+ {
+ chop $prefix while (! /^$prefix/);
+ }
+ return $prefix;
+}
+
+sub may_truncate {
+ my ($shortname, %transform) = @_;
+ my $blacklist = $transform{'TRUNCATE_OBJNAME_BLACKLIST'};
+
+ # disallow truncation if $derived ends with any of the blacklisted targets
+ # the blacklist still contains the non-canonicalize basename ($name.$ext).
+ foreach (@$blacklist)
+ {
+ return 0 if (canonicalize($_) eq $shortname);
+ }
+ return 1;
+}
# @OBJECTS
# handle_single_transform ($VAR, $TOPPARENT, $DERIVED, $OBJ, $FILE, %TRANSFORM)
@@ -1710,8 +1733,34 @@ sub handle_single_transform
# name that is too long for losing systems, in
# some situations. So we provide _SHORTNAME to
# override.
-
+ # If subdir-object is in effect, it's not necessary to
+ # use the complete 'DERIVED_OBJECT' since objects are
+ # placed next to their source file. Therefore it is already
+ # unique (within that directory). Thus, we try to avoid un
+ # necessarily long file names by stripping the directory
+ # components of 'DERIVED_OBJECT' (that quite likely the result from
+ # %canon_reldir%/%C% usage). This allows to avoid explicit _SHORTNAME
+ # usage in many cases.
+ # NOTE: In some setups, the stripping may lead to duplicated objects,
+ # even in presence of per-executable flags, for example if there
+ # are identically-named programs in different directories, so
+ # these targets must be exempted using a blacklist
my $dname = $derived;
+ if ($directory ne '' && option 'subdir-objects')
+ {
+ # At this point, we don't clear information about what parts
+ # of $derived are truly path components. We can determine
+ # by comparing against the canonicalization of $directory.
+ # And if $directory is empty there is nothing to strip anyway.
+ my $canon_dirname = canonicalize ($directory) . "_";
+ my @names = ($derived, $canon_dirname);
+ my $prefix = longest_common_prefix (@names);
+ # After canonicalization, "_" separates directories, thus use
+ # everything after the the last separator.
+ $dname = substr ($derived, rindex ($prefix, "_")+1);
+ # check if this dname is on the blacklist
+ $dname = $derived unless may_truncate ($dname, %transform);
+ }
my $var = var ($derived . '_SHORTNAME');
if ($var)
{
@@ -1722,7 +1771,6 @@ sub handle_single_transform
$dname = $var->variable_value;
}
$object = $dname . '-' . $object;
-
prog_error ($lang->name . " flags defined without compiler")
if ! defined $lang->compile;
@@ -2431,6 +2479,18 @@ sub handle_libtool ()
LTRMS => join ("\n", @libtool_rms));
}
+# Expects to be passed a list as returned via am_install_var()
+sub find_duplicate_basenames
+{
+ my %seen = ();
+ my @dups = ();
+ foreach my $pair (@_) {
+ my $base = basename(@$pair[1]);
+ push (@dups, $base) if ($seen{$base});
+ $seen{$base} = $base;
+ }
+ return @dups;
+}
sub handle_programs ()
{
@@ -2443,6 +2503,12 @@ sub handle_programs ()
my $seen_global_libobjs =
var ('LDADD') && handle_lib_objects ('', 'LDADD');
+ # Check if we have programs with identical basename. In this case, the
+ # object file name truncation is disabled for this program (and other programs
+ # with the same basename, because the object file names may clash
+ # (this only applies to subdir-object setups).
+ my @blacklist = find_duplicate_basenames (@proglist);
+
foreach my $pair (@proglist)
{
my ($where, $one_file) = @$pair;
@@ -2461,7 +2527,8 @@ sub handle_programs ()
$where->set (INTERNAL->get);
my $linker = handle_source_transform ($xname, $one_file, $obj, $where,
- NONLIBTOOL => 1, LIBTOOL => 0);
+ NONLIBTOOL => 1, LIBTOOL => 0,
+ TRUNCATE_OBJNAME_BLACKLIST => \@blacklist);
if (var ($xname . "_LDADD"))
{
@@ -2541,6 +2608,12 @@ sub handle_libraries ()
define_variable ('ARFLAGS', 'cru', INTERNAL);
define_verbose_tagvar ('AR');
+ # Check if we have libraries with identical basename. In this case, the
+ # object file name truncation is disabled for this library (and other libraries
+ # with the same basename, because the object file names may clash
+ # (this only applies to subdir-object setups).
+ my @blacklist = find_duplicate_basenames (@liblist);
+
foreach my $pair (@liblist)
{
my ($where, $onelib) = @$pair;
@@ -2597,7 +2670,8 @@ sub handle_libraries ()
set_seen ('EXTRA_' . $xlib . '_DEPENDENCIES');
handle_source_transform ($xlib, $onelib, $obj, $where,
- NONLIBTOOL => 1, LIBTOOL => 0);
+ NONLIBTOOL => 1, LIBTOOL => 0,
+ TRUNCATE_OBJNAME_BLACKLIST => \@blacklist);
# If the resulting library lies in a subdirectory,
# make sure this directory will exist.
@@ -2728,6 +2802,11 @@ sub handle_ltlibraries ()
skip_ac_subst => 1);
}
+ # Check if we have libraries with identical basename. In this case, the automatic
+ # object file name truncation is disabled because the object file names may clash
+ # (this only applies to subdir-object setups)
+ my @blacklist = find_duplicate_basenames (@liblist);
+
foreach my $pair (@liblist)
{
my ($where, $onelib) = @$pair;
@@ -2800,7 +2879,8 @@ sub handle_ltlibraries ()
my $linker = handle_source_transform ($xlib, $onelib, $obj, $where,
- NONLIBTOOL => 0, LIBTOOL => 1);
+ NONLIBTOOL => 0, LIBTOOL => 1,
+ TRUNCATE_OBJNAME_BLACKLIST => \@blacklist);
# Determine program to use for link.
my($xlink, $vlink) = define_per_target_linker_variable ($linker, $xlib);
diff --git a/t/list-of-tests.mk b/t/list-of-tests.mk
index defca1361..1e9f86f48 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -1046,6 +1046,7 @@ t/subdir-with-slash.sh \
t/subdir-subsub.sh \
t/subdir-distclean.sh \
t/subdir-keep-going-pr12554.sh \
+t/subdir-objects-objname.sh \
t/subobj.sh \
t/subobj2.sh \
t/subobj4.sh \
diff --git a/t/subdir-objects-objname.sh b/t/subdir-objects-objname.sh
new file mode 100644
index 000000000..1062d5858
--- /dev/null
+++ b/t/subdir-objects-objname.sh
@@ -0,0 +1,115 @@
+#! /bin/sh
+# Copyright (C) 1996-2015 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
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program 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 this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test to make sure the object-shortname option works as advertised
+# Beware that the ibject file name truncation is disabled if there are
+# targets with equal names
+
+. test-init.sh
+
+mkdir -p one/one one/two/sub one/three
+
+cat >> configure.ac << 'END'
+AC_PROG_CC
+AC_OUTPUT
+END
+
+# Files required because we are using '--gnu'.
+: > INSTALL
+: > NEWS
+: > README
+: > COPYING
+: > AUTHORS
+: > ChangeLog
+
+cat > Makefile.am << 'END'
+AUTOMAKE_OPTIONS = subdir-objects
+include one/Makefile.inc
+END
+
+cat > one/Makefile.inc << 'END'
+noinst_PROGRAMS =
+include %D%/one/Makefile.inc
+include %D%/two/Makefile.inc
+include %D%/three/Makefile.inc
+END
+
+cat > one/one/Makefile.inc << 'END'
+noinst_PROGRAMS += %D%/test
+%C%_test_CFLAGS = $(AM_CFLAGS)
+%C%_test_SOURCES = %D%/test.c
+END
+
+cat > one/one/test.c << 'END'
+int main()
+{
+ return 0;
+}
+END
+
+cat > one/two/Makefile.inc << 'END'
+noinst_PROGRAMS += %D%/test
+%C%_test_CFLAGS = $(AM_CFLAGS)
+%C%_test_SOURCES = %D%/test.c
+%C%_test_SOURCES += %D%/sub/test.c
+END
+
+cat > one/two/test.c << 'END'
+int main()
+{
+ return 0;
+}
+END
+
+: > one/two/sub/test.c
+
+cat > one/three/Makefile.inc << 'END'
+noinst_PROGRAMS += %D%/my_test
+%C%_my_test_CFLAGS = $(AM_CFLAGS)
+%C%_my_test_SOURCES = %D%/my_test.c
+END
+
+cat > one/three/my_test.c << 'END'
+int main()
+{
+ return 0;
+}
+END
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE --gnu
+
+./configure
+$MAKE
+MUST_EXIST="one/one/one_one_test-test.o
+one/two/one_two_test-test.o
+one/two/sub/one_two_test-test.o
+one/three/my_test-my_test.o"
+
+MUST_NOT_EXIST="one/one/test-test.o
+one/two/test-test.o
+one/two/sub/test-test.o
+one/three/one_three_my_test-my_test.o"
+
+for file in $MUST_EXIST; do
+ test -f $file || fail_ "$file must exist"
+done
+for file in $MUST_NOT_EXIST; do
+ test -f $file && fail_ "$file must not exist"
+done
+
+$MAKE -f Makefile clean
--
2.12.2
>From 5cf501dd932b4aabcc60b489e4f19c2ad8a757cc Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <m...@gnu.org>
Date: Thu, 13 Apr 2017 14:19:15 +0200
Subject: [PATCH 2/3] Test that should pass.
The test ensures that programs with equal names get unique object files even
if object file name truncation is in effect.
---
t/list-of-tests.mk | 1 +
t/subobj-objname-clash.sh | 58 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+)
create mode 100644 t/subobj-objname-clash.sh
diff --git a/t/list-of-tests.mk b/t/list-of-tests.mk
index 1e9f86f48..1a8302805 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -1063,6 +1063,7 @@ t/subobjname.sh \
t/subobj-clean-pr10697.sh \
t/subobj-clean-lt-pr10697.sh \
t/subobj-indir-pr13928.sh \
+t/subobj-objname-clash.sh \
t/subobj-vpath-pr13928.sh \
t/subobj-pr13928-more-langs.sh \
t/subpkg.sh \
diff --git a/t/subobj-objname-clash.sh b/t/subobj-objname-clash.sh
new file mode 100644
index 000000000..e747037f0
--- /dev/null
+++ b/t/subobj-objname-clash.sh
@@ -0,0 +1,58 @@
+#! /bin/sh
+# Copyright (C) 1996-2017 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
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program 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 this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Make sure that object names don't clash when using subdir-objects.
+
+. test-init.sh
+
+mkdir -p src
+
+cat >> configure.ac << 'END'
+AC_PROG_CC
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+AUTOMAKE_OPTIONS = subdir-objects foreign
+noinst_PROGRAMS = foo
+foo_SOURCES = src/foo.c
+foo_CPPFLAGS = -DVAL=0
+include src/local.mk
+END
+
+cat > src/local.mk << 'END'
+noinst_PROGRAMS += src/foo
+src_foo_CPPFLAGS = -DVAL=1
+src_foo_SOURCES = src/foo.c
+END
+
+cat > src/foo.c << 'END'
+int
+main ()
+{
+ return VAL;
+}
+END
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE --add-missing
+
+./configure
+$MAKE
+./foo || fail_ "./foo should return 0"
+./src/foo && fail_ "./src/foo should return 1"
+$MAKE clean
--
2.12.2
>From 508b7e19745d88dbd1100b403fec440abfd151bb Mon Sep 17 00:00:00 2001
From: Thomas Martitz <ku...@rockbox.org>
Date: Sat, 22 Apr 2017 00:51:44 +0200
Subject: [PATCH 3/3] Extend subobj-objname-clash.sh test with such that it
also looks for clashes in libraries.
---
t/subobj-objname-clash.sh | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/t/subobj-objname-clash.sh b/t/subobj-objname-clash.sh
index e747037f0..e18a66d92 100644
--- a/t/subobj-objname-clash.sh
+++ b/t/subobj-objname-clash.sh
@@ -15,6 +15,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# Make sure that object names don't clash when using subdir-objects.
+# The check is done for programs and libraries separately.
. test-init.sh
@@ -22,31 +23,54 @@ mkdir -p src
cat >> configure.ac << 'END'
AC_PROG_CC
+AC_PROG_RANLIB
AC_OUTPUT
END
cat > Makefile.am << 'END'
AUTOMAKE_OPTIONS = subdir-objects foreign
noinst_PROGRAMS = foo
-foo_SOURCES = src/foo.c
+foo_SOURCES = src/foo.c src/main.c
foo_CPPFLAGS = -DVAL=0
+noinst_LIBRARIES = libbar.a
+libbar_a_SOURCES = src/foo.c
+libbar_a_CPPFLAGS = -DVAL=0
+noinst_PROGRAMS += bar
+bar_SOURCES = src/main.c
+bar_LDADD = libbar.a
include src/local.mk
END
cat > src/local.mk << 'END'
noinst_PROGRAMS += src/foo
src_foo_CPPFLAGS = -DVAL=1
-src_foo_SOURCES = src/foo.c
+src_foo_SOURCES = src/foo.c src/main.c
+noinst_PROGRAMS += src/bar
+noinst_LIBRARIES += src/libbar.a
+src_libbar_a_SOURCES = src/foo.c
+src_libbar_a_CPPFLAGS = -DVAL=1
+src_bar_SOURCES = src/main.c
+src_bar_LDADD = src/libbar.a
END
cat > src/foo.c << 'END'
int
-main ()
+foo ()
{
return VAL;
}
END
+cat > src/main.c << 'END'
+int foo (void);
+
+int
+main ()
+{
+ return foo ();
+}
+END
+
$ACLOCAL
$AUTOCONF
$AUTOMAKE --add-missing
@@ -55,4 +79,6 @@ $AUTOMAKE --add-missing
$MAKE
./foo || fail_ "./foo should return 0"
./src/foo && fail_ "./src/foo should return 1"
+./bar || fail_ "./bar should return 0"
+./src/bar && fail_ "./src/bar should return 1"
$MAKE clean
--
2.12.2
