Re: Testing automake on Cygwin 1.7

[Peter Rosin]

Stefano Lattarini skrev 2011-11-25 20:00:
> [adding automake-patches]
> 
> Hi Peter, thanks for your work on this.

No problem.

> On Friday 25 November 2011, Peter Rosin wrote:
>> Stefano Lattarini skrev 2011-11-25 16:45:
>>> On Friday 25 November 2011, Peter Rosin wrote:
>>>>
>>>> I have recently (for some value of recently anyway) swapped computer
>>>> and the new one has some problem with the texinfo stuff. I remember
>>>> having to do some fixup for it on the old one but that was years
>>>> ago and I can't remember what I did.  So, these tests xfails on
>>>> my Cygwin-1.7-install as it stands (and, I believe, any Cygwin
>>>> with texinfo out of the box, i.e. not manually repaired).
>>>>
>>> Ouch, that's pretty bad.  Do you know if and how we can detect the
>>> breakage in Cygwin, so that the tests will be skipped rather than
>>> fail?  (This will probably avoid many spurious bug reports, and a
>>> bad impression about automake among the users running the testsuite).
>>> Or better again, there is a simple way to work around this breakage
>>> in automake proper, so that all the out-of-the-box Cygwin 1.7.x
>>> installations will benefit from it?  Also, tht you know, has the
>>> bug already been reported to Cyginw?
>>
>> As it happens I did a bit of digging just before your mail, and yes,
>> it was reported 5+ years ago and I just re-reported it:
>> http://www.cygwin.com/ml/cygwin/2011-11/msg00393.html
>>
> Thanks.
> 
>> I fixed my tex installation after distcheck-override-infodir and
>> instdir-texi had already been run, but I'll go back and rerun them
>> afterwards.
>>
>> Done, three failures left.
>>
>> First is transform2, which is known and expected to fail on Cygwin
>> (there can't be a 'foo' and a 'foo.exe' in the same dir).
>>
> Maybe we could skip the test like this?
> 
>   # On Cygwin there can't be a 'foo' and a 'foo.exe' file in the same
>   # directory, so this test would be bounded to fail.
>   : > foo.exe
>   : > foo
>   ls foo* # For debugging.
>   test `ls foo* | wcl -l` -eq 2 \
>     || skip_ "cannot have 'foo' and a 'foo.exe' file in the same dir"
>   rm -f foo foo.exe

Yes, that works (with s/wcl/wc/). But see below...

>> Second is txinfo21, which stumbles because the directory timestamp
>> is not properly updated when a file changes inside the dir.
>>
>> From the end of the log:
>> ...
>> make[1]: Leaving directory `/home/peda/automake/tests/txinfo21.dir'
>> + test -d main.html
>> + test -d sub/main2.html
>> + test -d rec/main3.html
>> + is_newest main.html main.texi
>> ++ find main.html main.texi -newer main.html
>> + is_newest_files=main.html/index.html
>> + test -z main.html/index.html
>> + exit_status=1
>> + set +e
>> + cd /home/peda/automake/tests
>> + case $exit_status,$keep_testdirs in
>> + test 0 '!=' 0
>> + echo 'txinfo21: exit 1'
>> txinfo21: exit 1
>> + exit 1
>>
>>
>> Can be fixed like this, but I don't know if that's appropriate...
>>
>> diff --git a/tests/txinfo21.test b/tests/txinfo21.test
>> index ae1d985..3d608e5 100755
>> --- a/tests/txinfo21.test
>> +++ b/tests/txinfo21.test
>> @@ -94,12 +94,12 @@ test -d main.html
>>  test -d sub/main2.html
>>  test -d rec/main3.html
>>
>> -# Rebuilding main.html should cause its timestamp to be updated.
>> -is_newest main.html main.texi
>> +# Rebuilding main.html/index.html should cause its timestamp to be updated.
>> +is_newest main.html/index.html main.texi
>>  $sleep
>>  touch main.texi
>>  $MAKE html
>> -is_newest main.html main.texi
>> +is_newest main.html/index.html main.texi
>>
>>  $MAKE clean
>>  test ! -d main.html
>>
> Hmm... we could make the testing of the directory timestamp conditional
> to the test being run on MinGW/Cygwin, in order not to reduce coverage
> on other systems.  Something like this maybe?
> 
>   # At least on Cygwin, the timestamp of a directory might not be
>   # properly updated when a file inside it is changed, and we need
>   # to account for that.
>   mkdir tmp.d
>   : > tmp.f
>   : > tmp.d/tmp.f
>   $sleep
>   touch tmp.d/tmp.f
>   if is_newest tmp.d tmp.f; then
>      have_correct_dir_timestamp=yes
>   else
>      have_correct_dir_timestamp=no
>   fi
>   rm -rf tmp.*
> 
>   # Rebuilding main.html should cause the timestamp of the regular
>   # file `main.html/index.htm' to be updated.
>   is_newest main.html/index.html main.texi
>   # Rebuilding main.html should cause its timestamp to be updated,
>   # But main.html is a directory, so be prepared to account for
>   # the Cygwin limitation described above.
>   if test $have_correct_dir_timestamp = yes; then
>     is_newest main.html main.texi || Exit 1
>   fi
>   $sleep
>   touch main.texi
>   $MAKE html
>   if test $have_correct_dir_timestamp = yes; then
>     is_newest main.html main.texi || Exit 1
>   fi
>   is_newest main.html/index.html main.texi

That works, but isn't it an awful lot of code for a corner case? You choice
though.

>>
>> Third is distcheck-override-infodir, which wrongly tries to access files
>> with a leading double slash, see end of log below:
>>
>> ...
>> make[1]: Entering directory 
>> `/home/peda/automake/tests/distcheck-override-infodir.dir/distcheck-override-infodir-1.0/_build'
>> if test x${infodir+set} != xset; then \
>>        ls -l 
>> "//home/peda/automake/tests/distcheck-override-infodir.dir/distcheck-override-infodir-1.0/_inst/blah/blah/foobar/"
>>  || exit 1; \
>>        test -f 
>> "//home/peda/automake/tests/distcheck-override-infodir.dir/distcheck-override-infodir-1.0/_inst/blah/blah/foobar/dir"
>>  || exit 1; \
>>      else \
>>        ls -l "/$infodir/" || exit 1; \
>>        test -f "/$infodir/dir" || exit 1; \
>>      fi
>> ls: cannot access 
>> //home/peda/automake/tests/distcheck-override-infodir.dir/distcheck-override-infodir-1.0/_inst/blah/blah/foobar/:
>>  No such file or directory
>> make[1]: *** [installcheck-local] Error 1
>> make[1]: Leaving directory 
>> `/home/peda/automake/tests/distcheck-override-infodir.dir/distcheck-override-infodir-1.0/_build'
>> make: *** [distcheck] Error 1
>> + exit_status=2
>> + set +e
>> + cd /home/peda/automake/tests
>> + case $exit_status,$keep_testdirs in
>> + test 0 '!=' 0
>> + echo 'distcheck-override-infodir: exit 2'
>> distcheck-override-infodir: exit 2
>> + exit 2
>>
>> I assume it's when both DESTDIR and infodir are empty?
>>
>> Can be fixed like this, but again, I don't know if this is appropriate:
>>
>> diff --git a/tests/distcheck-override-infodir.test 
>> b/tests/distcheck-override-in
>> index 19ad3d1..3cf38c5 100755
>> --- a/tests/distcheck-override-infodir.test
>> +++ b/tests/distcheck-override-infodir.test
>> @@ -32,11 +32,11 @@ info_TEXINFOS = main.texi
>>  ## Sanity check.
>>  installcheck-local:
>>         if test x$${infodir+set} != xset; then \
>> -         ls -l "$(DESTDIR)/$(prefix)/blah/blah/foobar/" || exit 1; \
>> -         test -f "$(DESTDIR)/$(prefix)/blah/blah/foobar/dir" || exit 1; \
>> +         ls -l "$(DESTDIR)$(prefix)/blah/blah/foobar/" || exit 1; \
>> +         test -f "$(DESTDIR)$(prefix)/blah/blah/foobar/dir" || exit 1; \
>>         else \
>> -         ls -l "$(DESTDIR)/$$infodir/" || exit 1; \
>> -         test -f "$(DESTDIR)/$$infodir/dir" || exit 1; \
>> +         ls -l "$(DESTDIR)$$infodir/" || exit 1; \
>> +         test -f "$(DESTDIR)$$infodir/dir" || exit 1; \
>>         fi
>>  END
>>
> This lookss good, thanks.  Maybe we could also add a comment stating
> that the lack of a slash after $(DESTDIR) is deliberate?

I don't think we should, because it is never correct to add the
extra slash after $(DESTDIR) and adding a comment about it just
adds to the confusion by somehow stating that it would be
desirable to add that slash, but that we sadly can't. On the
contrary, it's just plain wrong with the slash, and you'd have
to add that comment all over the place in order to be consistent.
But again, your choice.

>> So, with those two fixes, one fail left, i.e. transform2.test.
>>
> Could you let me know if my proposed change above make it correctly
> skipped?  If yes, I'll prepare three proper patches tomorrow -- unless
> you want to beat me at it ;-), in which case, please put a reference
> to this thread in the git commit message and in the ChangeLog entries.

It would be better with an xfail in my opinion, if that's possible to
accomplish conditionally?

Cheers,
Peter