Re: [PATCH] Check that symlinks are resolved by `make dist'.

[Stefano Lattarini]

At Sunday 04 April 2010, Ralf Wildenhues <address@hidden> 
wrote:
> Hello Stefano,
> 
> * Stefano Lattarini wrote on Fri, Apr 02, 2010 at 12:29:49AM CEST:
> > > I'll write a couple of tests soonish, making them SKIP'd if `ln
> > > -s' fails, or if `test -h' fails when applied to the resulting
> > > "symlink".
> 
> Why?  I mean, it's fine to skip if 'ln -s' fails outright, but if
>  it's emulated by 'cp -p', there is no need to skip the test?
Good observation, that skip would only reduce the coverage offered by 
the test without any real good reason.

> 
> > > By the way, how is the Automake-generated Makefile expected to
> > >  behave if a file in e.g. EXTRA_DIST is a broken symlink,
> >
> > ... this is still not tested (I don't know what result should be
> > expected in such a situation), and...
> 
> Well, what happens currently?  (I hope 'make dist' fails.)
Yep, `make distdir' fails (and thus `make dist' fails too).

Let's assume that a broken symlink `lnk' is added to EXTRA_DIST.
GNU make 3.81 and 3.75 fail with:
  make: *** No rule to make target `lnk', needed by `distdir'.  Stop.
Heirloom make "@(#)make.sl  1.40 (gritter) 3/15/07" fails with:
  make: fatal error: don't know how to make lnk1 (bu42)
Debian freebsd-make (from package "freebsd-buildutils", version 7.2-1) 
fails with:
  make: don't know how to make lnk. Stop

However, these are not very clear error messages IMHO.

> > >  or a symlink pointing to a directory?
> >
> > ... the test for this is xfailing, since a symlink pointing to a
> > directory is not expanded to the corresponding directory when
> > copied in $(distdir); this is probably due to the use of `cp
> > -fpR' as "directory-copying command" in distdir.am, where the
> > `-R' option causes GNU cp not to resolve symlinks -- POSIX seems
> > to leave unspecified the behaviour of `cp -R' w.r.t. symlinks, if
> > I understand correctly the description at:
> > 
> > <http://www.opengroup.org/onlinepubs/009695399/utilities/cp.html>
> >
> > So what is the right thing to do about this -- patch distdir.am
> > or modify the semantic of the new test?
> 
> I don't think expanding the directory is desirable in this case. 
> It could easily lead to file explosion.
Yes, expanding the directory seemed a little "dangerous" to me too, 
but since I had mixed feelings about the issue I wanted to hear your 
opinion and follow it.
> I'm not sure whether to leave that open as loop-hole for people 
> wanting symlinks in the distribution, or to offer an Automake option
> not to expand symlinks at all.
> We don't need to hurry to define this unless and until we have a
> good way to do so.  Let's leave this undefined.
I agree about this; I have rewritten the patch removing the extra test 
script `distsolvelinksdir.test'.
But maybe we could also patch distdir.am to make it issue a warning 
when a symlink to a directory is being distributed (while continuing 
to behave as it previously did in all other respects).

> 
> > From ee859147f10c073a6d7bae822e6a4eb259f35e94 Mon Sep 17 00:00:00
> > 2001 From: Stefano Lattarini <address@hidden>
> > Date: Wed, 31 Mar 2010 23:41:01 +0200
> > Subject: [PATCH] Check that symlinks are resolved by `make dist'.
> >
> > * tests/distsolvelinks.test: New test.
> 
> make that s/distsolvelinks/distlinks/ or so, that should be clear
> enough.
Agreed.
> 
> > * tests/distsolvelinksdir.test: Likewise.
> > * tests/Makefile.am (TESTS, XFAIL_TESTS): Updated accordingly.
> > Suggested by observations from Ralf Wildenhues.
> >
> >
> > --- /dev/null
> > +++ b/tests/distsolvelinks.test
> >
> > +# Check that distributed symlinks in the source tree will be
> > expanded +# as regular files in $(distdir).
> > +
> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +echo text > file
> > +
> > +ln -s file lnk \
> >
> > +  && cmp file lnk \
> > +  && test -h lnk \
> > +  && test ! -h file \
> 
> You can just omit these three lines I think,
OK, done.
> 
> > +  || {
> > +    echo "$me: cannot make and/or detect symlinks" >&2
> 
> ... and adjust the message.
Done.
> 
> > +    Exit 77
> > +  }
> > +
> > +mkdir A
> > +mkdir B
> > +echo aaa > A/aaa
> > +cd B
> > +ln -s ../A/aaa bbb
> > +cd ..
> > +
> > +echo FooBarBaz > foo
> > +
> > +ln -s foo  bar1
> > +ln -s bar1 bar2
> > +ln -s bar2 bar3
> > +
> > +ln -s "`pwd`/foo" quux
> > +
> > +echo AC_OUTPUT >>configure.in
> > +
> > +cat > Makefile.am << 'END'
> > +EXTRA_DIST = lnk B/bbb bar1 bar2 bar3 quux
> > +.PHONY: printdistdir
> > +printdistdir:
> > +   @echo $(distdir)
> > +END
> > +
> > +ls -l . A B
> > +
> > +$ACLOCAL
> > +$AUTOCONF
> > +$AUTOMAKE
> > +
> > +./configure
> > +$MAKE distdir
> > +distdir=`$MAKE printdistdir` || Exit 1
> 
> Don't try to get variables set through output from 'make', that's
>  very error-prone.
That was necessary with my previous "paranoid" approach, as I checked
that `test -h' and worked correctly *for the current shell* only, not
for the one spawned by $MAKE to execute target-associated commands.
Now that the sanity checks on `test -h' are dropped, this is no more
necessary, so I followed your advice.
>  Instead, move this:
> > +ls -l $distdir $distdir/B
> > +
> > +set  file lnk  A/aaa B/bbb  foo quux  foo bar1  foo bar2  foo
> > bar3 +while test $# -gt 0; do
> > +  file=$1; shift; link=$1; shift;
> > +  test -f $distdir/$link
> > +  test ! -h $distdir/$link
> > +  diff $file $distdir/$link
> > +done
> 
> into a rule in the makefile, as is done in several other tests.
> That rule can just have 'distdir' as prerequisite.
Done (with some minor bells&whistles).

Regards,
    Stefano

From c67de3e88043cbd82ea1842a14eb77e41a56f979 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Wed, 31 Mar 2010 23:41:01 +0200
Subject: [PATCH] Check that symlinks are resolved by `make dist'.

