FYI: cleanup top_distdir and distdir settings

[Alexandre Duret-Lutz]

There are a few issues with the $(top_distdir) and $(distdir)
variables in current Automake.

When automake builds a tarball, it first creates a $(distdir) 
directory at the top-level, and then call `make distdir' recursively to
fill this directory with files to distribute, adjusing `$(distdir)' at
each level so that each submake puts its files in the right directory.

This works fine because `$(distdir)' is set correctly at the top-level,
and overridden when calling submakes recursively.  However if you look
closely you'll see that foo/Makefile.am defines 
`distdir = ../$(PACKAGE)-$(VERSION)' instead of 
`distdir = ../$(PACKAGE)-$(VERSION)/foo'.  This is confusing, but
harmless since $(distdir) isn't expected to be used outside `make dist'
or `make distdir' where it is overriden from the parent.

The reason `$(distdir)' is passed to submakes is so that it
works with AC_CONFIG_SUBDIRS nested packages, where $(distdir)
should be relative to the package where `make dist' was run, not
to the current subdirectory package.

Another similar variable is `$(top_distdir)'.  As it's name
suggests, this should point to the top directory of the distdir
tree.  At least, this is how it was used in Automake 1.4 (and
probably earlier versions too).   At some point its value changed
and it started to point the the root of the *build* directory
(i.e., one directory before the root of the dist directory).
The top_distdir definition in each Makefile contains as much `../../..'
as needed, and during `make dist' this variable is also passed
to submake for the same reason $(distdir) is.  However top_distdir
is erroneously passed `as-is', without prepending additional `../'.

To summarize these issues:
  - distdir isn't defined correctly in each Makefile, except the top-level
  - top_distdir has misleading semantics: in each Makefile it is defined
    to point to the top of the build directory.
  - top_distdir has the wrong value when passed to submakes by make dist, 

Still and all, it works fine because top_level isn't used, and
distdir is passed to subdirectories with the right value.

I'm installing the following patch on HEAD.  This changes top_distdir
to point back to the right directory, fixes the passing of top_distdir
to submakes, suppresses the definitions of distdir and top_distdir
in all Makefiles but the top-level, documents these two variables,
and use them in the test-suite.

