[PATCH] Shorter object file names under subdir-objects

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] Shorter object file names under subdir-objects

Thomas Martitz
Mathieu, based on your reaction on the other thread, I reworked my patch.

This supersedes my other patch, "[PATCH] new option: object-shortname".
It is functionally the
same but does not introduce a new option, but ties the behavior to
subdir-objects instead. In
addition I made an additional bug fix and extended the test suite.

Please merge.
Commit message follows:

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 unecessary, 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).

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



0001-Shorter-object-file-names-under-subdir-objects.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Peter Rosin
Hi!

FWIW, I like this (assuming Mathieu doesn't dig up some road block).

On 2017-03-13 14:19, Thomas Martitz wrote:

> Mathieu, based on your reaction on the other thread, I reworked my patch.
>
> This supersedes my other patch, "[PATCH] new option: object-shortname".
> It is functionally the
> same but does not introduce a new option, but ties the behavior to
> subdir-objects instead. In
> addition I made an additional bug fix and extended the test suite.
>
> Please merge.
> Commit message follows:
>
> 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 unecessary, since uniqueness of the

s/unecessary/unnecessary/

> object file names
> is already achieved by placing them next to the unique source files.

s/is already/are already/

>
> Therefore, this changes strips paths components, that are caused by

s/changes/change/

> %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).
>
> 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
>
>

The lines in the commit message are too long and you do not follow the
GNU ChangeLog format.

[I'm pasting various bits from the attachment inline for easier commenting]

>  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 unecessary

s/is unece/unnece/

> +    in many cases. $var_SHORTNAME is discouraged anyway.
> +
>  * Bugs fixed:

*snip*

> -
> + # 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 strip the directory
> + # components of 'DERIVED_OBJECT' (that quite likely the result from
> + # %canon_reldir%/%C% usage). This enables avoiding explicit _SHORTNAME
> + # unecessary in many cases.

s/unecessary //

>   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);
> +  }

