Re: [Groff] Automake migration proposal

[Ingo Schwarze]

Hi Bertrand,

here is another thing that got broken: the decisions whether or not
to build and install HTML and PDF documentation and examples and
some other stuff.

The problem is that when $make_htmldoc (etc.) is empty,
the test utility is called with one argument (-n) instead of
with two arguments (-n "") and hence returns true instead
of false.  Apparently, these conditionals were never tested
before committing them.

The following patch fixes the problem.


Now how do we proceed?  I guess the problems found so far should
be fixed in the automake branch in any case.  However, what i found
is merely what i inadvertently bumped into.  There are no doubt
more bugs, and bugs like the one below are *very* hard to find in
a code audit of a 150 thousand line diff.  So who is really sure
the diff is of sufficient quality to merge it?  I am certainly not,
and the three subtle bugs i ran into without even starting a real
audit do not make me any more confident.

I'm sorry i sound so negative tonight; i certainly don't want to
scare away contributors.  The fact that after applying a diff of
this size to a tree of this size, most stuff more or less still
builds on various platforms clearly shows that Bertrand has spent
very considerable effort on it.  I mean, in the past, we have often
seen how relatively simple, local patches that could have been
relatively easily tested and reviewed still broke the tree.

So, it *is* clear that quite some effort was spent here, and with
quite some diligence.  But what can we do to make sure we can trust
the result?

Yours,
  Ingo


commit 81daba8c8507feeb9e01e13238f47693e6a571d8
Author: Ingo Schwarze <address@hidden>
Date:   Tue Aug 12 01:21:13 2014 +0200

    correct usage of the test utility
    
    According to POSIX, when called with one argument, test(1) shall
    behave as follows: Exit true (0) if $1 is not null; otherwise, exit
    false.
    
    Consequently, this behaviour is correct:
    
     $ test -n
     $ echo $?
    0
     $ test -n ""
     $ echo $?
    1

diff --git a/configure.ac b/configure.ac
index 0221549..340cae6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -189,13 +189,13 @@ AM_CONDITIONAL([MAKE_DONT_HAVE_RM], [test 
"x$groff_is_rm_defined" = "xno"])
 # Some programs have a "g" prefix if an existing groff installation is detected
 AM_CONDITIONAL([USEPROGRAMPREFIX], [test x$g = xg])
 
-AM_CONDITIONAL([BUILD_INFODOC], [test -n $make_infodoc])
-AM_CONDITIONAL([BUILD_HTML], [test -n $make_htmldoc])
-AM_CONDITIONAL([BUILD_HTMLEXAMPLES], [test -n $make_htmlexamples])
-AM_CONDITIONAL([BUILD_PDFDOC], [test -n $make_pdfdoc])
-AM_CONDITIONAL([BUILD_PDFEXAMPLES], [test -n $make_pdfexamples])
-AM_CONDITIONAL([BUILD_OTHERDOC], [test -n $make_otherdoc])
-AM_CONDITIONAL([BUILD_EXAMPLES], [test -n $make_examples])
+AM_CONDITIONAL([BUILD_INFODOC], [test -n "$make_infodoc"])
+AM_CONDITIONAL([BUILD_HTML], [test -n "$make_htmldoc"])
+AM_CONDITIONAL([BUILD_HTMLEXAMPLES], [test -n "$make_htmlexamples"])
+AM_CONDITIONAL([BUILD_PDFDOC], [test -n "$make_pdfdoc"])
+AM_CONDITIONAL([BUILD_PDFEXAMPLES], [test -n "$make_pdfexamples"])
+AM_CONDITIONAL([BUILD_OTHERDOC], [test -n "$make_otherdoc"])
+AM_CONDITIONAL([BUILD_EXAMPLES], [test -n "$make_examples"])
 
 AC_CONFIG_FILES([Makefile])
 AC_CONFIG_FILES([test-groff], [chmod +x test-groff])