Subject: bug#13524: Improving user experience for non-recursive builds

From: Peter Rosin <[email-masked]>
Date: Thu, 24 Jan 2013 13:22:45 +0100

On 2013-01-23 16:08, Stefano Lattarini wrote:
> On 01/23/2013 03:34 PM, Peter Rosin wrote:
>> On 2013-01-23 13:45, Stefano Lattarini wrote:
*snip*
>>> Too much automagic here IMO.  We'd better have two distinct subst, one for
>>> the "real" directory name, and one for the directory name "canonicalized"
>>> for use in Automake primaries.  I.e., from:
>>
>> The gain was that the '.' case needs to peak at, and perhaps eat, the
>> trailing separator anyway (or it will look bad, I mean, who wants __foo
>> after writing &{CANON_CURDIR}&_foo and curdir happens to be '.'?)
>>
> Good point.  We should allow the user to write something like
> "&{CANON_CURDIR}&foo_SOURCES" instead, so that it can work for the
> current directory too; not much important for human-written makefiles,
> but might be for autogenerated ones (I'm thinking ahead about a Gnulib
> integration here; the current support for non-recursive projects
> there is quite hacky, and could benefit from this new feature).

Are you saying that &{CURDIR}& should be replaced with the empty
string when the current relative directory is "." and the current
relative directory followed by a slash otherwise? (And similar, but
with a trailing underscore for &{CANON_CURDIR}&) I don't fancy
that, as I think &{CURDIR}&gazonk.c is that much harder to read than
&{CURDIR}&/gazonk.c.

So, I'd rather have the magic extend beyond the }& even if that is
ugly indeed. Maybe I'm just deluded to want that...

Or, do you mean that &{CURDIR}& should peak ahead and only add a
trailing "/" if it is not followed by a slash (or whitespace?)
already, making "&{CURDIR}&foo.c" and "&{CURDIR}&/foo.c" equivalent
except when &{CURDIR}& is "." (which would come out as "foo.c" and
"./foo.c" respectively)?

*snip*
> Thanks, I'll take a look at it tomorrow.

Another day, another version. I hope you didn't wast too much time
on v2...

I changed to &{CANON_CURDIR}& and &{CURDIR}&, added support for
$(top_srcdir) and improved the test. There's more code due to the
$(top_srcdir) support, and maybe there are some preexisting
functionality that strips common leading directory components that
could have been reused, but I'm ignorant. Besides, what's wrong
with NIH? :-)

Cheers,
Peter

>From 2b7e4915ce65a56d03a75579cb3e3c40914acd92 Mon Sep 17 00:00:00 2001
From: Peter Rosin <[email-masked]>
Date: Thu, 24 Jan 2013 12:22:45 +0100
Subject: [PATCH] curdir: add support for relative names in included fragments

The rationale for this change is that it is annoying to have
to repeat the directory name when including a Makefile fragment.
For deep directory structures these repeats can generate a lot
of bloat. It also hinders reuse and easy directory restructuring
if all Makefile fragments have to know exactly where they live.

Suggested by Bob Friesenhahn, and later discussed in bug #13524.

automake.in (read_am_file): Add third argument specifying the
relative directory of this Makefile fragment compared to the
main Makefile. Replace &{CURDIR}& and &{CANON_CURDIR}& in the
fragment with this relative directory (with slashes etc, or
canonicalized).
(read_main_am_file): Adjust.
t/curdir.sh: New test.
t/list-of-tests.mk: Augment.
---
 automake.in        |   55 ++++++++++++++++++++++--
 t/curdir.sh        |  115 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/list-of-tests.mk |    1 +
 3 files changed, 166 insertions(+), 5 deletions(-)
 create mode 100755 t/curdir.sh

diff --git a/automake.in b/automake.in
index 0e3b882..f52a439 100644
--- a/automake.in
+++ b/automake.in
@@ -6330,15 +6330,15 @@ sub check_trailing_slash ($\$)
 }