Will not the common $prefix always end with an underscore? In that
case, why not simply use the length of $prefix instead? (And the
comment above is misleading, since it fooled me into thinking that
you do not handle underscores embedded in the filename correctly.

*snip*

> +$MAKE -f Makefile \
> + && test -f one/one/test-test.o && test ! -f one/one/one_two_test-test.o \
> + && test -f one/two/test-test.o && test ! -f one/two/one_two_test-test.o \
> + && test -f one/two/sub/test-test.o && test ! -f one/two/sub/one_two_test-test.o \
> + && test -f one/three/my_test-my_test.o && test ! -f one/three/one_three_my_test-my_test.o

This is buggy. You need to consider the $OBJEXT variable.

Cheers,
Peter

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Thomas Martitz
Hello Peter,

thanks for your feedback. I'll post an update, but I'd like to wait for
Mathieu's feedback too.

Am 13.03.2017 um 15:15 schrieb Peter Rosin:

> Hi!
>
> FWIW, I like this (assuming Mathieu doesn't dig up some road block).
>
> On 2017-03-13 14:19, Thomas Martitz wrote:
>> Mathieu, based on your reaction on the other thread, I reworked my patch.
>>
>> This supersedes my other patch, "[PATCH] new option: object-shortname".
>> It is functionally the
>> same but does not introduce a new option, but ties the behavior to
>> subdir-objects instead. In
>> addition I made an additional bug fix and extended the test suite.
>>
>> Please merge.
>> Commit message follows:
>>
>> 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 unecessary, since uniqueness of the
>
> s/unecessary/unnecessary/
>
>> object file names
>> is already achieved by placing them next to the unique source files.
>
> s/is already/are already/
>
>>
>> Therefore, this changes strips paths components, that are caused by
>
> s/changes/change/
>
>> %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).
>>
>> 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
>>
>>
>
> The lines in the commit message are too long and you do not follow the
> GNU ChangeLog format.
>
> [I'm pasting various bits from the attachment inline for easier commenting]
>
>>  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 unecessary
>
> s/is unece/unnece/
>
>> +    in many cases. $var_SHORTNAME is discouraged anyway.
>> +
>>  * Bugs fixed:
>
> *snip*
>
>> -
>> + # 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 strip the directory
>> + # components of 'DERIVED_OBJECT' (that quite likely the result from
>> + # %canon_reldir%/%C% usage). This enables avoiding explicit _SHORTNAME
>> + # unecessary in many cases.
>
> s/unecessary //
>
>>   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);
>> +  }
>
> Will not the common $prefix always end with an underscore? In that
> case, why not simply use the length of $prefix instead? (And the
> comment above is misleading, since it fooled me into thinking that
> you do not handle underscores embedded in the filename correctly.
>

At this part of the code, I didn't want to reinforce that $derived is
made up only from path components and the target file name. I wanted to
have it work no matter how $derived is constructed (it's passed as a
parameter to the transform function, so the logic how to built $derived
is elsewhere).

In practice, you are right. I can make the change you suggested if wanted.

> *snip*
>
>> +$MAKE -f Makefile \
>> + && test -f one/one/test-test.o && test ! -f one/one/one_two_test-test.o \
>> + && test -f one/two/test-test.o && test ! -f one/two/one_two_test-test.o \
>> + && test -f one/two/sub/test-test.o && test ! -f one/two/sub/one_two_test-test.o \
>> + && test -f one/three/my_test-my_test.o && test ! -f one/three/one_three_my_test-my_test.o
>
> This is buggy. You need to consider the $OBJEXT variable.

You are right. Thanks.

Best regards

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Thomas Martitz
In reply to this post by Thomas Martitz
Am 13.03.2017 um 14:19 schrieb Thomas Martitz:
> Mathieu, based on your reaction on the other thread, I reworked my patch.
>

Hello Mathieue,

have you had a chance to look at my reworked patch?

Best regards.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mathieu Lirzin
Hello Thomas,

Thomas Martitz <[hidden email]> writes:

> Am 13.03.2017 um 14:19 schrieb Thomas Martitz:
>> Mathieu, based on your reaction on the other thread, I reworked my patch.
>>
>
> have you had a chance to look at my reworked patch?

Unfortunately I didn't find the time to search for Potential/Historical
issues regarding the use of shortnames of object files when
subdir-objects option is on.

This is a really busy period for me at the University (Programming
Projects + coming exams) and I am working on my Google Summer of Code
application in parallel.  So don't expect any news from me before the
end of April.  After that I will be more available (than ever) and we will
hopefully merge your patch!

I should have let you know about that situation.

Sorry about that.

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mathieu Lirzin
In reply to this post by Thomas Martitz
Hello Thomas,

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

Thomas Martitz <[hidden email]> writes:

> Mathieu, based on your reaction on the other thread, I reworked my patch.
>
> This supersedes my other patch, "[PATCH] new option:
> object-shortname". It is functionally the
> same but does not introduce a new option, but ties the behavior to
> subdir-objects instead. In
> addition I made an additional bug fix and extended the test suite.
>
> Please merge.
> Commit message follows:
>
> 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.

IIUC The long object names (including the canonicalized directory name)
are not related to the use of %reldir%, it just appears when combining
'subdir-objects' with compilation flags for non toplevel executables or
libraries.

> 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.

> 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).

> 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

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?

Thanks and sorry for the long delay.

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mathieu Lirzin
Mathieu Lirzin <[hidden email]> writes:

> 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.

Here is the missing attachment:


From 970d46d60e65e4c64e7ba6d4f0afa38c6f8620fe Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <[hidden email]>
Date: Thu, 13 Apr 2017 14:19:15 +0200
Subject: [PATCH] Test that should pass.

---
 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.11.0



--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Thomas Martitz
In reply to this post by Mathieu Lirzin
Am 13.04.2017 um 14:59 schrieb Mathieu Lirzin:

> Hello Thomas,
>
> 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).

> Thomas Martitz <[hidden email]> writes:
>
>> Mathieu, based on your reaction on the other thread, I reworked my patch.
>>
>> This supersedes my other patch, "[PATCH] new option:
>> object-shortname". It is functionally the
>> same but does not introduce a new option, but ties the behavior to
>> subdir-objects instead. In
>> addition I made an additional bug fix and extended the test suite.
>>
>> Please merge.
>> Commit message follows:
>>
>> 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.
>
> IIUC The long object names (including the canonicalized directory name)
> are not related to the use of %reldir%, it just appears when combining
> 'subdir-objects' with compilation flags for non toplevel executables or
> libraries.


Yes, you seem to be right. %reldir% is just a method of applying
directory-related name prefixes to targets names and source files.

>
>> 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.


>
>> 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).
>
>> 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
>
> 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. As an easy to implement
alternative, may I propose adding back the Automake option (to opt-in in
the new behavior), maybe with proposal to turn it on by default with
Automake 2.0 along with subdir-objects? Perhaps we can find a
description for the manual that makes you feel less uneasy about
maintainability? What do you think?

