Hi Ralf, sorry for the delay.
On Wednesday 15 September 2010, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Wed, Sep 15, 2010 at 01:26:50PM CEST:
> > OK for maint?
>
> I really think you should use an extra function normalize_file_name
> to do the job, for maintainability.
>
> Please run make maintainer-check.
>
> Typo patologic.
>
> A couple more nits below.
>
> I don't think we want subobj11c.test. What we want instead, and I
> agree with your earlier sentiments on this, is unit testing for the
> normalize_file_name function. Which suggests that it should be in
> lib/Automake/FileUtils.pm I guess,
I disagree, it would be a text-transformation function which doesn't
really belong to that file. And I'm not sure where it belongs,
either. What about a new "temporary" module `Automake::Misc' for
functions which do not (still) have a better place to go into? I've
tried this in the attached follow-up patch.
BTW, in the long run, my idea would be to move *almost all* of
automake.in into a "temporary" module `Automake::Main', to ease its
unit-testability. Then we could proceed to refactoring, and split
it up in several separate modules where needed, thus improving
modularity and clarity. But this second step should be undertaken
only once we have enough testsuite coverage -- no, the present one
is definitely not enough IMHO).
I'm not trying to do this code reoarganization now, because it would
disrupt useful pending patches, at least:
- Peter Rosin's patch on `AM_PROG_AR' and `ar-lib', for allowing
use of Microsoft `lib' as archiver
- Valentin David's patch providing much-wanted support for
user-extensions in automake
- my patch on better dependency declaration for Yacc/Lex (definitely
not as useful as the other ones, but still having its merits
IMVHO).
Thus, for the moment, I went with the non-disruptive `Automake::Misc'
new module. The attached patch is a follow-up for the version 3 of
the "Work around a bug in Solaris make's file-inclusion mechanism."
patch (posted in another message in this thread). I tested it on
Debian GNU/Linux with GNU make3.81 and FreeBSD make (8.0), and on
Solaris 10 with various combinations of make programs (GNU make 3.82.
SunStudio dmake, xpg4 make, ccs make) and shells (/bin/sh, bin/ksh,
/usr/xpg4/bin/sh). No regression showed up.
Regards,
Stefano
From 0efd46a8a0227a0a570d191546f6f2f7d666614f Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <stefano.lattar...@gmail.com>
Date: Thu, 23 Sep 2010 00:46:25 +0200
Subject: [PATCH] Refactor code for generating depfiles inclusion in Makefiles.
* automake.in: Import new module Automake::Misc.
(handle_single_transform): Move code to compute the path of
make-included depfile into ...
* lib/Automake/Misc.pm (compute_depfile_path): ... this new
function in this new module.
* lib/Automake/Makefile.am (dist_perllib_DATA): Updated.
* lib/Automake/tests/Misc.pl: New file, contains unit tests for
the new module Automake::Misc.
* lib/Automake/tests/Makefile.am (TESTS): Updated.
* tests/subobj11c.test: Removed, made obsoleted by new unit tests
above.
* tests/Makefile.am (TESTS): Updated.
Suggested by Ralf Wildenhues.
---
ChangeLog | 17 +++
automake.in | 24 +----
lib/Automake/Makefile.am | 1 +
lib/Automake/Makefile.in | 1 +
lib/Automake/Misc.pm | 96 +++++++++++++++++
lib/Automake/tests/Makefile.am | 1 +
lib/Automake/tests/Makefile.in | 1 +
lib/Automake/tests/Misc.pl | 222 ++++++++++++++++++++++++++++++++++++++++
tests/Makefile.am | 1 -
tests/Makefile.in | 1 -
tests/subobj11c.test | 53 ----------
11 files changed, 343 insertions(+), 75 deletions(-)
create mode 100644 lib/Automake/Misc.pm
create mode 100644 lib/Automake/tests/Misc.pl
delete mode 100755 tests/subobj11c.test
diff --git a/ChangeLog b/ChangeLog
index dbb7b82..28b9bca 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2010-09-23 Stefano Lattarini <stefano.lattar...@gmail.com>
+
+ Refactor code for generating depfiles inclusion in Makefiles.
+ * automake.in: Import new module Automake::Misc.
+ (handle_single_transform): Move code to compute the path of
+ make-included depfile into ...
+ * lib/Automake/Misc.pm (compute_depfile_path): ... this new
+ function in this new module.
+ * lib/Automake/Makefile.am (dist_perllib_DATA): Updated.
+ * lib/Automake/tests/Misc.pl: New file, contains unit tests for
+ the new module Automake::Misc.
+ * lib/Automake/tests/Makefile.am (TESTS): Updated.
+ * tests/subobj11c.test: Removed, made obsoleted by new unit tests
+ above.
+ * tests/Makefile.am (TESTS): Updated.
+ Suggested by Ralf Wildenhues.
+
2010-09-23 Ralf Wildenhues <ralf.wildenh...@gmx.de>
Stefano Lattarini <stefano.lattar...@gmail.com>
diff --git a/automake.in b/automake.in
index ab63811..ff8a864 100755
--- a/automake.in
+++ b/automake.in
@@ -151,6 +151,7 @@ use Automake::FileUtils;
use Automake::Location;
use Automake::Condition qw/TRUE FALSE/;
use Automake::DisjConditions;
+use Automake::Misc 'compute_depfile_path';
use Automake::Options;
use Automake::Version;
use Automake::Variable;
@@ -2106,26 +2107,9 @@ sub handle_single_transform ($$$$$%)
if scalar @dep_list > 0;
}
- # Transform .o or $o file into .P file (for automatic
- # dependency code).
- # Properly flatten multiple adjacent slashes, as Solaris 10 make
- # might fail over them in an include statement.
- # Leading double slashes may be special, as per Posix, so deal
- # with them carefully.
- if ($lang && $lang->autodep ne 'no')
- {
- my $depfile = $object;
- $depfile =~ s/\.([^.]*)$/.P$1/;
- $depfile =~ s/\$\(OBJEXT\)$/o/;
- my $maybe_extra_leading_slash = '';
- $maybe_extra_leading_slash = '/' if $depfile =~ m,^//[^/],;
- $depfile =~ s,/+,/,g;
- my $basename = basename ($depfile);
- # This might make $dirname empty, but we account for that below.
- (my $dirname = dirname ($depfile)) =~ s/\/*$//;
- $dirname = $maybe_extra_leading_slash . $dirname;
- $dep_files{$dirname . '/$(DEPDIR)/' . $basename} = 1;
- }
+ # For automatic dependency code.
+ $dep_files{&compute_depfile_path ($object)} = 1
+ if ($lang && $lang->autodep ne 'no');
}
return @result;
diff --git a/lib/Automake/Makefile.am b/lib/Automake/Makefile.am
index 0858b68..226c49d 100644
--- a/lib/Automake/Makefile.am
+++ b/lib/Automake/Makefile.am
@@ -32,6 +32,7 @@ dist_perllib_DATA = \
ItemDef.pm \
Location.pm \
Options.pm \
+ Misc.pm \
Rule.pm \
RuleDef.pm \
Struct.pm \
diff --git a/lib/Automake/Makefile.in b/lib/Automake/Makefile.in
index e068ab8..b7ff4c6 100644
--- a/lib/Automake/Makefile.in
+++ b/lib/Automake/Makefile.in
@@ -238,6 +238,7 @@ dist_perllib_DATA = \
ItemDef.pm \
Location.pm \
Options.pm \
+ Misc.pm \
Rule.pm \
RuleDef.pm \
Struct.pm \
diff --git a/lib/Automake/Misc.pm b/lib/Automake/Misc.pm
new file mode 100644
index 0000000..f4a1830
--- /dev/null
+++ b/lib/Automake/Misc.pm
@@ -0,0 +1,96 @@
+# Copyright (C) 2010 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/>.
+
+###############################################################
+# The main copy of this file is in Automake's git repository. #
+# Updates should be sent to automake-patc...@gnu.org. #
+###############################################################
+
+package Automake::Misc;
+
+=head1 NAME
+
+Automake::Misc - miscellaneous functions
+
+=head1 SYNOPSIS
+
+ use Automake::Misc
+
+=head1 DESCRIPTION
+
+This perl module provides functions which doesn't truly belong to any
+of the other Automake perl modules.
+
+=cut
+
+use strict;
+use Exporter;
+use File::Basename;
+
+use vars qw (@ISA @EXPORT);
+
+...@isa = qw (Exporter);
+...@export = qw (&compute_depfile_path);
+
+
+=item XXX C<open_quote ($file_name)>
+
+XXX Quote C<$file_name> for open.
+
+=cut
+
+# $DEPENDENCY_FILE_NAME
+# compute_depfile_path ($OBJECT_FILE_NAME)
+#-----------------------------------------
+# Transform .o, .obj or .lo filename into .P filename, to be included by
+# Makefiles for automatic dependency code.
+# Properly flatten multiple adjacent slashes, as Solaris 10 make might
+# fail over them in an include statement.
+# Leading double slashes may be special, as per Posix, so deal with
+# them carefully.
+sub compute_depfile_path ($)
+{
+ # FIXME: should we abort if $OBJECT_FILE_NAME has no extension?
+ my $depfile = shift;
+ $depfile =~ s/\.([^.]*)$/.P$1/;
+ $depfile =~ s/\$\(OBJEXT\)$/o/;
+ my $maybe_extra_leading_slash = '';
+ $maybe_extra_leading_slash = '/' if $depfile =~ m,^//[^/],;
+ $depfile =~ s,/+,/,g;
+ my $basename = basename ($depfile);
+ # This might make $dirname empty, but we account for that below.
+ (my $dirname = dirname ($depfile)) =~ s/\/*$//;
+ $dirname = $maybe_extra_leading_slash . $dirname;
+ return ($dirname . '/$(DEPDIR)/' . $basename);
+}
+
+1; # for require
+
+### Setup "GNU" style for perl-mode and cperl-mode.
+## Local Variables:
+## perl-indent-level: 2
+## perl-continued-statement-offset: 2
+## perl-continued-brace-offset: 0
+## perl-brace-offset: 0
+## perl-brace-imaginary-offset: 0
+## perl-label-offset: -2
+## cperl-indent-level: 2
+## cperl-brace-offset: 0
+## cperl-continued-brace-offset: 0
+## cperl-label-offset: -2
+## cperl-extra-newline-before-brace: t
+## cperl-merge-trailing-else: nil
+## cperl-continued-statement-offset: 2
+## End:
diff --git a/lib/Automake/tests/Makefile.am b/lib/Automake/tests/Makefile.am
index c5e53d2..82f5184 100644
--- a/lib/Automake/tests/Makefile.am
+++ b/lib/Automake/tests/Makefile.am
@@ -24,6 +24,7 @@ Condition.pl \
Condition-t.pl \
DisjConditions.pl \
DisjConditions-t.pl \
+Misc.pl \
Version.pl \
Wrap.pl
diff --git a/lib/Automake/tests/Makefile.in b/lib/Automake/tests/Makefile.in
index 567802b..a072751 100644
--- a/lib/Automake/tests/Makefile.in
+++ b/lib/Automake/tests/Makefile.in
@@ -246,6 +246,7 @@ Condition.pl \
Condition-t.pl \
DisjConditions.pl \
DisjConditions-t.pl \
+Misc.pl \
Version.pl \
Wrap.pl
diff --git a/lib/Automake/tests/Misc.pl b/lib/Automake/tests/Misc.pl
new file mode 100644
index 0000000..cddcef9
--- /dev/null
+++ b/lib/Automake/tests/Misc.pl
@@ -0,0 +1,222 @@
+# Copyright (C) 2010 Free Software Foundation, Inc.
+#
+# This file is part of GNU Automake.
+#
+# GNU Automake is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# GNU Automake is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+use Automake::Misc 'compute_depfile_path';
+
+# $FAILED
+# check_depfile $OBJECT_DIRNAME $OBJECT_BASENAME $OBJECT_EXTENSION
+# $DEPFILE_DIRNAME $DEPFILE_EXTENSION
+sub check_depfile ($$$$$)
+{
+ my ($obj_dirname, $obj_basename, $obj_ext, $depfile_dirname,
+ $depfile_ext) = @_;
+ my $depdir_base = '$(DEPDIR)';
+ my $exp = $depfile_dirname . $depdir_base . '/' . $obj_basename .
+ '.' . $depfile_ext;
+ my $obj = $obj_dirname . $obj_basename . '.' . $obj_ext;
+ my $got = compute_depfile_path($obj);
+ print STDERR " ---\n";
+ print STDERR "dir: $obj_dirname\n";
+ print STDERR "bas: $obj_basename\n";
+ print STDERR "ext: $obj_ext\n";
+ print STDERR "dpd: $depfile_dirname\n";
+ print STDERR "obj: $obj\n";
+ print STDERR "exp: $exp\n";
+ print STDERR "got: $got\n";
+ if ($got ne $exp)
+ {
+ print STDERR "***FAIL***\n";
+ return 1;
+ }
+ else
+ {
+ return 0;
+ }
+}
+
+sub test_compute_depfile_path ()
+{
+ my $failed = 0;
+ my @object_basenames = ('foo', "-b_ a\tr", 'q.u.u.x');
+ my @test_data = (
+# FORMAT:
+# [
+# [...@dirs_for_object_file],
+# $depfile_dirname
+# ]
+ [
+ ['', qw{./ .// ./// .////}],
+ './',
+ ],
+ [
+ [qw{/ /// ////}],
+ '/',
+ ],
+ [
+ [qw{//}],
+ '//',
+ ],
+ [
+ [qw{/dir/ /dir// ///dir/ ///dir// ///dir/// ////dir/}],
+ '/dir/'
+ ],
+ [
+ [qw{//server/ //server// //server///}],
+ '//server/',
+ ],
+ [
+ [qw{./dir/ ./dir// .//dir/// .///dir/ .////dir//}],
+ './dir/',
+ ],
+ [
+ [qw{dir/ dir// dir/// dir////}],
+ 'dir/',
+ ],
+ [
+ [qw{
+ /dir/sub/
+ /dir//sub/
+ /dir/sub//
+ /dir//sub//
+ /dir///sub/
+ /dir/sub///
+
+ ///dir/sub/
+ ///dir//sub/
+ ///dir/sub//
+ ///dir///sub/
+ ///dir/sub///
+ ///dir//sub//
+ ///dir////sub/////
+
+ ////dir/////sub///
+ }],
+ '/dir/sub/',
+ ],
+ [
+ [qw{
+ //server/path/
+
+ //server//path/
+ //server/path//
+
+ //server///path/
+ //server//path//
+ //server/path///
+
+ //server////path/
+ //server///path//
+ //server//path///
+ //server/path////
+ }],
+ '//server/path/',
+ ],
+ [
+ [qw{
+ ./dir/sub/sub2/
+ .//dir/sub/sub2/
+ .///dir/sub/sub2/
+
+ ./dir//sub/sub2/
+ ./dir/sub//sub2/
+ ./dir/sub/sub2//
+
+ .//dir//sub/sub2/
+ .//dir/sub//sub2/
+ .//dir/sub/sub2//
+
+ ./dir///sub/sub2/
+ .////dir///sub//sub2///
+ .//dir/sub//////sub2///
+ }],
+ './dir/sub/sub2/',
+ ],
+ [
+ [qw{
+ //server/path/dir/
+
+ //server//path/dir/
+ //server/path//dir/
+ //server/path/dir//
+
+ //server///path/dir/
+ //server//path//dir/
+ //server/path/dir//
+
+ //server////path//dir//
+ //server/path///dir////
+ //server///path////dir/
+ }],
+ '//server/path/dir/',
+ ],
+ [
+ [qw{
+ dir0/dir1/dir2/dir3/
+ dir0//dir1//dir2//dir3//
+ dir0/dir1//dir2///dir3////
+ dir0/////dir1//dir2///dir3/
+ dir0/dir1/////dir2/dir3//
+ dir0////dir1/dir2////dir3////
+ }],
+ 'dir0/dir1/dir2/dir3/',
+ ],
+ );
+ for (@test_data)
+ {
+ my @object_dirnames = @{$_->[0]};
+ my $depfile_dirname = $_->[1];
+ for my $obj_bn (@object_basenames)
+ {
+ for my $obj_dn (@object_dirnames)
+ {
+ # `foo.xxx' brings to depfile `foo.Pxxx' ...
+ $failed = 1 if check_depfile ($obj_dn, $obj_bn, 'o',
+ $depfile_dirname, 'Po');
+ $failed = 1 if check_depfile ($obj_dn, $obj_bn, 'lo',
+ $depfile_dirname, 'Plo');
+ $failed = 1 if check_depfile ($obj_dn, $obj_bn, 'baz',
+ $depfile_dirname, 'Pbaz');
+ $failed = 1 if check_depfile ($obj_dn, $obj_bn, '$(foo)',
+ $depfile_dirname, 'P$(foo)');
+ # ... with the exception of `foo.$(OBJEXT)', which brings
+ # to depfile `foo.Po'
+ $failed = 1 if check_depfile ($obj_dn, $obj_bn, '$(OBJEXT)',
+ $depfile_dirname, 'Po');
+ }
+ }
+ }
+ return $failed;
+}
+
+exit (test_compute_depfile_path);
+
+### Setup "GNU" style for perl-mode and cperl-mode.
+## Local Variables:
+## perl-indent-level: 2
+## perl-continued-statement-offset: 2
+## perl-continued-brace-offset: 0
+## perl-brace-offset: 0
+## perl-brace-imaginary-offset: 0
+## perl-label-offset: -2
+## cperl-indent-level: 2
+## cperl-brace-offset: 0
+## cperl-continued-brace-offset: 0
+## cperl-label-offset: -2
+## cperl-extra-newline-before-brace: t
+## cperl-merge-trailing-else: nil
+## cperl-continued-statement-offset: 2
+## End:
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 775fe2a..ee637e5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -674,7 +674,6 @@ subobj9.test \
subobj10.test \
subobj11a.test \
subobj11b.test \
-subobj11c.test \
subobjname.test \
subpkg.test \
subpkg2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index ec56fb5..a7b1c28 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -912,7 +912,6 @@ subobj9.test \
subobj10.test \
subobj11a.test \
subobj11b.test \
-subobj11c.test \
subobjname.test \
subpkg.test \
subpkg2.test \
diff --git a/tests/subobj11c.test b/tests/subobj11c.test
deleted file mode 100755
index ac9940a..0000000
--- a/tests/subobj11c.test
+++ /dev/null
@@ -1,53 +0,0 @@
-#! /bin/sh
-# Copyright (C) 2010 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/>.
-
-# Automatic dependency tracking with subdir-objects option active:
-# check for a patologic case of slash-collapsing in the name of
-# included makefile fragments (containing dependency info).
-# See also related tests `subobj11a.test' and `subobj11b.test'
-
-. ./defs || Exit 1
-
-set -e
-
-cat >> configure.in << 'END'
-AC_PROG_CC
-AM_PROG_CC_C_O
-END
-
-cat > Makefile.am << 'END'
-AUTOMAKE_OPTIONS = subdir-objects
-bin_PROGRAMS = foo
-foo_SOURCES = //zardoz.c
-END
-
-$ACLOCAL
-$AUTOMAKE -a
-
-#
-# This check depends on automake internals, but presently this is
-# the only way to test the code path we are interested in.
-# Please update these checks when (and if) the relevant automake
-# internals are changed.
-#
-# Be a little lax in the regexp, to account for automake conditionals,
-# quoting, and similar stuff.
-#
-# FIXME: Are we sure this is the most sensible output in our situation?
-#
-grep '^[^/]*am__include[^/]*//\$(DEPDIR)/zardoz\.[^/]*$' Makefile.in
-
-:
--
1.7.1
