Quantcast

config.log claims invocation is FLAGS=foo bar when really FLAGS='foo bar'

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

config.log claims invocation is FLAGS=foo bar when really FLAGS='foo bar'

Kevin Brubeck Unhammer
Tested with GNU Autoconf 2.69.

To reproduce, open any autoconf-using project and run:

$ ./configure FLAGS='foo bar' >/dev/null && grep -m1 /configure config.log
  $ ./configure FLAGS=foo bar

Expected output:
  $ ./configure "FLAGS='foo bar'"


I know the shell interprets the quotes etc., but it's possible to work
around that; e.g.

C=''
for i in "$@"; do
    i=$(echo "$i" | sed 's/\\/\\\\/g; s/\"/\\\"/g')
    C="$C \"$i\""
done
echo "$C"

(based on bash version at http://stackoverflow.com/a/8723305/69663 )

--
Kevin Brubeck Unhammer

GPG: 50D487960B863F054B6A12F9742606DE766AC60C

signature.asc (834 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: config.log claims invocation is FLAGS=foo bar when really FLAGS='foo bar'

Eric Blake-3
On 11/04/2016 05:51 AM, Kevin Brubeck Unhammer wrote:
> Tested with GNU Autoconf 2.69.
>
> To reproduce, open any autoconf-using project and run:
>
> $ ./configure FLAGS='foo bar' >/dev/null && grep -m1 /configure config.log
>   $ ./configure FLAGS=foo bar
>
> Expected output:
>   $ ./configure "FLAGS='foo bar'"

Thanks for the report.  The same problem also exists when config.status
reports its invocation line.

>
>
> I know the shell interprets the quotes etc., but it's possible to work
> around that; e.g.
>
> C=''
> for i in "$@"; do
>     i=$(echo "$i" | sed 's/\\/\\\\/g; s/\"/\\\"/g')
>     C="$C \"$i\""
> done
> echo "$C"
That works (although we'd want to tweak it to not pollute outside the
autoconf namespace), but adds a number of forks to what is otherwise a
fork-free output of "$@".  It's probably possible to optimize for the
common case of arguments that don't need shell quoting to reduce some of
the cost.  Since both lib/autoconf/general.m4 and lib/autoconf/status.m4
output an invocation line, it's probably worth factoring out a common
helper macro, probably to lib/m4sugar/m4sh.m4; would you like to try
your hand at a patch? If not, I can probably do it.

--
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


signature.asc (617 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: config.log claims invocation is FLAGS=foo bar when really FLAGS='foo bar'

Kevin Brubeck Unhammer
Eric Blake <[hidden email]> čálii:

> On 11/04/2016 05:51 AM, Kevin Brubeck Unhammer wrote:

[...]

>> C=''
>> for i in "$@"; do
>>     i=$(echo "$i" | sed 's/\\/\\\\/g; s/\"/\\\"/g')
>>     C="$C \"$i\""
>> done
>> echo "$C"
>
> That works (although we'd want to tweak it to not pollute outside the
> autoconf namespace), but adds a number of forks to what is otherwise a
> fork-free output of "$@".  It's probably possible to optimize for the
> common case of arguments that don't need shell quoting to reduce some of
> the cost.  Since both lib/autoconf/general.m4 and lib/autoconf/status.m4
> output an invocation line, it's probably worth factoring out a common
> helper macro, probably to lib/m4sugar/m4sh.m4; would you like to try
> your hand at a patch? If not, I can probably do it.
I've tested the following to work under both dash and bash; and it only
runs sed if the argument contains a ‘'’.


#!/bin/sh
C=''
for i in "$@"; do
    case "$i" in
        *\'*)
           i=`printf "%s" "$i" | sed "s/'/'\"'\"'/g"`
           ;;
        *) :
           ;;
    esac
    C="$C '$i'"
done
printf " $0%s\n" "$C"


I tested by making a script containing

#!/bin/sh
C=''
for i in "$@"; do
    case "$i" in
        *\'*)
           echo 'fork!' >&2
           i=`printf "%s" "$i" | sed "s/'/'\"'\"'/g"`
           ;;
        *) :
           ;;
    esac
    C="$C '$i'"
done
printf "%s\n" "$@" >/tmp/gold
echo "Copy-paste this line, should give no output:"
printf " printf \"%%s\\\n\"  %s >/tmp/test; diff /tmp/gold /tmp/test\n" "$C"

and

running e.g.

$ dash /tmp/esc.sh 'foo bar' 'x\"y' 'x\\"y'  'x\\\"y' 'x\\ \"y!' '!!' "foo'bar"

and copy-pasting the output. Seems to hold …


I don't know much about m4 though (and don't have much free time – but
then who does), so maybe it's best if you turn that into a real patch,
otherwise it's going to take quite a while :)