Best regards


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mathieu Lirzin
Hello,

Thomas Martitz <[hidden email]> 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 <[hidden email]> 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. As an easy to
> implement alternative, may I propose adding back the Automake option
> (to opt-in in the new behavior), maybe with proposal to turn it on by
> default with Automake 2.0 along with subdir-objects?

That would be an easy alternative indeed.  But as I already stated, I am
not in favour of adding an option for this.  However I am OK with
changes that are transparent to the users, even if it requires extra
work.  :)

> Perhaps we can find a description for the manual that makes you feel
> less uneasy about maintainability? What do you think?

I doubt rewording things in the manual could be done so that this option
doesn't appear as a facility to achieve something similar to the
discouraged _SHORTNAME with the extra possibility to combine it with
%reldir%.

Thanks.

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Thomas Martitz
Hello Matieu,

good news: I found a solution.

Am 19.04.2017 um 12:54 schrieb Mathieu Lirzin:

> Hello,
>
> Thomas Martitz <[hidden email]> 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 <[hidden email]> 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.

0001-Shorter-object-file-names-under-subdir-objects.patch (12K) Download Attachment
0002-Test-that-should-pass.patch (2K) Download Attachment
0003-Extend-subobj-objname-clash.sh-test-with-such-that-i.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mathieu Lirzin
Hello Thomas,

Thomas Martitz <[hidden email]> writes:

> good news: I found a solution.

Great!  You have been fast.  :)

> 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.

I am still busy, so I will need some time before being able to review
your patch seriously.  After the 4 may, I will have more time.  Anyone
wanting to help in the review process is of course welcome.

> From 8cd0a71abac70bbdcaac804dc7964fa9f34753a0 Mon Sep 17 00:00:00 2001
> From: Thomas Martitz <[hidden email]>
> 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

[...]

> From 5cf501dd932b4aabcc60b489e4f19c2ad8a757cc Mon Sep 17 00:00:00 2001
> From: Mathieu Lirzin <[hidden email]>
> 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

[...]

> From 508b7e19745d88dbd1100b403fec440abfd151bb Mon Sep 17 00:00:00 2001
> From: Thomas Martitz <[hidden email]>
> 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(-)

At first glance, I think it would be reasonable to reduce the 3 tests to
only 1 which checks that there is no name clash between multiple
_PROGRAMS and _LIBRARIES.

Can you send an updated patch which includes both your code and the
suggested test?

Thank you.

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Thomas Martitz
Am 23.04.2017 um 18:39 schrieb Mathieu Lirzin:
> Hello Thomas,
>
> Thomas Martitz <[hidden email]> writes:
>
>> good news: I found a solution.
>
> Great!  You have been fast.  :)

You too!

>
>> 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.
>
> I am still busy, so I will need some time before being able to review
> your patch seriously.  After the 4 may, I will have more time.  Anyone
> wanting to help in the review process is of course welcome.
>
>> From 8cd0a71abac70bbdcaac804dc7964fa9f34753a0 Mon Sep 17 00:00:00 2001
>> From: Thomas Martitz <[hidden email]>
>> 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
>
> [...]
>
>> From 5cf501dd932b4aabcc60b489e4f19c2ad8a757cc Mon Sep 17 00:00:00 2001
>> From: Mathieu Lirzin <[hidden email]>
>> 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
>
> [...]
>
>> From 508b7e19745d88dbd1100b403fec440abfd151bb Mon Sep 17 00:00:00 2001
>> From: Thomas Martitz <[hidden email]>
>> 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(-)
>
> At first glance, I think it would be reasonable to reduce the 3 tests to
> only 1 which checks that there is no name clash between multiple
> _PROGRAMS and _LIBRARIES.
>
> Can you send an updated patch which includes both your code and the
> suggested test?


Sorry, I don't get which 3 tests you refer to, please explain. Currently
there are two test scripts included: one checks for the object file
names to see if they are truncated (or not). The other one checks in an
abstract way if the wrong object file was included in a binary (due to
potential name clashes with my change).

Also, do you suggest that I shall squash the 3 patch files into one? I
didn't do so as to not lose the authorship information on your test script.


Best regards.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mathieu Lirzin
Hello,

Thomas Martitz <[hidden email]> writes:

