Re: 2.63b autotest vs. bison testsuite

[Ralf Wildenhues]

Hi Eric,

* Eric Blake wrote on Thu, Apr 23, 2009 at 06:08:07AM CEST:
> According to Eric Blake on 4/15/2009 2:05 PM:
> >> I'm wondering whether s/AT_CHECK_NOESCAPE/AT_CHECK_EXPAND/g would be
> >> good.  Even with that, the name makes me think that the macro would do
> >> something different with its first argument, rather than arguments
> >> number three and four.  Unfortunately, I don't have a good suggestion
> >> to improve this.
> > 
> > Or even AT_CHECK_UNQUOTED, to match the existing naming of
> > AC_DEFINE/AC_DEFINE_UNQUOTED.  Well, now's the time to make a name change,
> > if any, as part of making it documented.
> 
> I'm thinking of applying this series; any problems?

The first (very minor) one that I see is that your log messages are not
verbose enough for me to follow.  For example, this:

> * lib/autotest/general.m4 (AT_CHECK_NOESCAPE): Keep older,
> undocumented semantics, which mishandle ".
> (AT_CHECK_UNQUOTED): Handle " correctly.

could somehow mention which argument of the macro the change applies to,
and maybe even which way is "correctly".  Keep in mind that the
expectation of what seems correct to you now may be something different
for somebody else, at a different point in time.  (More likely, there
are no expectations ...)

>  The first does the rename mentioned above.

Fine with me.

> The second patch fixes a couple of
> inconsistencies from the previously undocumented interface, to make it
> more like AC_DEFINE_UNQUOTED (there, we only do $, ``, and \ expansion,
> but now " is literal, as if in an unquoted here-doc, rather than causing
> the test to fall apart).

Again, I am not sure how to read this description.  AC_DEFINE_UNQUOTED
differs from AC_DEFINE in its handling of both of its important
arguments.

Does any of the changes cause the first argument of AT_CHECK_UNQUOTED to
be treated differently now?

For the manual, I'd prefer "here-document" or "here document" over
"here-doc".

> The third swaps the order of m4_expand and
> AS_ESCAPE in AT_CHECK, so that even characters inside macro expansions
> that need escaping are protected.

If it's fine with bison, it's fine with me.  I won't stand in the way of
the big animal.

> Of the three, only the last one presents any backwards incompatibilities,
> but as the bison testsuite recently went through some serious hoops to try
> to work around AT_CHECK failing to shell-escape macro contents, it seems
> like it is a true bug fix as opposed to a gratuitous change in behavior.
> I'm presently running several tests to see how many packages even try to
> use macros expanding to \"`$ inside the AT_CHECK arguments, which would be
> altered by this patch.


> +# AT_CHECK_NOESCAPE(COMMANDS, [STATUS = 0], STDOUT, STDERR,
> +#                   [RUN-IF-FAIL], [RUN-IF-PASS])
> +# ---------------------------------------------------------
> +# Obsolete spelling of AT_CHECK_UNQUOTED.
> +m4_define([AT_CHECK_NOESCAPE],
> +[m4_warn([obsolete], [use AT_CHECK_UNQUOTED instead of $0])]dnl
> +[AT_CHECK_UNQUOTED($@)])

Bummer we're moving away from using AC_DEFUN, AU_DEFUN, and autoupdate,
in favor of m4_* nowadays.  ;-)

Cheers, and thanks for your good work, as always,
Ralf