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

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