> Am 23.04.2017 um 18:39 schrieb Mathieu Lirzin:
>
>>> From 8cd0a71abac70bbdcaac804dc7964fa9f34753a0 Mon Sep 17 00:00:00 2001
>>> From: Thomas Martitz <[hidden email]>
>>> 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
>>
>> [...]
>>
>>> From 5cf501dd932b4aabcc60b489e4f19c2ad8a757cc Mon Sep 17 00:00:00 2001
>>> From: Mathieu Lirzin <[hidden email]>
>>> 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
>>
>> [...]
>>
>>> From 508b7e19745d88dbd1100b403fec440abfd151bb Mon Sep 17 00:00:00 2001
>>> From: Thomas Martitz <[hidden email]>
>>> 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(-)
>>
>> At first glance, I think it would be reasonable to reduce the 3 tests to
>> only 1 which checks that there is no name clash between multiple
>> _PROGRAMS and _LIBRARIES.
>>
>> Can you send an updated patch which includes both your code and the
>> suggested test?
>
>
> Sorry, I don't get which 3 tests you refer to, please
> explain.

There is one test in each patch you sent (including the patch I sent).

> Currently there are two test scripts included: one checks for
> the object file names to see if they are truncated (or not).

Since Automake doesn't provide any garantee about object file names.  I
don't think we need to include such test in the test suite.

> The other one checks in an abstract way if the wrong object file was
> included in a binary (due to potential name clashes with my change).
>
> Also, do you suggest that I shall squash the 3 patch files into one? I
> didn't do so as to not lose the authorship information on your test
> script.

Yes, that was my suggestion.  In this particular case, I don't care
about my authorship information.  You deserve all the credits.  :)

Thanks.

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Thomas Martitz
Am 26.04.2017 um 13:49 schrieb Mathieu Lirzin:).
>
>> Currently there are two test scripts included: one checks for
>> the object file names to see if they are truncated (or not).
>
> Since Automake doesn't provide any garantee about object file names.  I
> don't think we need to include such test in the test suite.
>


Hi,

Jim Meyering asked me[1] to add a test case to make sure my change is
working as intended. However, I'll drop it, since it the exact file
names are internal not documented or guaranteed to the public.

I'll send an updated patch that squashes the files into one and drops
the test that checks for the exact object file names (keeping yours).


[1] http://lists.gnu.org/archive/html/automake-patches/2017-01/msg00005.html

Best regards.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v2] Shorter object file names under subdir-objects

Thomas Martitz
In reply to this post by Thomas Martitz
Hello,

here's the updated patch. Changes since v1:

- The test that checks for exact object file names was replaced by
another one that tests if object files clash due to truncation of the name
- When there were two programs or libraries with the same base name (but
in different dirs), then the wrong object was linked into one of the
programs/libs.

Note: There is still one extreme edge case which is not handled yet. You
can apparently name a program libfoo_la. If there is also library
libfoo.la generated in another dir, then the objects will clash again.
My fix above only checks for clashes within programs (and within
libraries). This is due to the code structure (libraries are written out
before programs parsed at all). I wanted to get opinions on how to
tackle this before going a wrong route. Maybe change the code so that
all programs and libs are parsed first, checked for clashes second and
then written out?

Best regards

