Re: Bug in gnulib-tools prevents bison from bootstrapping

[Eric Blake-1]

> > Note that _AC_PATH_PROG_FEATURE_CHECK already has a bug in its current
> > usage in programs.m4 - it is currently only used by AC_PROG_GREP and
> > AC_PROG_SED, both of which invoke it inside AC_CACHE_CHECK.  Therefore,
> > its attempt to do AC_SUBST([$1]) is useless - if you run ./configure -C
> > with the program variable unset, the first run will set the variable,
> but
> > the second run will leave it unset.  Fortunately, both AC_PROG_GREP and
> > AC_PROG_SED (and my patch below) work around this bug, by re-calling
> > AC_SUBST(variable) after the cache context ends.
> 
> I see no AC_SUBST(M4) in your patch, though.

The AC_SUBST is implicit, via the AC_ARG_VAR.  So I only
needed M4=$ac_cv_path_M4 after the program check.

> > I'm thinking a public macro should look like:
> > 
> > # AC_PATH_PROG_FEATURE_CHECK(VARIABLE, PROGNAME-LIST, MSG,
> > #                            FEATURE-TEST, [PATH=$PATH])
> > # --------------------------------------------------------
> 
> The name 'AC_PATH_PROGS_FEATURE_CHECK' is more accurate: the macro follows
> AC_PATH_PROGS more than AC_PATH_PROG.

Agreed.

>   The MSG argument is unusual in an
> Autoconf macro; typically the calling code would do that itself, if
> needed.
> 
> > # Cache a search for a program, using MSG as in AC_CACHE_CHECK.

The only reason for MSG in my suggestion is because of the tie-in
with AC_CACHE_VAL.  As I discovered, attempting:
 AC_CACHE_CHECK([msg], [ac_cv_path_PROG],
  [_AC_PATH_PROG_FEATURE_CHECK([PROG], [proga progb],
    [feature-check])])
might not set PROG, depending on the existence of the cache, even though
  AC_CHECK_PROGS([PROG], [proga progb])
always sets PROG.  If nothing else, in my earlier testing of the
patch, I got confused when M4 was set the first time but not the
second, before I added the explicit M4=$ac_cv_path_M4.

What makes this awkward is that since FEATURE-TEST requires the
manipulation of ac_cv_path_PROG to end the _AC_PATH_PROG_FEATURE_CHECK
loop successfully, it does not make much sense to call
_AC_PATH_PROG_FEATURE_CHECK outside of a cache context (which means
there must not be any side effects, such as setting VARIABLE or using
AC_SUBST([VARIABLE]); but users probably expect a macro with similar
semantics to AC_CHECK_PROGS.  So my thought was to have a public macro
wrap the cache check, in which case it needs a message.

The one other question I have come up with in the meantime is whether,
if VARIABLE is already set, we should still run it through
FEATURE-CHECK.  In the current implementation of AC_PROG_SED, if you
run ./configure SED=/bad/sed, configure will blindly use /bad/sed,
even though ./configure PATH=/bad:$PATH recognizes that /bad/sed fails
the feature check.

> Aborting should not be the fixed failure resolution of a public, generic
> macro.
> AC_PATH_PROGS has a VALUE-IF-NOT-FOUND argument; it is probably best to
> keep
> that pattern.  Callers can test for that value and abort.  On the other
> hand,
> but for current practice, an ACTION-IF-NOT-FOUND allows for more intuitive
> control flow in the caller.

Agreed.  How about the following two prototypes, then?

# AC_PATH_PROGS_FEATURE_CHECK(VARIABLE, PROGNAME-LIST,
#                             FEATURE-TEST, [ACTION-IF-NOT_FOUND],
#                             [PATH=$PATH])
# ----------------------------------------------------------------
# Designed to be used inside AC_CACHE_VAL.  It is recommended,
# but not required, that the user also use AC_ARG_VAR([VARIABLE]).
# If VARIABLE is not empty, set the cache variable
# $ac_cv_path_VARIABLE to VARIABLE without any questions.
# Otherwise, call FEATURE_TEST repeatedly with $ac_path_VARIABLE
# set to the name of a program in PROGNAME-LIST found in PATH.  If
# no invocation of FEATURE-TEST sets $ac_cv_path_VARIABLE to the
# path of an acceptable program, ACTION-IF-NOT-FOUND is executed.
# FEATURE-TEST is invoked even when $ac_cv_path_VARIABLE is set,
# in case a better candidate occurs later in PATH; to accept the
# current setting without further checks, FEATURE-TEST should set
# $ac_path_VARIABLE_found=':'.  Note that, unlike AC_CHECK_PROGS,
# this macro does not have any side effect on the current value
# of VARIABLE.  See also AC_CACHE_PATH_PROGS_FEATURE_CHECK.

# AC_CACHE_PATH_PROGS_FEATURE_CHECK(VARIABLE, PROGNAME-LIST, MSG,
#                                   FEATURE-TEST,
#                                   [ACTION-IF-NOT-FOUND],
#                                   [PATH=$PATH])
# ---------------------------------------------------------------
# Designed to make caching AC_PATH_PROGS_FEATURE_CHECK easier.  It
# is recommended, but not required, that the user also use
# AC_ARG_VAR([VARIABLE]).  Perform a cache check, using MSG as in
# AC_CACHE_CHECK.  If VARIABLE is set, set the cache variable
# $ac_cv_path_VARIABLE to VARIABLE without any questions.
# Otherwise, call FEATURE_TEST repeatedly with $ac_path_VARIABLE
# set to the name of a program in PROGNAME-LIST found in PATH.  If
# no invocation of FEATURE-TEST sets $ac_cv_path_VARIABLE to the
# path of an acceptable program, then ACTION-IF-NOT-FOUND is run.
# If a suitable $ac_path_VARIABLE is found in the FEATURE-TEST
# macro, it can set $ac_path_VARIABLE_found=':' to accept that
# value without any further checks.  Calls AC_SUBST for
# VARIABLE, with the final value of $ac_cv_path_VARIABLE.

-- 
Eric Blake

-- 
View this message in context: 
http://www.nabble.com/Re%3A-Bug-in-gnulib-tools-prevents-bison-from-bootstrapping-tf4360008.html#a12430388
Sent from the Gnu - Autoconf - Patches mailing list archive at Nabble.com.