I'm leery of making this change to branch-1-7, since (1) `make dist'
works fine even with these issue, and (2) people might (I hope not!)
rely on the wrong semantic of $(top_distdir) *outside* `make dist'.
I'll only backport some of the documentation reordering.

2003-01-21  Alexandre Duret-Lutz  <address@hidden>

        * lib/am/distdir.am (top_distdir, distdir): Define these
        variables only in top level Makefile.  Redefine $(top_distdir)
        as $(distdir).
        (distdir): Fix passing of $(top_distdir) to sub-makes.
        * automake.in (handle_dist): Do not define the DISTDIR and
        TOP_DISTDIR transformations.  We don't need them anymore.
        * automake.texi (Dist): Reorder the dist-hook section, and move
        the DIST_SUBDIRS paragraph earlier.  Document distdir and
        top_distdir.
        * tests/subpkg.test: Update to check for $(distdir) and
        $(top_distdir).

Index: NEWS
===================================================================
RCS file: /cvs/automake/automake/NEWS,v
retrieving revision 1.200
diff -u -r1.200 NEWS
--- NEWS        10 Jan 2003 17:25:34 -0000      1.200
+++ NEWS        21 Jan 2003 17:58:30 -0000
@@ -1,4 +1,10 @@
 New in 1.7a:
+* Cleanup the definitions of $(distdir) and $(top_distdir).
+  $(top_distdir) now points to the root of the distribution directory
+  created during `make dist', as it did in Automake 1.4, not to the
+  root of the build tree as it did in intervening versions.
+  Furthermore these two variables are now only defined in the top
+  level Makefile, and passed to sub-directories when running `make dist'.
 * elisp sources are compiled all at once, instead of one by one.
   This allows interdependencies and speed up compilation.
 * AM_PROG_CC_STDC is now empty.  The content of this macro was
Index: automake.in
===================================================================
RCS file: /cvs/automake/automake/automake.in,v
retrieving revision 1.1417
diff -u -r1.1417 automake.in
--- automake.in 19 Jan 2003 23:01:03 -0000      1.1417
+++ automake.in 21 Jan 2003 17:58:56 -0000
@@ -4175,10 +4175,6 @@
       if &target_defined ('dist-hook');
     $transform{'DIST-TARGETS'} = join(' ', @dist_targets);
 
-    # Defining $(DISTDIR).
-    $transform{'DISTDIR'} = !variable_defined('distdir');
-    $transform{'TOP_DISTDIR'} = backname ($relative_dir);
-
     $output_rules .= &file_contents ('distdir',
                                     new Automake::Location,
                                     %transform);
Index: automake.texi
===================================================================
RCS file: /cvs/automake/automake/automake.texi,v
retrieving revision 1.318
diff -u -r1.318 automake.texi
--- automake.texi       19 Jan 2003 15:19:26 -0000      1.318
+++ automake.texi       21 Jan 2003 17:59:14 -0000
@@ -4371,9 +4371,18 @@
 Please note that this will also copy @emph{everything} in the directory,
 including CVS/RCS version control files.  We recommend against using
 this feature.
-
 @vindex EXTRA_DIST
 
+If you define @code{SUBDIRS}, Automake will recursively include the
+subdirectories in the distribution.  If @code{SUBDIRS} is defined
+conditionally (@pxref{Conditionals}), Automake will normally include all
+directories that could possibly appear in @code{SUBDIRS} in the
+distribution.  If you need to specify the set of directories
+conditionally, you can set the variable @code{DIST_SUBDIRS} to the exact
+list of subdirectories to include in the distribution (@pxref{Top level}).
address@hidden DIST_SUBDIRS
+
+
 @section Fine-grained distribution control
 
 Sometimes you need tighter control over what does @emph{not} go into the
@@ -4398,25 +4407,6 @@
 
 @section The dist hook
 
-Another way to to use this is for removing unnecessary files that get
-recursively included by specifying a directory in EXTRA_DIST:
-
address@hidden
-EXTRA_DIST = doc
-
-dist-hook:
-       rm -rf `find $(distdir)/doc -name CVS`
address@hidden example
-
-If you define @code{SUBDIRS}, Automake will recursively include the
-subdirectories in the distribution.  If @code{SUBDIRS} is defined
-conditionally (@pxref{Conditionals}), Automake will normally include all
-directories that could possibly appear in @code{SUBDIRS} in the
-distribution.  If you need to specify the set of directories
-conditionally, you can set the variable @code{DIST_SUBDIRS} to the exact
-list of subdirectories to include in the distribution.
address@hidden DIST_SUBDIRS
-
 @trindex dist-hook
 
 Occasionally it is useful to be able to change the distribution before
@@ -4430,6 +4420,39 @@
         mkdir $(distdir)/random
         cp -p $(srcdir)/random/a1 $(srcdir)/random/a2 $(distdir)/random
 @end example
+
+Another way to to use this is for removing unnecessary files that get
+recursively included by specifying a directory in EXTRA_DIST:
+
address@hidden
+EXTRA_DIST = doc
+
+dist-hook:
+       rm -rf `find $(distdir)/doc -name CVS`
address@hidden example
+
address@hidden distdir
address@hidden top_distdir
+Two variables that come handy when writting @code{dist-hook} targets are
address@hidden(distdir)} and @code{$(top_distdir)}.
+
address@hidden(distdir)} points to the directory where the @code{dist} target
+will copy files from the current directory before creating the
+tarball.  If you are at the top-level directory, then @code{distdir =
+$(PACKAGE)-$(VERSION)}.  When used from subdirectory named
address@hidden/}, then @code{distdir = ../$(PACKAGE)-$(VERSION)/foo}.
+
address@hidden(top_distdir)} always points to the root directory of the
+distributed tree.  At the top-level it's equal to @code{$(distdir)}.
+In the @file{foo/} subdirectory
address@hidden = ../$(PACKAGE)-$(VERSION)}.
+
+Note that when packages are nested using @code{AC_CONFIG_SUBDIRS}
+(@pxref{Subdirectories, AC_CONFIG_SUBDIRS, Configuring Other Packages
+in Subdirectories, autoconf, The Autoconf Manual}), then
address@hidden(distdir)} and @code{$(top_distdir)} are relative to the
+package where @code{make dist} was run, not to any sub-packages
+involved.
 
 @section Checking the distribution
 
Index: lib/am/distdir.am
===================================================================
RCS file: /cvs/automake/automake/lib/am/distdir.am,v
retrieving revision 1.43
diff -u -r1.43 distdir.am
--- lib/am/distdir.am   13 Jan 2003 14:22:24 -0000      1.43
+++ lib/am/distdir.am   21 Jan 2003 17:59:16 -0000
@@ -1,5 +1,5 @@
 ## automake - create Makefile.in from Makefile.am
-## Copyright (C) 2001, 2002 Free Software Foundation, Inc.
+## Copyright (C) 2001, 2002, 2003 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
@@ -19,15 +19,10 @@
 ## DIST_COMMON comes first so that README can be the very first file.
 DISTFILES = $(DIST_COMMON) $(DIST_SOURCES) $(TEXINFOS) $(EXTRA_DIST)
 
-top_distdir = %TOP_DISTDIR%
 if %?TOPDIR_P%
-## Avoid unsightly `./'.
-?DISTDIR?distdir = $(PACKAGE)-$(VERSION)
-else !%?TOPDIR_P%
-?DISTDIR?distdir = $(top_distdir)/$(PACKAGE)-$(VERSION)
-endif !%?TOPDIR_P%
+distdir = $(PACKAGE)-$(VERSION)
+top_distdir = $(distdir)
 