best regards,
Kevin Brubeck Unhammer

signature.asc (834 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 0/2] Fix logged command line

Eric Blake-3
In reply to this post by Kevin Brubeck Unhammer
7 weeks after Kevin's report, and I finally had time to try my
hand at the proposed fix for what gets logged when you do:

./configure FLAGS='foo bar'

It would be nice to get a review, but I will probably include it
in autoconf 2.70 (which I still hope to release next week) whether
or not it has been reviewed.

Note that './config-status --version' already outputs a
correctly-quoted list of arguments given to ./configure, but not
quite in the same format (it splits $0 and $@ with other text),
so it's still not as easy to paste that into a command line as it
is to paste the text grep'ed out of config.log.

Eric Blake (2):
  m4sh: Add _AS_INVOCATION
  autoconf: Properly quote logged command line

 lib/autoconf/general.m4 |  3 ++-
 lib/autoconf/status.m4  |  3 ++-
 lib/m4sugar/m4sh.m4     | 14 ++++++++++++++
 tests/m4sh.at           | 23 +++++++++++++++++++++++
 4 files changed, 41 insertions(+), 2 deletions(-)

--
2.9.3


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/2] autoconf: Properly quote logged command line

Eric Blake-3
We were previously logging the wrong command line if the user
invoked any argument with spaces, making it difficult to
re-invoke configure with the same complex arguments. To reproduce,
open any autoconf-using project and run:

$ ./configure FLAGS='foo bar' >/dev/null && grep -m1 /configure config.log
  $ ./configure FLAGS=foo bar

Now we get the expected output:
  $ ./configure 'FLAGS=foo bar'

(Okay, it would be prettier as FLAGS='foo bar' in the output, by
delaying the opening ' until after the first non-alphanumeric or =,
but that is more expensive to produce, and makes no difference to
shell parsing of the pasted line.)

* lib/autoconf/general.m4 (_AC_INIT_CONFIG_LOG): Use _AS_INVOCATION.
* lib/autoconf/status.m4 (_AC_OUTPUT_CONFIG_STATUS): Likewise.
Reported by Kevin Brubeck Unhammer <[hidden email]>

Signed-off-by: Eric Blake <[hidden email]>
---

