[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] use m4_require to implement AS_REQUIRE

From: Eric Blake
Subject: Re: [PATCH] use m4_require to implement AS_REQUIRE
Date: Tue, 14 Oct 2008 06:59:30 -0600
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20080914 Thunderbird/ Mnenhy/

Hash: SHA1

According to Paolo Bonzini on 10/12/2008 10:20 AM:
> Hi, I know I should be writing testcases for AS_ME_PREPARE. :-) But
> while I was looking at adding shell functions to Autoconf proper, I
> decided that the current basic implementation of AS_REQUIRE is not
> scalable.

OK, I've reviewed this.  I think the following quoting change is needed,
but then the patch can be applied.

> +m4_defun([AS_REQUIRE],
> +[m4_define([_m4_divert_desired], m4_default_quoted([$3], [M4SH-INIT]))dnl
> +m4_if(m4_eval(_m4_divert(_m4_divert_dump) <= 
> _m4_divert(_m4_divert_desired)), 1,

These two lines do the wrong thing if INIT is m4_define'd at this point,
since you are defining _m4_divert_desired to an unquoted name (the quoting
provided by m4_default_quoted is stripped by the argument parsing of
m4_define).  One way to work around this is to make sure
_m4_divert_desired expands to a quoted name, by changing the first line:

m4_define([_m4_divert_desired], [m4_default_quoted([$3], [M4SH-INIT])])dnl

> +      [m4_require([$1], [$2])],
> +      [m4_divert_require([_m4_divert_desired], [$1], [$2])])])

Then, as you observed, if m4_divert_require expands its first argument,
this will do the right thing.

> +# m4_divert_require(DIVERSION, NAME-TO-CHECK, [BODY-TO-EXPAND])
> +# --------------------------------------------------------------
> +# Same as m4_require, but BODY-TO-EXPAND goes into the named diversion;
> +# requirements still go in the current diversion though.
> +#
> +m4_define([m4_divert_require],
> +m4_do([[m4_ifdef([_m4_expanding($2)],
> +              [m4_fatal([$0: circular dependency of $2])])]],
> +      [[m4_ifdef([_m4_divert_dump], [],
> +              [m4_fatal([$0($2): cannot be used outside of an m4_defun'd 
> macro])])]],
> +      [[m4_provide_if([$2],
> +                   [],
> +                   [_m4_require_call([$2], [$3], [$1])])]]))

m4_do is a bit of an overkill (yes, I know I've used it in places, but not
recently).  If you want, you could rewrite this as:

  [m4_fatal([$0: circular dependency of $2])])]dnl
[m4_ifdef([_m4_divert_dump], [],
  [m4_fatal([$0($2): cannot be used outside of an m4_defun'd macro])])]dnl
[m4_provide_if([$2], [],
  [_m4_require_call([$2], [$3], [$1])])])

> +
>  # m4_defun(NAME, EXPANSION)
>  # -------------------------
>  # Define a macro which automatically provides itself.  Add machinery
> @@ -1681,12 +1695,13 @@ m4_do([[m4_ifdef([_m4_expanding($1)],
>  m4_bmatch([$0], [^AC_], [[AC_DEFUN]], [[m4_defun]])['d macro])])]],
>        [[m4_provide_if([$1],
>                     [],
> -                   [_m4_require_call([$1], [$2])])]]))
> +                   [_m4_require_call([$1], [$2], 
> [_m4_defn([_m4_divert_dump])])])]]))

With the quoting change to AS_REQUIRE, this means that all clients of
_m4_require_call are passing text that still needs expansion as the third

> -      [[m4_divert(_m4_defn([_m4_divert_dump]))]],
> +      [[m4_divert($3)]],

so I agree with your analysis that you don't need to quote $3 here.

[This is not the only solution; you could also quote $3 here and change
all callers of _m4_require_call to pass a diversion name, rather than
macro text that expands to a diversion name, but as you noted in your
follow-up, that adjusts more lines of your original patch.]

>  ## ------------------------------------ ##
>  ## AS_REQUIRE_SHELL_FN and m4_require.  ##
>  ## ------------------------------------ ##
>  # Hypothesis: M4sh expands the requirements of AS_REQUIRE_SHELL_FN
> -# in the main diversion, and not in M4SH-INIT.
> +# in M4SH-INIT-FN.  This changed after Autoconf 2.63.

AS_REQUIRE_SHELL_FN didn't even exist in 2.63, so I'd just strike the last
sentence of this comment.  And to prove the point of quoting, I'd add an
m4_define([INIT], [oops]) prior to the AS_INIT in this test.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at
Comment: Using GnuPG with Mozilla -


reply via email to

[Prev in Thread] Current Thread [Next in Thread]