-# &read_am_file ($AMFILE, $WHERE)
-# -------------------------------
+# &read_am_file ($AMFILE, $WHERE, $CURDIR)
+# ----------------------------------------
 # Read Makefile.am and set up %contents.  Simultaneously copy lines
 # from Makefile.am into $output_trailer, or define variables as
 # appropriate.  NOTE we put rules in the trailer section.  We want
 # user rules to come after our generated stuff.
 sub read_am_file ($$)
 {
-    my ($amfile, $where) = @_;
+    my ($amfile, $where, $curdir) = @_;
 
     my $am_file = new Automake::XFile ("< $amfile");
     verb "reading $amfile";
@@ -6423,6 +6423,18 @@ sub read_am_file ($$)
 
 	my $new_saw_bk = check_trailing_slash ($where, $_);
 
+	my $cur_dir = &canonicalize ($curdir);
+	if ($curdir eq '.')
+	  {
+	    # If present, eat the following '_' or '/', converting
+	    # "&{CURDIR}&/foo" and "&{CANON_CURDIR}&_foo" into plain "foo"
+	    # when $curdir is '.'.
+	    $_ =~ s,&\{CURDIR\}&/,,g;
+	    $_ =~ s,&\{CANON_CURDIR\}&_,,g;
+	  }
+	$_ =~ s/&\{CURDIR\}&/${curdir}/g;
+	$_ =~ s/&\{CANON_CURDIR\}&/${cur_dir}/g;
+
 	if (/$IGNORE_PATTERN/o)
 	{
 	    # Merely delete comments beginning with two hashes.
@@ -6559,6 +6571,7 @@ sub read_am_file ($$)
 	elsif (/$INCLUDE_PATTERN/o)
 	{
 	    my $path = $1;
+	    my $new_curdir;
 
 	    if ($path =~ s/^\$\(top_srcdir\)\///)
 	      {
@@ -6571,6 +6584,36 @@ sub read_am_file ($$)
 		# the dependency.
 		# (subdircond2.test and subdircond3.test will fail.)
 		push_dist_common ("\$\(top_srcdir\)/$path");
+		$new_curdir = $path;
+		$new_curdir = '.' if $new_curdir !~ s,/[^/]*$,,;
+		if ($relative_dir ne '.')
+		  {
+		    # Ok, the base Makefile.am is not in the $top_srcdir,
+		    # trim as much as possible and hope that the fragment
+		    # is below the base Makefile.am.
+		    my $dir;
+		    my $rel = $relative_dir;
+		    foreach $dir (split ('/', $rel))
+		      {
+			if ($dir ne $new_curdir
+			    && $dir . "/" ne substr ($new_curdir, 0, length ($dir) + 1))
+			  {
+			    last;
+			  }
+			# Phew, this directory component matched, trim it.
+			substr ($new_curdir, 0, length ($dir) + 1, "");
+			substr ($rel, 0, length ($dir) + 1, "");
+		      }
+		    if ($rel ne '')
+		      {
+			# Sad, the fragment wasn't below the base Makefile.am.
+			# Oh well, do the best we can...
+			$rel =~ s#([^\/]{2,}|[^\.\/])#\.\.#g;
+			$new_curdir = $rel . "/" . $new_curdir;
+		      }
+		    $new_curdir =~ s,/\.?$,,;
+		    $new_curdir = '.' if $new_curdir eq '';
+		  }
 	      }
 	    else
 	      {
@@ -6583,9 +6626,11 @@ sub read_am_file ($$)
 		# (subdircond2.test and subdircond3.test will fail.)
 		push_dist_common ("\$\(srcdir\)/$path");
 		$path = $relative_dir . "/" . $path if $relative_dir ne '.';
+		$new_curdir = $path;
+		$new_curdir = '.' if $new_curdir !~ s,/[^/]*$,,;
 	      }
 	    $where->push_context ("'$path' included from here");
-	    &read_am_file ($path, $where);
+	    &read_am_file ($path, $where, $new_curdir);
 	    $where->pop_context;
 	}
 	else
@@ -6658,7 +6703,7 @@ sub read_main_am_file ($$)
     &define_standard_variables;
 
     # Read user file, which might override some of our values.
-    &read_am_file ($amfile, new Automake::Location);
+    &read_am_file ($amfile, new Automake::Location, '.');
 }

diff --git a/t/curdir.sh b/t/curdir.sh
new file mode 100755
index 0000000..69b82b0
--- /dev/null
+++ b/t/curdir.sh
@@ -0,0 +1,115 @@
+#! /bin/sh
+# Copyright (C) 2013 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test &{CURDIR}& and &{CANON_CURDIR}&.
+
+. test-init.sh
+
+cat >> configure.ac << 'END'
+AC_PROG_CC
+AM_PROG_CC_C_O
+AC_CONFIG_FILES([zot/Makefile])
+AC_OUTPUT
+END
+
+mkdir foo
+mkdir foo/bar
+mkdir foo/foobar
+mkdir zot
+
+cat > Makefile.am << 'END'
+AUTOMAKE_OPTIONS = subdir-objects
+bin_PROGRAMS =
+include $(top_srcdir)/foo/local.mk
+include $(srcdir)/foo/foobar/local.mk
+include local.mk
+END
+
+cat > zot/Makefile.am << 'END'
+AUTOMAKE_OPTIONS = subdir-objects
+bin_PROGRAMS =
+include $(top_srcdir)/zot/local.mk
+include $(top_srcdir)/top.mk
+END
+
+cat > local.mk << 'END'
+&{CANON_CURDIR}&_whoami:
+	@echo "I am &{CURDIR}&/local.mk"
+
+bin_PROGRAMS += &{CURDIR}&/mumble
+&{CANON_CURDIR}&_mumble_SOURCES = &{CURDIR}&/one.c
+END
+
+cat > top.mk << 'END'
+&{CANON_CURDIR}&_top_whoami:
+	@echo "I am &{CURDIR}&/top.mk"
+
+bin_PROGRAMS += &{CURDIR}&/scream
+&{CANON_CURDIR}&_scream_SOURCES = &{CURDIR}&/two.c
+END
+
+cat > one.c << 'END'
+int main(void) { return 0; }
+END
+
+cp local.mk foo
+cp local.mk foo/bar
+cp local.mk foo/foobar
+cp local.mk zot
+echo "include &{CURDIR}&/bar/local.mk" >> foo/local.mk
+
+cp one.c foo
+cp one.c foo/bar
+cp one.c foo/foobar
+cp one.c zot
+cp one.c two.c
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a
+./configure
+
+$MAKE whoami >output 2>&1 || { cat output; exit 1; }
+cat output
+grep "I am local.mk" output
+$MAKE foo_whoami >output 2>&1 || { cat output; exit 1; }
+cat output
+grep "I am foo/local.mk" output
+$MAKE foo_bar_whoami >output 2>&1 || { cat output; exit 1; }
+cat output
+grep "I am foo/bar/local.mk" output
+$MAKE foo_foobar_whoami >output 2>&1 || { cat output; exit 1; }
+cat output
+grep "I am foo/foobar/local.mk" output
+
+$MAKE
+./mumble
+foo/mumble
+foo/bar/mumble
+foo/foobar/mumble
+
+cd zot
+
+$MAKE ___top_whoami >output 2>&1 || { cat output; exit 1; }
+cat output
+grep "I am ../top.mk" output
+$MAKE whoami >output 2>&1 || { cat output; exit 1; }
+cat output
+grep "I am local.mk" output
+
+$MAKE
+./mumble
+../scream
diff --git a/t/list-of-tests.mk b/t/list-of-tests.mk
index 44f598e..1121735 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -340,6 +340,7 @@ t/cscope.tap \
 t/cscope2.sh \
 t/cscope3.sh \
 t/c-demo.sh \
+t/curdir.sh \
 t/cxx.sh \
 t/cxx2.sh \
 t/cxxcpp.sh \
-- 
1.7.9