Re: [PATCH] gdb/configure.ac: Fix auto-load options to work with Windows path separator

[Andrew Burgess <aburgess@redhat.com>]

Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:

> Both --with-auto-load-dir and --with-auto-load-safe-path accept a list of
> directories, which are separated by ':' on Unix and ';' on Windows.
> However, as mentioned in PR 18898 this doesn't work on the latter OS.
>
> This is because the AC_DEFINE_DIR macro calls eval twice on the value
> provided via these configure options, causing the ';' to be interpreted by
> the shell.  E.g.,
>
>   $ ~/src/binutils-gdb/configure \
>       --disable-{binutils,ld,gold,gas,sim,gprof,gprofng} \
>       --with-auto-load-dir='foo;bar' && make
>       ⋮
>   checking for default auto-load directory... /home/bauermann/src/binutils-gdb/gdb/configure: line 18004: bar: command not found
>   foo;bar
>   checking for default auto-load safe-path... /home/bauermann/src/binutils-gdb/gdb/configure: line 18031: bar: command not found
>   foo;bar
>
> Line 18004 is:
>
>   ac_define_dir=`eval echo $escape_dir`
>
> Line 18031 is identical.
>
> With some escaping, it's possible to avoid the problem:
>
>   $ ~/src/binutils-gdb/configure \
>       --disable-{binutils,ld,gold,gas,sim,gprof,gprofng} \
>       --with-auto-load-dir='foo\\\;bar\\\;baz' && make
>       ⋮
>   checking for default auto-load directory... foo\\\;bar\\\;baz
>   checking for default auto-load safe-path... foo\\\;bar\\\;baz
>       ⋮
>   $ grep AUTO_LOAD gdb/config.h
>   #define AUTO_LOAD_DIR "foo;bar;baz"
>   #define AUTO_LOAD_SAFE_PATH "foo;bar;baz"
>
> We provide the definition of AC_DEFINE_DIR in gdb/acinclude.m4 so the
> simplest approach to fix this problem would be to remove the evals from the
> macro.
>
> I don't know why they are there however, so instead I decided to make
> gdb/configure.ac automatically add those escaping characters to the values
> of the options if the host OS is Windows.  I don't know if Cygwin also
> needs this change, so for now I'm only adding it for MinGW.

I tracked down a later version of AC_DEFINE_DIR (renamed to
AC_DEFINE_DIR) in the autoconf-archive.  The macro has actually been
removed as it apparently violates some GNU guideline on how paths like
this should be handled, but I didn't really understand...