0001-Shorter-object-file-names-under-subdir-objects.patch (11K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mathieu Lirzin
Hello,

Thomas Martitz <[hidden email]> writes:

> here's the updated patch. Changes since v1:
>
> - The test that checks for exact object file names was replaced by
> another one that tests if object files clash due to truncation of the
> name
> - When there were two programs or libraries with the same base name
> (but in different dirs), then the wrong object was linked into one of
> the programs/libs.
>
> Note: There is still one extreme edge case which is not handled
> yet. You can apparently name a program libfoo_la. If there is also
> library libfoo.la generated in another dir, then the objects will
> clash again. My fix above only checks for clashes within programs (and
> within libraries). This is due to the code structure (libraries are
> written out before programs parsed at all). I wanted to get opinions
> on how to tackle this before going a wrong route. Maybe change the
> code so that all programs and libs are parsed first, checked for
> clashes second and then written out?
For now you are using a '%blacklist' which is local to each function
('handle_programs' and 'handle_libraries').  Maybe if you make it global
and resettable in '&initialize_per_input', the name clash detection
could work across programs and libraries?

I haven't tried implemented this idea so maybe there are some issues
with it.

> From fe1130722d542d42d081696cf76a94746e0abdfc Mon Sep 17 00:00:00 2001
> From: Thomas Martitz <[hidden email]>
> Date: Mon, 13 Mar 2017 12:41:59 +0100
> Subject: [PATCH] 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 (but are in different directories),
> 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.
>
> The handling of the above case is ensured in a test case. The test ensures that
> programs and libraries with equal base names get unique object files even if
> object file name truncation is in effect.
>
> Example:
> previously:
>   AUTOMAKE_OPTIONS = subdir-objects
>   bin_PROGRAMS += path/to/foo
>   path_to_foo_CFLAGS = $(AM_CFLAGS) -g
>
> resulted in objects:
>   sub/path_to_foo-foo.o
>
> now object file name is:
>   sub/foo-foo.o
Regarding the commit message, it needs to have a ChangeLog style at the
end.  Here is what I am proposing with a shorter rationale which IMO
gives enough context:

--8<---------------cut here---------------start------------->8---
automake: Shorter object file names under subdir-objects

Combining the 'subdir-objects' option with target-specific flags, had
the consequence of producing long object file names.  This was done to
preventively ensure the uniqueness of object file names.  We are now
using shorter names by default, and handle long names when an actual
conflict is detected.  This will hopefully reduce the necessity of
using the 'prog_SHORTNAME' facility.

Example Makefile.am:
  AUTOMAKE_OPTIONS = subdir-objects
  bin_PROGRAMS += path/to/foo
  path_to_foo_CFLAGS = $(AM_CFLAGS) -g

previously resulted in object:
  $(top_builddir)/path/to/path_to_foo-foo.o

now object file name is:
  $(top_builddir)/path/to/foo-foo.o

* bin/automake.in (longest_common_prefix, may_truncate)
(find_duplicate_basenames): New subroutines.
(handle_programs, handle_libraries): Keep track of duplicate 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.
--8<---------------cut here---------------end--------------->8---

> ---
>  NEWS                      |  6 ++++
>  bin/automake.in           | 90 ++++++++++++++++++++++++++++++++++++++++++++---
>  t/list-of-tests.mk        |  1 +
>  t/subobj-objname-clash.sh | 86 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 178 insertions(+), 5 deletions(-)
>  create mode 100644 t/subobj-objname-clash.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
                                                                     ^^^
                                                             extra 'is'.
> +    in many cases. $var_SHORTNAME is discouraged anyway.
                                   
Please take care of not moving above the 79 characters limit per line.
Here is my proposition which explicitely state the cases where the name
will be shorter:

--8<---------------cut here---------------start------------->8---
  - When subdir-objects is in effect, Automake will now construct
    shorter object file names when no programs and libraries name
    clashes are encountered.  This should make the discouraged use of
    'foo_SHORTNAME' unnecessary in many cases.
--8<---------------cut here---------------end--------------->8---

>  * 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;
> +}
Even when the subroutine seems trivial, I think it is nice to have a
docstring which explains what are the expected inputs, and what is the
returned value or the side effects.

I have updated the indentation to use GNU style, added docstrings, and
added prototypes:

--8<---------------cut here---------------start------------->8---
# $PREFIX
# longest_common_prefix (@WORDS)
# ------------------------------
# Return the longest common prefix of @WORDS.  If there is no common prefix
# return the empty string.
#
# Implementation taken from
# 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;
}

# $BOOLEAN
# may_truncate ($SHORTNAME, %TRANSFORM)
# -------------------------------------
# Check if $SHORTNAME can be truncated based on the
# 'TRUNCATE_OBJNAME_BLACKLIST' property of %TRANSFORM.
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 my $basename (@$blacklist)
    {
      return 0
        if (canonicalize ($basename) eq $shortname);
    }
  return 1;
}
--8<---------------cut here---------------end--------------->8---

>  
>  # @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);
                                                                    ^^^
...) + 1)

> +    # check if this dname is on the blacklist
> +    $dname = $derived unless may_truncate ($dname, %transform);
> +  }
>   my $var = var ($derived . '_SHORTNAME');
>   if ($var)
>   {
> @@ -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;
> +}
I have added a docstring and corrected the indentation, like for the
previous subroutines:

--8<---------------cut here---------------start------------->8---
# @DUPLICATES
# find_duplicate_basenames (@VARIABLES)
# -------------------------------------
# Expects @VARIABLES to be 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;
}
--8<---------------cut here---------------end--------------->8---

>  
>  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
                                                                               ^^^