-if %?TOPDIR_P%
 am__remove_distdir = \
   { test ! -d $(distdir) \
     || { find $(distdir) -type d ! -perm -200 -exec chmod u+w {} ';' \
@@ -157,13 +152,13 @@
 if %?SUBDIRS%
        list='$(%DIST_SUBDIR_NAME%)'; for subdir in $$list; do \
          if test "$$subdir" = .; then :; else \
-           test -d $(distdir)/$$subdir \
-           || mkdir $(distdir)/$$subdir \
+           test -d "$(distdir)/$$subdir" \
+           || mkdir "$(distdir)/$$subdir" \
            || exit 1; \
            (cd $$subdir && \
              $(MAKE) $(AM_MAKEFLAGS) \
-               top_distdir="$(top_distdir)" \
-               distdir=../$(distdir)/$$subdir \
+               top_distdir="../$(top_distdir)" \
+               distdir="../$(distdir)/$$subdir" \
                distdir) \
              || exit 1; \
          fi; \
Index: tests/subpkg.test
===================================================================
RCS file: /cvs/automake/automake/tests/subpkg.test,v
retrieving revision 1.3
diff -u -r1.3 subpkg.test
--- tests/subpkg.test   8 Sep 2002 13:07:56 -0000       1.3
+++ tests/subpkg.test   21 Jan 2003 17:59:17 -0000
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2002  Free Software Foundation, Inc.
+# Copyright (C) 2002, 2003  Free Software Foundation, Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -36,6 +36,14 @@
 # Yes, This program is named LDADD.  So what?
 bin_PROGRAMS = LDADD
 LDADD_LDADD = lib/liblib.a
+
+# It's ok to override distdir.
+distdir = subpack-1
+
+# Make sure $(distdir) and $(top_distdir) work as expected.
+dist-hook:
+       test -f $(distdir)/LDADD.c
+       test -f $(top_distdir)/LDADD.c
 EOF
 
 cat >LDADD.c <<'EOF'
@@ -63,6 +71,12 @@
 cat >lib/Makefile.am <<'EOF'
 noinst_LIBRARIES = liblib.a
 liblib_a_SOURCES = src/x.c
+
+dist-hook:
+       test ! -f $(distdir)/LDADD.c
+       test -f $(top_distdir)/LDADD.c
+       test -f $(distdir)/src/x.c
+       test ! -f $(top_distdir)/src/x.c
 EOF
 
 cat >lib/src/x.c <<'EOF'
@@ -84,3 +98,4 @@
 
 ./configure
 $MAKE distcheck
+test -f subpack-1.tar.gz

-- 
Alexandre Duret-Lutz