Re: 20-distdir-am.patch

[Akim Demaille]

Pavel Roskin [EMAIL PROTECTED] writes:

 Hello, Akim!
 
 Another problem. Test pr87.test fails. It fails in a makefile that
 contains the following fragment:
 
 distdir = $(top_builddir)/$(PACKAGE)-$(VERSION)/$(subdir)
 
 @for file in $(DISTFILES); do \
 
 distdir: $(DISTFILES)
 @for file in $(DISTFILES); do \
   d=$(srcdir); \
   if test -d $$d/$$file; then \
 
 Now let's check where 'for file in $(DISTFILES)' comes from.
 
 $ find -type f -exec egrep -nH 'for file in .?\$\(DISTFILES\)' {} \;
 ./m4/Makefile.in:130:   @for file in $(DISTFILES); do \
 ./Makefile.in:515:  @for file in $(DISTFILES); do \
 ./automake.in:2687:$output_rules .= "\t\@for file in \$(DISTFILES); do
 \\\n";
 ./tests/Makefile.in:425:@for file in $(DISTFILES); do \
 ./distdir.am:40:@for file in $(DISTFILES); do \
 
 So, you added it to distdir.am but didn't remove it from automake.in (it
 is handle_dist_worker() that you claim to have adjusted). I guess you
 forgot to commit something else.
 
 Also texinfo8.test fails exactly with the same symptoms.

I'm sorry.  Some recent changes from Tom conflicted with this area,
and I tried to solve the conflicts from my home, and apparently broke
it in some way.  Please, do not hesitate to fix this if you see what
happens.  Maybe reading the patch I sent can give hints where I messed
Automake up.

I'm getting worried with my Automake patches stack, but I _perfectly_
understand Tom's cautious approach.  I really apologize for the
mixture I made, but really I don't want to handle this now: it is too
uncomfortable, which results in my making mistakes (I did run the test
suite for instance.  Don't ask me how I manage to have it run here).

20-distdir-am.patch

[Akim Demaille]

Actually, it seems to me there is not enough paragraph rules at all in
automake.  Instead of the attrocious

@SUBDIRS@   for subdir in $(@DIST_SUBDIR_NAME@); do \
@SUBDIRS@ if test "$$subdir" = .; then :; else \
@SUBDIRS@   test -d $(distdir)/$$subdir \
@SUBDIRS@   || mkdir $(distdir)/$$subdir \
@SUBDIRS@   || exit 1; \
@SUBDIRS@   (cd $$subdir  \
@SUBDIRS@ $(MAKE) $(AM_MAKEFLAGS) top_distdir=../$(@TOP_DISTDIR@) 
distdir=../$(distdir)/$$subdir distdir) \
@SUBDIRS@ || exit 1; \
@SUBDIRS@ fi; \
@SUBDIRS@   done

one would want some for of conditional, such as

@if HAS_SUBDIRS@
for subdir in $(@DIST_SUBDIR_NAME@); do \
  if test "$$subdir" = .; then :; else \
test -d $(distdir)/$$subdir \
|| mkdir $(distdir)/$$subdir \
|| exit 1; \
(cd $$subdir  \
  $(MAKE) $(AM_MAKEFLAGS) top_distdir=../$(@TOP_DISTDIR@) 
distdir=../$(distdir)/$$subdir distdir) \
  || exit 1; \
  fi; \
done
@fi HAS_SUBDIRS@

or whatever the actual syntax.  This means that we want to run the
COMMAND of file_contents on the *whole* file.  Given that we have a
paragraph reading afterwards, it is absolutely possible.

Just my $0.002.  Tell me what you think about it.


So the point of this patch is (unofficially, that I continue learning
and feeling about Automake to get an idea of what could be done to
make it easier to understand/extend/migrate to another language) is to
externalize the handling of the dist target.  Because it was on my way
to use even more %dependencies.

I'm sorry for the size, one can't do less IMHO.

Note that aside from the new file to distribute (distdir.am), the
changes of the Makefile.ins are limited to a new empty line (of which
we can get rid of but eliminating the leading empty lines in
distdir.am, or, alternatively, decide that file_contents should
always get rid of leading lines).

Akim

Index: ChangeLog
from  Akim Demaille  [EMAIL PROTECTED]
* distdir.am (distdir): New file, extracted from...
* automake.in (handle_dist_worker): here.
Adjust.

Index: automake.in
--- automake.in Wed, 31 Jan 2001 21:12:00 +0100 akim (am/f/39_automake.i 1.26 755)
+++ automake.in Wed, 31 Jan 2001 22:25:04 +0100 akim (am/f/39_automake.i 1.26 755)
@@ -2613,28 +2613,26 @@ sub handle_multilib
 # Worker for handle_dist.
 sub handle_dist_worker
 {
-local ($makefile) = @_;
-
-$output_rules .= 'distdir: $(DISTFILES)' . "\n";
+my $makefile = @_;
+my $xform = '';

 # Initialization; only at top level.
 if ($relative_dir eq '.')
 {
-   if (defined $options{'check-news'})
-   {
-   # For Gnits users, this is pretty handy.  Look at 15 lines
-   # in case some explanatory text is desirable.
-   $output_rules .= '  @if sed 15q $(srcdir)/NEWS | fgrep -e "$(VERSION)"  
/dev/null; then :; else \\
- echo "NEWS not updated; not releasing" 12; \\
- exit 1; \\
-   fi
-';
-   }
-
+$xform .= 's/\@TOPDIR\@//g;';
+}
+else
+{
+$xform .= 's/\@TOPDIR\@.*//g;';
+}

-   # Create dist directory.
-   $output_rules .= ("\t-chmod -R a+w \$(distdir)  /dev/null 21; rm -rf 
\$(distdir)\n"
- . "\tmkdir \$(distdir)\n");
+if (defined $options{'check-news'})
+{
+$xform .= 's/\@CK-NEWS\@//g;';
+}
+else
+{
+$xform .= 's/\@CK-NEWS\@.*//g;';
 }

 # Scan EXTRA_DIST to see if we need to distribute anything from a
@@ -2674,39 +2672,23 @@ sub handle_dist_worker
# hash lets us ensure that each directory is used only once.
local (%dhash);
grep ($dhash{'$(distdir)/' . $_} = 1, @dist_dirs);
-   $output_rules .= "\t";
-   pretty_print_rule ('$(mkinstalldirs)', "\t   ", sort keys %dhash);
+   $xform .= transform ('DISTDIRS', join (' ', sort keys %dhash));
+}
+else
+{
+$xform .= 's/.*\@DISTDIRS\@.*//g;';
 }

-# In loop, test for file existence because sometimes a file gets
-# included in DISTFILES twice.  For example this happens when a
-# single source file is used in building more than one program.
-# Also, there are situations in which "ln" can fail.  For instance
-# a file to distribute could actually be a cross-filesystem
-# symlink -- this can easily happen if "gettextize" was run on the
-# distribution.
-$output_rules .= "\t\@for file in \$(DISTFILES); do \\\n";
 if ($cygnus_mode)
 {
-   $output_rules .= "\t  if test -f \$\$file; then d=.; else d=\$(srcdir); fi; 
\\\n";
+$xform .= 's/\@CYGNUS\@\t*/\t/g;';
+$xform .= 's/.*\@NCYGNUS\@.*//g;';
 }
 else
 {
-   $output_rules .= "\t  d=\$(srcdir); \\\n";
+$xform .= 's/\@NCYGNUS\@\t*/\t/g;';
+$xform .= 's/.*\@CYGNUS\@.*//g;';
 }
-$output_rules .= ("\t  if test -d \$\$d/\$\$file;