Re: [PATCH v3] Shorter object file names under subdir-objects

[Mathieu Lirzin]

Hello,

I have reviewed this updated patch, and modulo minor fixes described
below I am ready to push.  Please confirm if you are OK with the
changes I am proposing.

Thomas Martitz <address@hidden> writes:

> From 7d260d8307a0f6d0eaee7df7c88079dff1865640 Mon Sep 17 00:00:00 2001
> From: Thomas Martitz <address@hidden>
> Date: Mon, 13 Mar 2017 12:41:59 +0100
> Subject: [PATCH] Shorter object file names under subdir-objects
[...]
> * bin/automake.in (handle_targets): New subroutine.
> (handle_targets): Keep track of duplicate short names.
> (handle_single_transform): Truncate object file names when possible.
> * t/subobj-objname-clash.sh: New test.
> * t/list-of-tests.mk (handwritten_TESTS): Add it.
> * NEWS: Update.

* bin/automake.in (proglist, liblist, ltliblist)
(dup_shortnames): New globals.
(initialize_per_input): Initialize them.
(handle_targets): New subroutine.
(handle_single_transform): Truncate object file names when possible.
* t/subobj-objname-clash.sh: New test.
* t/list-of-tests.mk (handwritten_TESTS): Add it.
* NEWS: Update.

[...]
> @@ -1708,10 +1723,48 @@ sub handle_single_transform
>               # (1) uniqueness, and (2) continuity between
>               # invocations.  However, this will result in a
>               # name that is too long for losing systems, in
> -             # some situations.  So we provide _SHORTNAME to
> -             # override.
> -
> +             # some situations.  So we attempt to shorten
> +             # automatically under subdir-objects, and provide
> +             # _SHORTNAME to override as a last resort.
> +             # If subdir-object is in effect, it's usually unnecessary to
> +             # use the complete 'DERIVED_OBJECT' (that is often the result
> +             # from %canon_reldir%/%C% usage) since objects are
> +             # placed next to their source file. Generally, this means it
> +             # is already unique within that directory (see below for an
> +             # exception).
> +             # Thus, we try to avoid unnecessarily long file names by
> +             # stripping the directory components of 'DERIVED_OBJECT'.
> +             # This allows to avoid explicit _SHORTNAME usage in many cases.
> +             # EXCEPTION: If two (or more) targets in different directories
> +             # but with the same base name (after canonicalization), using
> +             # target-specific FLAGS, link the same object, then this logic
> +             # clashes. Thus, we don't strip if this is detected.
> +             #
> +             # See test in t/subobj-objname-clash.sh

I have replaces usage of 'path' with 'file name' to conform to the GNU
terminology.

[...]
>  
> +sub handle_targets ()
> +{
> +  my %seen = ();
> +  my @dups = ();
> +  @proglist = am_install_var ('progs', 'PROGRAMS',
> +                           'bin', 'sbin', 'libexec', 'pkglibexec',
> +                           'noinst', 'check');
> +  @liblist = am_install_var ('libs', 'LIBRARIES',
> +                          'lib', 'pkglib', 'noinst', 'check');
> +  @ltliblist = am_install_var ('ltlib', 'LTLIBRARIES',
> +                          'noinst', 'lib', 'pkglib', 'check');
> +
> +  # Record duplications that may arise after canonicalization of the
> +  # base names, in order to prevent object file clashes in the presence
> +  # of target-specifc *FLAGS
> +  my @array = ( @proglist, @liblist, @ltliblist );

I have replaced @array with @targetlist.

> +  foreach my $pair (@array) {
> +    my $base = canonicalize(basename(@$pair[1]));
> +    push (@dup_shortnames, $base) if ($seen{$base});
> +    $seen{$base} = $base;
> +  }
> +}
> +
> +

> diff --git a/t/subobj-objname-clash.sh b/t/subobj-objname-clash.sh
> new file mode 100644
> index 000000000..b2a3eabd7
> --- /dev/null
> +++ b/t/subobj-objname-clash.sh
> @@ -0,0 +1,104 @@
[...]
> +. test-init.sh
> +
> +mkdir -p src

no need for -p option here

[...]
> +./configure
> +$MAKE
> +set +e
> +./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"
> +./libzap_a; test $? != 2 && fail_ "./libfoo_a should return 2"
> +./src/zap; test $? != 3 && fail_ "./src/prog_libfoo should return 3"
> +./src/foo-uniq; test $? != 4 && fail_ "./foo_uniq should return 4"

IMHO "test $? = X || fail_ ..." reads better.  I have been using && in
the first place only because I didn't know how to properly check the
return value.  :)

> +set -e
> +$MAKE clean

Here is the patch including those changes.

0001-Shorter-object-file-names-under-subdir-objects.patch
Description: Text Data

Thank you for your good work, patience, and good will!

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37