I wonder if it would also be worth updating these two logging points
to include the output of 'date' as a (rough) timestamp of when the
recorded invocation started.

 lib/autoconf/general.m4 | 3 ++-
 lib/autoconf/status.m4  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index ef6285d..bac9fcd 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -1183,6 +1183,7 @@ fi])dnl
 m4_define([_AC_INIT_CONFIG_LOG],
 [m4_divert_text([INIT_PREPARE],
 [m4_define([AS_MESSAGE_LOG_FD], 5)dnl
+_AS_INVOCATION
 cat >config.log <<_ACEOF
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
@@ -1191,7 +1192,7 @@ It was created by m4_ifset([AC_PACKAGE_NAME], [AC_PACKAGE_NAME ])dnl
 $as_me[]m4_ifset([AC_PACKAGE_VERSION], [ AC_PACKAGE_VERSION]), which was
 generated by m4_PACKAGE_STRING.  Invocation command line was

-  $ $[0] $[@]
+  $as_invocation

 _ACEOF
 exec AS_MESSAGE_LOG_FD>>config.log
diff --git a/lib/autoconf/status.m4 b/lib/autoconf/status.m4
index 308d3d8..f692b6d 100644
--- a/lib/autoconf/status.m4
+++ b/lib/autoconf/status.m4
@@ -1334,6 +1334,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1
 [#] Save the log message, to keep $[0] and so on meaningful, and to
 # report actual input values of CONFIG_FILES etc. instead of their
 # values after options handling.
+_AS_INVOCATION
 ac_log="
 This file was extended by m4_ifset([AC_PACKAGE_NAME], [AC_PACKAGE_NAME ])dnl
 $as_me[]m4_ifset([AC_PACKAGE_VERSION], [ AC_PACKAGE_VERSION]), which was
@@ -1343,7 +1344,7 @@ generated by m4_PACKAGE_STRING.  Invocation command line was
   CONFIG_HEADERS  = $CONFIG_HEADERS
   CONFIG_LINKS    = $CONFIG_LINKS
   CONFIG_COMMANDS = $CONFIG_COMMANDS
-  $ $[0] $[@]
+  $as_invocation

 on `(hostname || uname -n) 2>/dev/null | sed 1q`
 "
--
2.9.3


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/2] m4sh: Add _AS_INVOCATION

Eric Blake-3
In reply to this post by Eric Blake-3
When outputting a script's command line to a log file, it helps
if we quote arguments suitable for re-pasting into a shell command
line.  Since we have more than one site (configure and config.status)
that weren't doing this, it helps if we first add an m4sh primitive
for easily creating this format, while minimizing forks and quoting
on arguments that don't need special handling.

For now, I've kept the macro undocumented; if there is demand, we
can promote it to the stable name AS_INVOCATION and add it to the
manual.

* lib/m4sugar/m4sh.m4 (_AS_INVOCATION): New macro.
* tests/m4sh.at (_AS@&t@_INVOCATION): Test it.
Inspired by a report by Kevin Brubeck Unhammer <[hidden email]>.
Signed-off-by: Eric Blake <[hidden email]>
---
 lib/m4sugar/m4sh.m4 | 14 ++++++++++++++
 tests/m4sh.at       | 23 +++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/lib/m4sugar/m4sh.m4 b/lib/m4sugar/m4sh.m4
index 3b37683..e0470ed 100644
--- a/lib/m4sugar/m4sh.m4
+++ b/lib/m4sugar/m4sh.m4
@@ -687,6 +687,20 @@ m4_defun([AS_UNSET],
 ## ------------------------------------------ ##


+# _AS_INVOCATION
+# --------------
+# Set $as_invocation to a quoted representation of '$ $0 $@' that can be
+# pasted into a shell command line to achieve the same invocation.
+m4_define([_AS_INVOCATION],
+[as_invocation='$'
+for as_arg in "$[0]" "$[@]"; do
+  case $as_arg in #((
+    *\'*) as_arg=\'`printf %s "$as_arg" | sed "s/'/'\\\\\\\\''/g"`\' ;;
+    '' | *[[!a-zA-Z0-9_.,:/+-]]*) as_arg=\'$as_arg\' ;;
+  esac
+  AS_VAR_APPEND([as_invocation], [" $as_arg"])
+done])
+
 # AS_MESSAGE_FD
 # -------------
 # Must expand to the fd where messages will be sent.  Defaults to 1,
diff --git a/tests/m4sh.at b/tests/m4sh.at
index c18cdcf..ff4f20f 100644
--- a/tests/m4sh.at
+++ b/tests/m4sh.at
@@ -1959,3 +1959,26 @@ AT_CHECK([$CONFIG_SHELL ./script], [], [foobar
 ])

 AT_CLEANUP
+
+
+## ---------------- ##
+## _AS_INVOCATION.  ##
+## ---------------- ##
+
+AT_SETUP([_AS@&t@_INVOCATION])
+
+AT_DATA_M4SH([script.as], [[dnl
+AS_INIT
+_AS_PREPARE
+_AS_INVOCATION
+echo "$as_invocation"
+]])
+
+AT_CHECK_M4SH
+AT_CHECK([$CONFIG_SHELL ./script], [], [$ ./script
+])
+AT_CHECK([$CONFIG_SHELL ./script a.1 "b  c" "d'e" 'f"g"' "" "#" "*" h=i],
+         [], [$ ./script a.1 'b  c' 'd'\''e' 'f"g"' '' '#' '*' 'h=i'
+])
+
+AT_CLEANUP
--
2.9.3


Loading...