...but the interesting bit is the definition of the macro just prior to
its removal:

  AU_ALIAS([AC_DEFINE_DIR], [AX_DEFINE_DIR])
  AC_DEFUN([AX_DEFINE_DIR], [
    prefix_NONE=
    exec_prefix_NONE=
    test "x$prefix" = xNONE && prefix_NONE=yes && prefix=$ac_default_prefix
    test "x$exec_prefix" = xNONE && exec_prefix_NONE=yes && exec_prefix=$prefix
  dnl In Autoconf 2.60, ${datadir} refers to ${datarootdir}, which in turn
  dnl refers to ${prefix}.  Thus we have to use `eval' twice.
    eval ax_define_dir="\"[$]$2\""
    eval ax_define_dir="\"$ax_define_dir\""
    AC_SUBST($1, "$ax_define_dir")
    AC_DEFINE_UNQUOTED($1, "$ax_define_dir", [$3])
    test "$prefix_NONE" && prefix=NONE
    test "$exec_prefix_NONE" && exec_prefix=NONE
  ])

Not only does this offer a little insight into why the double eval
exists, but I notice the use of eval has changed slightly, and there are
now quotes being used, e.g. "\"[$]$2\""

I wonder if using some of the updates would offer an alternative
solution?

Thanks,
Andrew


>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=18898
> ---
>
> Hello,
>
> This addresses the problem that Eli has been having with these options
> on MinGW. I don't have access to a Windows system, so I tested the patch
> on Linux by changing the case to match "linux*" rather than "mingw*" and
> using the build and grep commands mentioned in the commit message.
>
> It would be nice if someone could test on an actual Windows system, and
> also let me know if other host_os strings should also be matched (e.g.,
> "cygwin*" or "windows*")
>
>  gdb/configure    | 20 ++++++++++++++++++--
>  gdb/configure.ac | 20 ++++++++++++++++++--
>  2 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/configure b/gdb/configure
> index 5361bf42952a..96f0e3da9637 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -17997,7 +17997,15 @@ else
>    with_auto_load_dir='$debugdir:$datadir/auto-load'
>  fi
>  
> -escape_dir=`echo $with_auto_load_dir | sed -e 's/[$]datadir\>/\\\\\\\\\\\\&/g' -e 's/[$]debugdir\>/\\\\\\\\\\\\&/g'`
> +# If we're on Windows, we need to protect the PATH separator ';' from the couple
> +# of eval calls done by AC_DEFINE_DIR.
> +extra_sed_args=()
> +case "$host_os" in
> +  mingw*)
> +  extra_sed_args=(-e 's/;/\\\\\\;/g')
> +  ;;
> +esac
> +escape_dir=`echo $with_auto_load_dir | sed -e 's/[$]datadir\>/\\\\\\\\\\\\&/g' -e 's/[$]debugdir\>/\\\\\\\\\\\\&/g' "${extra_sed_args[@]}"`
>  
>    test "x$prefix" = xNONE && prefix="$ac_default_prefix"
>    test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
> @@ -18024,7 +18032,15 @@ else
>    with_auto_load_safe_path="$with_auto_load_dir"
>  fi
>  
> -escape_dir=`echo $with_auto_load_safe_path | sed -e 's/[$]datadir\>/\\\\\\\\\\\\&/g' -e 's/[$]debugdir\>/\\\\\\\\\\\\&/g'`
> +# If we're on Windows, we need to protect the PATH separator ';' from the couple
> +# of eval calls done by AC_DEFINE_DIR.
> +extra_sed_args=()
> +case "$host_os" in
> +  mingw*)
> +  extra_sed_args=(-e 's/;/\\\\\\;/g')
> +  ;;
> +esac
> +escape_dir=`echo $with_auto_load_safe_path | sed -e 's/[$]datadir\>/\\\\\\\\\\\\&/g' -e 's/[$]debugdir\>/\\\\\\\\\\\\&/g' "${extra_sed_args[@]}"`
>  
>    test "x$prefix" = xNONE && prefix="$ac_default_prefix"
>    test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 3912b77b27f2..cc8d2bfc5e8b 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -153,7 +153,15 @@ AC_ARG_WITH(auto-load-dir,
>  AS_HELP_STRING([--with-auto-load-dir=PATH],
>    [directories from which to load auto-loaded scripts @<:@$debugdir:$datadir/auto-load@:>@]),,
>    [with_auto_load_dir='$debugdir:$datadir/auto-load'])
> -escape_dir=`echo $with_auto_load_dir | sed -e 's/[[$]]datadir\>/\\\\\\\\\\\\&/g' -e 's/[[$]]debugdir\>/\\\\\\\\\\\\&/g'`
> +# If we're on Windows, we need to protect the PATH separator ';' from the couple
> +# of eval calls done by AC_DEFINE_DIR.
> +extra_sed_args=()
> +case "$host_os" in
> +  mingw*)
> +  extra_sed_args=(-e 's/;/\\\\\\;/g')
> +  ;;
> +esac
> +escape_dir=`echo $with_auto_load_dir | sed -e 's/[[$]]datadir\>/\\\\\\\\\\\\&/g' -e 's/[[$]]debugdir\>/\\\\\\\\\\\\&/g' "${extra_sed_args[[@]]}"`
>  AC_DEFINE_DIR(AUTO_LOAD_DIR, escape_dir,
>  	      [Directories from which to load auto-loaded scripts.])
>  AC_MSG_RESULT([$with_auto_load_dir])
> @@ -168,7 +176,15 @@ AS_HELP_STRING([--without-auto-load-safe-path],
>       with_auto_load_safe_path="/"
>       fi],
>  [with_auto_load_safe_path="$with_auto_load_dir"])
> -escape_dir=`echo $with_auto_load_safe_path | sed -e 's/[[$]]datadir\>/\\\\\\\\\\\\&/g' -e 's/[[$]]debugdir\>/\\\\\\\\\\\\&/g'`
> +# If we're on Windows, we need to protect the PATH separator ';' from the couple
> +# of eval calls done by AC_DEFINE_DIR.
> +extra_sed_args=()
> +case "$host_os" in
> +  mingw*)
> +  extra_sed_args=(-e 's/;/\\\\\\;/g')
> +  ;;
> +esac
> +escape_dir=`echo $with_auto_load_safe_path | sed -e 's/[[$]]datadir\>/\\\\\\\\\\\\&/g' -e 's/[[$]]debugdir\>/\\\\\\\\\\\\&/g' "${extra_sed_args[[@]]}"`
>  AC_DEFINE_DIR(AUTO_LOAD_SAFE_PATH, escape_dir,
>  	      [Directories safe to hold auto-loaded files.])
>  AC_MSG_RESULT([$with_auto_load_safe_path])