Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On 2020-10-22, Zack Weinberg wrote: > I acknowledge that requiring double-quotation of AC_INIT arguments > when they contain characters significant to M4 _should_ work; however, > it did not work in my tests (which were not exactly the same as the > above; see the "AC_INIT with unusual version strings" test case in > tests/base.m4, on the branch). Also, it increases the compat hit > we're taking, since e.g. > > AC_INIT(GNU MP, GMP_VERSION, [gmp-b...@gmplib.org, see > https://gmplib.org/manual/Reporting-Bugs.html], gmp) > > which also worked with 2.69, will now be considered invalid, If this works in 2.69 I don't see why this snippet would be rendered invalid if AC_INIT did not over/underquote, because ... > Would you care to propose a complete patch to be applied on top of > zack/ac-init-quoting? In addition to "reverting hunks" you would need > to make sure that AC_PACKAGE_* are always treated consistently within > lib/autoconf/*.m4, fix the testsuite by adding double quotation to AC_INIT > arguments where necessary, and document in both doc/autoconf.texi and NEWS > the changed requirements for AC_INIT arguments. ... I am not suggesting we change any behaviour to AC_INIT arguments wrt. quoting, as compared to Autoconf 2.69. As far as I know this version dutifully follows typical m4 quoting conventions, I am not aware of any specific under/overquotation in existing releases. This underquotation (2.69c) and overquotation (zack/ac-init-quoting branch) is a behaviour change compared to 2.69. I am proposing we NOT change the amount of quoting, but rather we should stick with normal m4 conventions, which would avoid all the AC_INIT-related regressions I've seen reported so far to this list. Anyway, I should have some time on the weekend, I'll see what I can do about proposing a proper patch :) Cheers, Nick
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On 22/10/2020, Zack Weinberg wrote: > On Thu, Oct 22, 2020 at 11:53 AM Nick Bowler wrote: >> On 2020-10-22, Zack Weinberg wrote: >> > On Wed, Oct 21, 2020 at 10:25 PM Paul Eggert >> > wrote: >> >> >> >> On 10/21/20 6:15 AM, Zack Weinberg wrote: >> >> > We*could* add a special case in AC_INIT where, if any of the third, >> >> > fourth, or fifth arguments contain the literal strings >> >> > `AC_PACKAGE_NAME` or `AC_PACKAGE_VERSION`, those are replaced with >> >> > the >> >> > values of the first and second argument, respectively. This would >> >> > keep the GHC code working as-is. I'm not sure whether that's a good >> >> > idea; cc:ing Paul and Eric for their thoughts. >> >> >> >> I'm not following all the details here >> > >> > The concrete problem is that, without the hack I described, we cannot >> > support both >> > >> > AC_INIT([foo], [1.0], [foo-...@foo.org], [foo-AC_PACKAGE_VERSION]) >> > >> > and >> > >> > AC_INIT([bar], [1.0], [foo-bug@[192.0.2.1]]) >> >> I think this is missing the point. The m4 way is that such an >> email address should simply be double quoted to avoid the unwanted >> m4 expansion, for example: >> >> AC_INIT([bar], [1.0], [[foo-bug@[192.0.2.1]]]) > > I tried that and it doesn't work. No amount of extra quotation (ok, I > only went up to four levels before I gave up) will prevent the square > brackets from being lost, if I don't have autoconf use m4_defn to set > the value of the shell variable PACKAGE_BUGREPORT. It works perfectly fine for me with Autoconf-2.69... % cat >configure.ac <<'EOF' AC_INIT([bar], [1.0], [[foo-bug@[192.0.2.1]]]) AS_ECHO(["AC_PACKAGE_BUGREPORT"]) AS_ECHO(["$PACKAGE_BUGREPORT"]) AC_OUTPUT EOF % autoconf-2.69 % ./configure 2.69 foo-bug@[192.0.2.1] foo-bug@[192.0.2.1] configure: creating ./config.status And it also works as expected with the zack/ac-init-quoting branch if I simply revert the patch hunks identified earlier in this thread: % autoconf-zack-patched % ./configure 2.69c.10-6487-dirty foo-bug@[192.0.2.1] foo-bug@[192.0.2.1] configure: creating ./config.status If the hunks are not reverted, quotation problems are readily apparent: % autoconf-zack-unpatched 2.69c.10-6487 foo-bug@[192.0.2.1] [foo-bug@[192.0.2.1]] configure: creating ./config.status (those patch hunks are not the only instances of overquotation added by the patch, I see that the patch also overquotes the bugreport address in the configure --help text) Cheers, Nick
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On Thu, Oct 22, 2020 at 11:53 AM Nick Bowler wrote: > On 2020-10-22, Zack Weinberg wrote: > > On Wed, Oct 21, 2020 at 10:25 PM Paul Eggert wrote: > >> > >> On 10/21/20 6:15 AM, Zack Weinberg wrote: > >> > We*could* add a special case in AC_INIT where, if any of the third, > >> > fourth, or fifth arguments contain the literal strings > >> > `AC_PACKAGE_NAME` or `AC_PACKAGE_VERSION`, those are replaced with the > >> > values of the first and second argument, respectively. This would > >> > keep the GHC code working as-is. I'm not sure whether that's a good > >> > idea; cc:ing Paul and Eric for their thoughts. > >> > >> I'm not following all the details here > > > > The concrete problem is that, without the hack I described, we cannot > > support both > > > > AC_INIT([foo], [1.0], [foo-...@foo.org], [foo-AC_PACKAGE_VERSION]) > > > > and > > > > AC_INIT([bar], [1.0], [foo-bug@[192.0.2.1]]) > > I think this is missing the point. The m4 way is that such an > email address should simply be double quoted to avoid the unwanted > m4 expansion, for example: > > AC_INIT([bar], [1.0], [[foo-bug@[192.0.2.1]]]) I tried that and it doesn't work. No amount of extra quotation (ok, I only went up to four levels before I gave up) will prevent the square brackets from being lost, if I don't have autoconf use m4_defn to set the value of the shell variable PACKAGE_BUGREPORT. zw
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On 2020-10-22, Zack Weinberg wrote: > On Wed, Oct 21, 2020 at 10:25 PM Paul Eggert wrote: >> >> On 10/21/20 6:15 AM, Zack Weinberg wrote: >> > We*could* add a special case in AC_INIT where, if any of the third, >> > fourth, or fifth arguments contain the literal strings >> > `AC_PACKAGE_NAME` or `AC_PACKAGE_VERSION`, those are replaced with the >> > values of the first and second argument, respectively. This would >> > keep the GHC code working as-is. I'm not sure whether that's a good >> > idea; cc:ing Paul and Eric for their thoughts. >> >> I'm not following all the details here > > The concrete problem is that, without the hack I described, we cannot > support both > > AC_INIT([foo], [1.0], [foo-...@foo.org], [foo-AC_PACKAGE_VERSION]) > > and > > AC_INIT([bar], [1.0], [foo-bug@[192.0.2.1]]) I think this is missing the point. The m4 way is that such an email address should simply be double quoted to avoid the unwanted m4 expansion, for example: AC_INIT([bar], [1.0], [[foo-bug@[192.0.2.1]]]) This works already, as expected, in existing versions of Autoconf. But if your package actually used such an email address today, it will be broken by the patch due to the overquotation in AC_INIT. To avoid regressions like the one reported, and to be consistent with how most macros are expected to function, we should simply not overquote in the definition of AC_INIT. Cheers, Nick
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On Wed, Oct 21, 2020 at 10:25 PM Paul Eggert wrote: > > On 10/21/20 6:15 AM, Zack Weinberg wrote: > > We*could* add a special case in AC_INIT where, if any of the third, > > fourth, or fifth arguments contain the literal strings > > `AC_PACKAGE_NAME` or `AC_PACKAGE_VERSION`, those are replaced with the > > values of the first and second argument, respectively. This would > > keep the GHC code working as-is. I'm not sure whether that's a good > > idea; cc:ing Paul and Eric for their thoughts. > > I'm not following all the details here The concrete problem is that, without the hack I described, we cannot support both AC_INIT([foo], [1.0], [foo-...@foo.org], [foo-AC_PACKAGE_VERSION]) and AC_INIT([bar], [1.0], [foo-bug@[192.0.2.1]]) I currently think supporting the latter is more important, based on the not-at-all-scientific observation that one package was broken by avoiding further expansion cycles when AC_PACKAGE_TARNAME is used, but at least three packages were broken by extra expansion cycles (compared to 2.69) eating punctuation in URLs. I'm also, on net, not a fan of the hack. zw
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On 10/21/20 6:15 AM, Zack Weinberg wrote: We*could* add a special case in AC_INIT where, if any of the third, fourth, or fifth arguments contain the literal strings `AC_PACKAGE_NAME` or `AC_PACKAGE_VERSION`, those are replaced with the values of the first and second argument, respectively. This would keep the GHC code working as-is. I'm not sure whether that's a good idea; cc:ing Paul and Eric for their thoughts. I'm not following all the details here, but my kneejerk reaction is that we should avoid hacks like that. If AC_INIT is that poorly designed, the best approach might be to come up with a new macro that is better, and suggest that people use that instead AC_INIT; if so, perhaps the hack you suggest is the best we can do for the now-obsolescent AC_INIT.
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On Wed, 21 Oct 2020 09:15:41 -0400 Zack Weinberg wrote: > On Tue, Oct 20, 2020 at 4:57 PM Nick Bowler wrote: > > On 2020-10-20, Sergei Trofimovich wrote: > > > Initial bug is reported as autoconf failure on ghc-8.8.4: > > > https://bugs.gentoo.org/750191 > > > There autconf 2.69 works, 2.69c does not. > ... > > > $ cat configure.ac > > > AC_INIT([The Glorious Glasgow Haskell Compilation System], [9.1.0], > > > [glasgow-haskell-b...@haskell.org], [ghc-AC_PACKAGE_VERSION]) > > > > > > echo "$PACKAGE_VERSION" > > > > > > AC_OUTPUT > > If I understand correctly, the intention is to have $PACKAGE_VERSION > set to "9.1.0" and $PACKAGE_TARNAME set to "ghc-9.1.0"? I think the intention is to have PACKAGE_VERSION=9.1.0 (and tarball) for a release (RELEASE=YES in configure.ac). For development versions PACKAGE_VERSION (and tarball name) is mangled later in configure.ac after AC_INIT based on current commit: https://github.com/ghc/ghc/blob/master/aclocal.m4#L1646 -- Sergei
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On 2020-10-21, Zack Weinberg wrote: > On Tue, Oct 20, 2020 at 4:57 PM Nick Bowler wrote: >> Note: the change you report is introduced by Zack's fix for related >> AC_INIT quoting regressions. This patch is not included in 2.69c (or >> even on git master), but does seem to be applied by the Gentoo package. > > Yeah, this is a "can't have it both ways" kind of thing. We can > reliably round-trip "unusual" characters (like the ones that appear > all the time in URLs and email addresses) through AC_INIT's arguments, > or we can expand macros in those arguments even when they're quoted on > input; I don't think there's any way to do both. M4 macros (including AC_INIT) should normally follow the m4 quoting rule of thumb, which is that the amount of quotation should exactly equal the depth of macro expansion. Remembering that extra quotation added by macros such as m4_defn and m4_dquote count for this. Previously AC_INIT had not enough quotation, i.e., less levels of quotation than expansion, which lead to unexpected behaviour. Now, with the patch, AC_INIT is adding more levels of quotation than expansion, leading to different unexpected behaviour. M4 macros are happiest when the level of quotation is just right :) > This only works by accident in 2.69, incidentally. AC_PACKAGE_VERSION > is defined *after* AC_PACKAGE_TARNAME (see _AC_INIT_PACKAGE, lines > 235-261 of $prefix/share/autoconf/general.m4) so both old and new > autoconf set AC_PACKAGE_TARNAME to the literal string > "ghc-AC_PACKAGE_VERSION". While I agree it's probably a bit "naughty" to use AC_PACKAGE_VERSION in the argument to AC_INIT it is a red herring. Use of any macro would have the exact same problem. I'd expect double-quoted arguments to AC_INIT to be similarly broken with this patch while previously they would work as expected. > The value undergoes an extra round of expansion when it's used to set > the shell variable PACKAGE_TARNAME (lines 416-428 of the same file). > This extra round of expansion is undesirable in general. I don't think I agree, when macro expansion is undesired the normal way is to double-quote the arguments, which properly suppresses expansion when macro definitions follow quoting the rule of thumb. Cheers, Nick
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On Tue, Oct 20, 2020 at 4:57 PM Nick Bowler wrote: > On 2020-10-20, Sergei Trofimovich wrote: > > Initial bug is reported as autoconf failure on ghc-8.8.4: > > https://bugs.gentoo.org/750191 > > There autconf 2.69 works, 2.69c does not. ... > > $ cat configure.ac > > AC_INIT([The Glorious Glasgow Haskell Compilation System], [9.1.0], > > [glasgow-haskell-b...@haskell.org], [ghc-AC_PACKAGE_VERSION]) > > > > echo "$PACKAGE_VERSION" > > > > AC_OUTPUT If I understand correctly, the intention is to have $PACKAGE_VERSION set to "9.1.0" and $PACKAGE_TARNAME set to "ghc-9.1.0"? > Note: the change you report is introduced by Zack's fix for related > AC_INIT quoting regressions. This patch is not included in 2.69c (or > even on git master), but does seem to be applied by the Gentoo package. Yeah, this is a "can't have it both ways" kind of thing. We can reliably round-trip "unusual" characters (like the ones that appear all the time in URLs and email addresses) through AC_INIT's arguments, or we can expand macros in those arguments even when they're quoted on input; I don't think there's any way to do both. This only works by accident in 2.69, incidentally. AC_PACKAGE_VERSION is defined *after* AC_PACKAGE_TARNAME (see _AC_INIT_PACKAGE, lines 235-261 of $prefix/share/autoconf/general.m4) so both old and new autoconf set AC_PACKAGE_TARNAME to the literal string "ghc-AC_PACKAGE_VERSION". The value undergoes an extra round of expansion when it's used to set the shell variable PACKAGE_TARNAME (lines 416-428 of the same file). This extra round of expansion is undesirable in general. You can fix this in configure.ac without repeating the version number by defining an extra M4 macro and using it in both arguments: m4_define([ghc_VERSION], [9.1.0]) AC_INIT([The Glorious Glasgow Haskell Compilation System], m4_defn([ghc_VERSION]), [glasgow-haskell-b...@haskell.org], [ghc-]m4_defn([ghc_VERSION])) I'm being extra careful with quotation here; `ghc_VERSION` and `m4_defn([ghc_VERSION])` both expand to the definition of ghc_VERSION, but the latter quotes its output. This would matter if the value of ghc_VERSION could contain more M4 macros, which it *currently* doesn't... (If you don't mind, I think I might add this to the manual as an example of computing the arguments to AC_INIT, with all the GHC-specific terms removed, of course.) We *could* add a special case in AC_INIT where, if any of the third, fourth, or fifth arguments contain the literal strings `AC_PACKAGE_NAME` or `AC_PACKAGE_VERSION`, those are replaced with the values of the first and second argument, respectively. This would keep the GHC code working as-is. I'm not sure whether that's a good idea; cc:ing Paul and Eric for their thoughts. zw
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
On Tue, 20 Oct 2020 16:57:16 -0400 Nick Bowler wrote: > Hi, > > On 2020-10-20, Sergei Trofimovich wrote: > > Initial bug is reported as autoconf failure on ghc-8.8.4: > > https://bugs.gentoo.org/750191 > > There autconf 2.69 works, 2.69c does not. > > Note: the change you report is introduced by Zack's fix for related > AC_INIT quoting regressions. This patch is not included in 2.69c (or > even on git master), but does seem to be applied by the Gentoo package. > > The 2.69c release version seems to handle the example fine. Oh, I did not realize we carry an out of tree patch with such a behaviour change. Redirected the bug to Gentoo autoconf package maintainers. Thank you! -- Sergei
Re: AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
Hi, On 2020-10-20, Sergei Trofimovich wrote: > Initial bug is reported as autoconf failure on ghc-8.8.4: > https://bugs.gentoo.org/750191 > There autconf 2.69 works, 2.69c does not. Note: the change you report is introduced by Zack's fix for related AC_INIT quoting regressions. This patch is not included in 2.69c (or even on git master), but does seem to be applied by the Gentoo package. The 2.69c release version seems to handle the example fine. > Here is the minimal example: > > OK: > > $ cat configure.ac > AC_INIT([The Glorious Glasgow Haskell Compilation System], [9.1.0], > [glasgow-haskell-b...@haskell.org], [ghc-AC_PACKAGE_VERSION]) > > echo "$PACKAGE_VERSION" > > AC_OUTPUT > $ autoconf-2.69 > $ ./configure > 9.1.0 > configure: creating ./config.status > > BAD: > > $ autoconf-2.70_beta2 > configure.ac:1: error: possibly undefined macro: AC_PACKAGE_VERSION > If this token and others are legitimate, please use m4_pattern_allow. > See the Autoconf documentation. Yes I think now Zack's underquotation fixes have added the opposite problem. There is now too much quotation so the tarname (and other arguments) are not fully expanded when used. At least these changes should probably be simply dropped from the patch or at least they perhaps need more consideration... @@ -436,18 +427,12 @@ AC_SUBST([SHELL])dnl AC_SUBST([PATH_SEPARATOR])dnl # Identity of this package. -AC_SUBST([PACKAGE_NAME], -[m4_ifdef([AC_PACKAGE_NAME], ['AC_PACKAGE_NAME'])])dnl -AC_SUBST([PACKAGE_TARNAME], -[m4_ifdef([AC_PACKAGE_TARNAME], ['AC_PACKAGE_TARNAME'])])dnl -AC_SUBST([PACKAGE_VERSION], -[m4_ifdef([AC_PACKAGE_VERSION], ['AC_PACKAGE_VERSION'])])dnl -AC_SUBST([PACKAGE_STRING], -[m4_ifdef([AC_PACKAGE_STRING],['AC_PACKAGE_STRING'])])dnl -AC_SUBST([PACKAGE_BUGREPORT], -[m4_ifdef([AC_PACKAGE_BUGREPORT], ['AC_PACKAGE_BUGREPORT'])])dnl -AC_SUBST([PACKAGE_URL], -[m4_ifdef([AC_PACKAGE_URL], ['AC_PACKAGE_URL'])])dnl +AC_SUBST([PACKAGE_NAME], ['m4_defn([AC_PACKAGE_NAME])'])dnl +AC_SUBST([PACKAGE_TARNAME], ['m4_defn([AC_PACKAGE_TARNAME])'])dnl +AC_SUBST([PACKAGE_VERSION], ['m4_defn([AC_PACKAGE_VERSION])'])dnl +AC_SUBST([PACKAGE_STRING],['m4_defn([AC_PACKAGE_STRING])'])dnl +AC_SUBST([PACKAGE_BUGREPORT], ['m4_defn([AC_PACKAGE_BUGREPORT])'])dnl +AC_SUBST([PACKAGE_URL], ['m4_defn([AC_PACKAGE_URL])'])dnl m4_divert_pop([DEFAULTS])dnl m4_wrap_lifo([m4_divert_text([DEFAULTS], @@ -1099,9 +1084,8 @@ Fine tuning of the installation directories: --infodir=DIR info documentation [DATAROOTDIR/info] --localedir=DIR locale-dependent data [DATAROOTDIR/locale] --mandir=DIRman documentation [DATAROOTDIR/man] -]AS_HELP_STRING([--docdir=DIR], - [documentation root ]@<:@DATAROOTDIR/doc/m4_ifset([AC_PACKAGE_TARNAME], -[AC_PACKAGE_TARNAME], [PACKAGE])@:>@)[ + --docdir=DIRdocumentation root @<:@DATAROOTDIR/doc/]dnl +m4_default_quoted(m4_defn([AC_PACKAGE_TARNAME]), [PACKAGE])[@:>@ --htmldir=DIR html documentation [DOCDIR] --dvidir=DIRdvi documentation [DOCDIR] --pdfdir=DIRpdf documentation [DOCDIR] If you drop those two hunks from Zack's patch the example should work again. Cheers, Nick
AC_PACKAGE_VERSION visibility slightly changed in autoconf-2.69c. Bug or feature?
Initial bug is reported as autoconf failure on ghc-8.8.4: https://bugs.gentoo.org/750191 There autconf 2.69 works, 2.69c does not. Here is the minimal example: OK: $ cat configure.ac AC_INIT([The Glorious Glasgow Haskell Compilation System], [9.1.0], [glasgow-haskell-b...@haskell.org], [ghc-AC_PACKAGE_VERSION]) echo "$PACKAGE_VERSION" AC_OUTPUT $ autoconf-2.69 $ ./configure 9.1.0 configure: creating ./config.status BAD: $ autoconf-2.70_beta2 configure.ac:1: error: possibly undefined macro: AC_PACKAGE_VERSION If this token and others are legitimate, please use m4_pattern_allow. See the Autoconf documentation. https://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Initializing-configure.html says AC_INIT should define the AC_PACKAGE_VERSION, but evaluation probably happens later. Should autoconf restore the old behaviour or should ghc adapt? If we are to adapt what would be the best way not to repeat the version? Thank you! -- Sergei