line is too long.

> +  # 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);
                                                                                ^^^
line is too long.  Please indent like this:

--8<---------------cut here---------------start------------->8---
      my $linker =
        handle_source_transform ($xname, $one_file, $obj, $where,
                                 NONLIBTOOL => 1, LIBTOOL => 0,
                                 TRUNCATE_OBJNAME_BLACKLIST => \@blacklist);
--8<---------------cut here---------------end--------------->8---

>  
>        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
                                                                               ^^^
lines are too long.

> +  # (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);
line is too long.  Please indent like previous case.

>        # 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..a5b253504 100644
> --- a/t/list-of-tests.mk
> +++ b/t/list-of-tests.mk
> @@ -1062,6 +1062,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..11fc9631b
> --- /dev/null
> +++ b/t/subobj-objname-clash.sh
> @@ -0,0 +1,86 @@
> +#! /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.
> +# The check is done for programs and libraries separately.
> +
> +. test-init.sh
> +
> +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 =
> +noinst_LIBRARIES =
> +include src/local.mk
> +noinst_PROGRAMS += foo
> +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
> +END
> +
> +cat > src/local.mk << 'END'
> +noinst_PROGRAMS += src/foo
> +src_foo_CPPFLAGS = -DVAL=1
> +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
For more clarity, I would prefer if the progam and librarie parts of the
Makefiles were separated them in two Makefile fragments with a comment
for each.  See my proposition:

--8<---------------cut here---------------start------------->8---
cat > Makefile.am << 'END'
AUTOMAKE_OPTIONS = subdir-objects foreign

include prog.mk
include lib.mk
END

# Check that programs with same basename don't end up sharing the same
# object files.
cat > prog.mk << 'END'
noinst_PROGRAMS = foo src/foo
src_foo_CPPFLAGS = -DVAL=1
src_foo_SOURCES = src/foo.c src/main.c

foo_CPPFLAGS = -DVAL=0
foo_SOURCES = src/foo.c src/main.c
END

# Check the same thing for libraries.
cat > lib.mk << 'END'
noinst_LIBRARIES = libbar.a src/libbar.a

libbar_a_SOURCES = src/foo.c
libbar_a_CPPFLAGS = -DVAL=0

src_libbar_a_SOURCES = src/foo.c
src_libbar_a_CPPFLAGS = -DVAL=1

noinst_PROGRAMS += bar src/bar

bar_SOURCES = src/main.c
bar_LDADD = libbar.a

src_bar_SOURCES = src/main.c
src_bar_LDADD = src/libbar.a
END
--8<---------------cut here---------------end--------------->8---

> +
> +cat > src/foo.c << 'END'
> +int
> +foo ()
> +{
> +  return VAL;
> +}
> +END
> +
> +cat > src/main.c << 'END'
> +int foo (void);
> +
> +int
> +main ()
> +{
> +  return foo ();
> +}
> +END
> +
> +$ACLOCAL
> +$AUTOCONF
> +$AUTOMAKE --add-missing
> +
> +./configure
> +$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
Here is an updated patch which includes all the requested changes.



Thanks.

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

0001-automake-Shorter-object-file-names-under-subdir-obje.patch (11K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Thomas Martitz
Hello,

huge thanks for your review thus far.

I have reworked the patch, making most of your remarks moot. I have
incorporated the remaining ones.

Change since v2:
- I changed the logic to first collect the targets via am_install_var,
then scan for duplicated names (by the new handle_targets function), and
then call handle_programs etc.
- The whole truncation is now done after the blacklist check is succeeds
(before the blacklist check was done last)
- I changed the truncation logic itself to only truncate at directory
boundaries (before it could truncate in the middle if a path component
contained underscores), and also simplified it.
- I refactored the test to be better readable and added a case for a
clash between a program and a library.
- I changed to commit message and NEWS item according to your suggestions


Please review!

Best regards.

0001-Shorter-object-file-names-under-subdir-objects.patch (10K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 <[hidden email]> writes:

> From 7d260d8307a0f6d0eaee7df7c88079dff1865640 Mon Sep 17 00:00:00 2001
> From: Thomas Martitz <[hidden email]>
> 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.



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

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

0001-Shorter-object-file-names-under-subdir-objects.patch (11K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Thomas Martitz
Am 15.06.2017 um 02:23 schrieb 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.

Hello,

your patch looks fine. Please apply!

Thank you for your support.

Best regards.


Loading...