Re: [PATCHES] yacc: support variable expansions in *YFLAGS definition (was: Re: bug#7800: automake fails to honor `-d' in AM_YFLAGS when variable expansions are involved)

[Ralf Wildenhues]

* Stefano Lattarini wrote on Fri, Jan 07, 2011 at 11:50:51PM CET:
> Subject: [PATCH 2/2] yacc: support variable expansions in *YFLAGS definition.
> 
> This commit fixes automake bug#7800.

OK with nits addressed.

Thanks!
Ralf

> * automake.in (lang_yacc_target_hook): Use 'value_as_list_recursive'
> instead of 'variable_value' to get the value of *YFLAGS variables.
> Related changes.
> ($DASH_D_PATTERN): Removed as obsolete.

Just "Removed." is sufficient.  There is nothing obsolete about this,
this is not public API, and it is merely unneeded now.

> * tests/Makefile.am (XFAIL_TESTS): Remove yflags-var-expand.test.
> * tests/yacc-clean.test: Remove a now-useless workaround.

Strictly speaking, "useless" is not the correct description here.
It is simply not needed any more.  I'd just write "Remove workaround."
but I am aware that I'm far off into picky picky land here already ...

> * NEWS: Update.

> --- a/NEWS
> +++ b/NEWS
> @@ -56,6 +56,12 @@ Bugs fixed in 1.11.0a:
>    - The code for automatic dependency tracking works around a Solaris
>      make bug triggered by sources containing repeated slashes when the
>      `subdir-objects' option was used.
> +
> +  - Automake is now smart enough to detect the presence of the `-d' flag

Let's not brag: s/is now smart enough to detect/now detects/

> +    in the various `*YFLAGS' variables even when their definitions involve
> +    indirections through other variables, such as in:
> +      foo_opts = -d
> +      AM_YFLAGS = $(foo_opts)
>  
>  New in 1.11:
>  

> --- a/automake.in
> +++ b/automake.in
> @@ -208,8 +208,6 @@ my $INCLUDE_PATTERN = ('^include\s+'
>                      . '|(\$\(srcdir\)/' . $PATH_PATTERN . ')'
>                      . '|([^/\$]' . $PATH_PATTERN . '))\s*(#.*)?' . "\$");
>  
> -# Match `-d' as a command-line argument in a string.
> -my $DASH_D_PATTERN = "(^|\\s)-d(\\s|\$)";
>  # Directories installed during 'install-exec' phase.
>  my $EXEC_DIR_PATTERN =
>    '^(?:bin|sbin|libexec|sysconf|localstate|lib|pkglib|.*exec.*)' . "\$";
> @@ -6063,11 +6061,14 @@ sub lang_yacc_target_hook
>  {
>      my ($self, $aggregate, $output, $input, %transform) = @_;
>  
> -    my $flag = $aggregate . "_YFLAGS";
> -    my $flagvar = var $flag;
> -    my $YFLAGSvar = var 'YFLAGS';
> -    if (($flagvar && $flagvar->variable_value =~ /$DASH_D_PATTERN/o)
> -     || ($YFLAGSvar && $YFLAGSvar->variable_value =~ /$DASH_D_PATTERN/o))
> +    my $flagvar = var ($aggregate . "_YFLAGS");
> +    my $YFLAGSvar = var ('YFLAGS');
> +    # We cannot work reliably with conditionally-defined YFLAGS.
> +    $flagvar->check_defined_unconditionally if $flagvar;
> +    $YFLAGSvar->check_defined_unconditionally if $YFLAGSvar;
> +    my @flags = $flagvar ? $flagvar->value_as_list_recursive : ();
> +    my @YFLAGS = $YFLAGSvar ? $YFLAGSvar->value_as_list_recursive : ();
> +    if (grep (/^-d$/, @flags) || grep (/^-d$/, @YFLAGS))
>      {
>       (my $output_base = $output) =~ s/$KNOWN_EXTENSIONS_PATTERN$//;
>       my $header = $output_base . '.h';