* tests/distlinks.test: New test.
* tests/Makefile.am (TESTS): Updated accordingly.
Suggested by observations from Ralf Wildenhues.
---
 ChangeLog            |    7 +++++
 tests/Makefile.am    |    1 +
 tests/Makefile.in    |    1 +
 tests/distlinks.test |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 82 insertions(+), 0 deletions(-)
 create mode 100755 tests/distlinks.test

diff --git a/ChangeLog b/ChangeLog
index a5b5426..35f81f4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2010-04-04  Stefano Lattarini  <address@hidden>
 
+       Add tests checking that symlinks are resolved by `make dist'.
+       * tests/distlinks.test: New test.
+       * tests/Makefile.am (TESTS): Updated accordingly.
+       Suggested by observations from Ralf Wildenhues.
+
+2010-04-04  Stefano Lattarini  <address@hidden>
+
        Generated tests are now just a thin layer around other tests.
        * tests/Makefile.am: Rewrite the rule to generate the `*-p.test'
        test scripts so that any of them simply includes the corresponding
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 62ad6aa..3626b37 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -284,6 +284,7 @@ distcom5.test \
 distcom6.test \
 distcom7.test \
 distdir.test \
+distlinks.test \
 distname.test \
 dollar.test \
 dollarvar.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 295d259..daddd45 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -524,6 +524,7 @@ distcom5.test \
 distcom6.test \
 distcom7.test \
 distdir.test \
+distlinks.test \
 distname.test \
 dollar.test \
 dollarvar.test \
diff --git a/tests/distlinks.test b/tests/distlinks.test
new file mode 100755
index 0000000..2595e65
--- /dev/null
+++ b/tests/distlinks.test
@@ -0,0 +1,73 @@
+#! /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/>.
+
+# Check that distributed symlinks in the source tree will be expanded
+# as regular files in $(distdir).
+
+. ./defs || Exit 1
+
+set -e
+
+echo text > file
+
+ln -s file lnk || {
+  echo "$me: cannot make symlinks" >&2
+  Exit 77
+}
+
+mkdir A
+mkdir B
+echo aaa > A/aaa
+cd B
+ln -s ../A/aaa bbb
+cd ..
+
+echo FooBarBaz > foo
+
+ln -s foo  bar1
+ln -s bar1 bar2
+ln -s bar2 bar3
+
+ln -s "`pwd`/foo" quux
+
+echo AC_OUTPUT >>configure.in
+
+echo "me = $me" > Makefile.am  # for better failure messages
+cat >> Makefile.am << 'END'
+EXTRA_DIST = lnk B/bbb bar1 bar2 bar3 quux
+.PHONY: test
+test: distdir
+       ls -l $(distdir) $(distdir)/B
+       fail() { echo "$(me): $$*" >&2; e=1; }; \
+       e=0; \
+       set file lnk A/aaa B/bbb foo quux foo bar1 foo bar2 foo bar3; \
+       while test $$# -gt 0; do \
+         file=$$1; shift; link=$(distdir)/$$1; shift; \
+         test -f $$link || fail "$$link is not a regular file"; \
+         test ! -h $$link || fail "$$link is a symlink"; \
+         diff $$file $$link || fail "$$link differs from $$file"; \
+       done; \
+       exit $$e;
+END
+
+ls -l . A B
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+./configure
+$MAKE test
-- 
1.6.5