At Sunday 04 April 2010, Ralf Wildenhues <ralf.wildenh...@gmx.de> 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 <stefano.lattar...@gmail.com> > > 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 <stefano.lattar...@gmail.com> 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 <stefano.lattar...@gmail.com> + 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 <stefano.lattar...@gmail.com> + 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
