Whitespace in `${srcdir}' during `configure'

Whitespace in `${srcdir}' during `configure'

[Alexander Shukaev]

[-- Attachment #1.1: Type: text/plain, Size: 721 bytes --]

Currently in some places of `configure.ac' `${srcdir}' is used without
surrounding double quotes what results in `configure' failure. The attached
patch makes usage of `${srcdir}' more consistent and fixes it. Yes, I know
that when one enter `configure' phase with `${srcdir}' which contains
whitespace, one would see

checking whether build environment is sane... configure: error: unsafe
> srcdir value: ...


anyway. But that's another story. Maybe on some bright day, it would be
finally safe according to Autotools. Call this pedantic, but I think it is
better to receive this official prohibition error, rather than confusing

configure: line 3557: ...: No such file or directory


for `${srcdir}' with whitespace.

[-- Attachment #1.2: Type: text/html, Size: 1233 bytes --]

[-- Attachment #2: configure.ac.diff --]
[-- Type: text/plain, Size: 3626 bytes --]

--- configure.ac.orig	2014-11-10 03:06:29.602406600 +0100
+++ configure.ac	2014-11-10 03:05:41.103915800 +0100
@@ -27,9 +27,9 @@
 dnl We get MINGW64 with MSYS2
 if test "x$MSYSTEM" = "xMINGW32" -o "x$MSYSTEM" = "xMINGW64"
 then
-  . $srcdir/nt/mingw-cfg.site
+  . "${srcdir}"/nt/mingw-cfg.site
 
-  case $srcdir in
+  case "${srcdir}" in
     /* | ?:*)
       # srcdir is an absolute path.  In this case, force the format
       # "/c/foo/bar", to simplify later conversions to native Windows
@@ -610,7 +610,7 @@
       *-mingw32 )
 		opsys=mingw32
 		# MinGW overrides and adds some system headers in nt/inc.
-		GCC_TEST_OPTIONS="-I $srcdir/nt/inc"
+		GCC_TEST_OPTIONS="-I ${srcdir}/nt/inc"
 		;;
       *-sysv4.2uw* )	  	opsys=unixware ;;
       *-sysv5uw* )	  	opsys=unixware ;;
@@ -625,7 +625,7 @@
       *-mingw32 )
 		opsys=mingw32
 		# MinGW overrides and adds some system headers in nt/inc.
-		GCC_TEST_OPTIONS="-I $srcdir/nt/inc"
+		GCC_TEST_OPTIONS="-I ${srcdir}/nt/inc"
 		;;
       ## Otherwise, we'll fall through to the generic opsys code at the bottom.
     esac
@@ -1042,7 +1042,7 @@
 ## In a repository checkout on the other hand, the manuals are not included.
 ## So makeinfo is a requirement to build from the repository, and configure
 ## should test for it as it does for any other build requirement.
-## We use the presence of $srcdir/info/emacs to distinguish a release,
+## We use the presence of ${srcdir}/info/emacs to distinguish a release,
 ## with pre-built manuals, from a repository checkout.
 HAVE_MAKEINFO=yes
 
@@ -1050,7 +1050,7 @@
   MAKEINFO=makeinfo
   if test "x${with_makeinfo}" = "xno"; then
     HAVE_MAKEINFO=no
-  elif test ! -e "$srcdir/info/emacs" && test ! -e "$srcdir/info/emacs.info"; then
+  elif test ! -e "${srcdir}/info/emacs" && test ! -e "${srcdir}/info/emacs.info"; then
     AC_MSG_ERROR( [You do not seem to have makeinfo >= 4.7, and your
 source tree does not seem to have pre-built manuals in the `info' directory.
 Either install a suitable version of makeinfo, or re-run configure
@@ -1566,13 +1566,13 @@
       deps_frag=autodeps.mk
    fi
 fi
-deps_frag=$srcdir/src/$deps_frag
+deps_frag="${srcdir}/src/$deps_frag"
 AC_SUBST(MKDEPDIR)
 AC_SUBST(DEPFLAGS)
 AC_SUBST_FILE(deps_frag)
 
 
-lisp_frag=$srcdir/src/lisp.mk
+lisp_frag="${srcdir}/src/lisp.mk"
 AC_SUBST_FILE(lisp_frag)
 
 
@@ -5120,7 +5120,7 @@
 dnl test/ is not present in release tarfiles.
 opt_makefile=test/automated/Makefile
 
-if test -f "$srcdir/$opt_makefile.in"; then
+if test -f "${srcdir}/$opt_makefile.in"; then
   SUBDIR_MAKEFILES="$SUBDIR_MAKEFILES $opt_makefile"
   dnl Again, it's best not to use a variable.  Though you can add
   dnl ", [], [opt_makefile='$opt_makefile']" and it should work.
@@ -5129,7 +5129,7 @@
 
 
 dnl The admin/ directory used to be excluded from tarfiles.
-if test -d $srcdir/admin; then
+if test -d "${srcdir}/admin"; then
   SUBDIR_MAKEFILES="$SUBDIR_MAKEFILES admin/unidata/Makefile admin/grammars/Makefile"
   AC_CONFIG_FILES([admin/unidata/Makefile])
   AC_CONFIG_FILES([admin/grammars/Makefile])
@@ -5157,10 +5157,10 @@
 ], [GCC="$GCC" CPPFLAGS="$CPPFLAGS" opsys="$opsys"])
 
 dnl NB we have to cheat and use the ac_... version because abs_top_srcdir
-dnl is not yet set, sigh.  Or we could use ../$srcdir/src/.gdbinit,
+dnl is not yet set, sigh.  Or we could use ../${srcdir}/src/.gdbinit,
 dnl or a symlink?
 AC_CONFIG_COMMANDS([src/.gdbinit], [
-if test ! -f src/.gdbinit && test -f "$srcdir/src/.gdbinit"; then
+if test ! -f src/.gdbinit && test -f "${srcdir}/src/.gdbinit"; then
   echo "source $ac_abs_top_srcdir/src/.gdbinit" > src/.gdbinit
 fi
 ])

Re: Whitespace in `${srcdir}' during `configure'

[Glenn Morris]

As you say, a srcdir with spaces is impossible, and given the way
Makefiles work, it seems to me like it always will be. So I don't see
much point in quoting it as if it could contain spaces.

Re: Whitespace in `${srcdir}' during `configure'

[Paul Eggert]

It'd be nice if Emacs 'configure' allowed arbitrary source-directory names. 
Unfortunately, almost none of the changes in the patch that you sent fix any 
bugs, and almost all the bugs in this area would remain unfixed by the patch. 
If you'd like to help fix the many problems in this area, I suggest building and 
configuring Emacs in a directory whose name contains spaces, tabs, and 
characters from the set !"#$%&'()*+,-.:;<=>?@[\]^`{|}~ and make sure that 
everything works; also, make sure that every change in the patch actually fixes 
a bug.  It could be that we can support some characters (e.g, spaces) but not 
others (e.g., apostrophes, colons), but in any event the more characters that we 
can support the better, and it'd be good to document the ones we can't.

Re: Whitespace in `${srcdir}' during `configure'

[Paul Eggert]

Glenn Morris wrote:
> As you say, a srcdir with spaces is impossible, and given the way
> Makefiles work, it seems to me like it always will be.

As I understand it the next version of GNU Make will address this issue, which 
means that once that we can assume that version everywhere (a few years from 
now), we might be able to pull it off.  Admittedly this is not much comfort to a 
builder who wants spaces in directory names today....

Re: Whitespace in `${srcdir}' during `configure'

[Glenn Morris]

Paul Eggert wrote:

> It'd be nice if Emacs 'configure' allowed arbitrary source-directory
> names.

s/Emacs/any project using autoconf/

(AFAIK)

Re: Whitespace in `${srcdir}' during `configure'

[Glenn Morris]

Paul Eggert wrote:

> As I understand it the next version of GNU Make will address this
> issue, which means that once that we can assume that version
> everywhere (a few years from now), we might be able to pull it off.

Maybe this can be revisited after such a Make gets released.

> Admittedly this is not much comfort to a builder who wants spaces in
> directory names today....

AFAIK you can build in a directory with spaces today (srcdir=.),
you just can't pass a srcdir containing spaces (out-of-tree build).

But really, the only case I can think of is a user whose home directory
contains spaces and is unable to use /tmp. In which case, I can only
assume that their sysadmin hates them and wants them to be miserable.
Compiling software is probably the least of their problems. :)

Re: Whitespace in `${srcdir}' during `configure'

[Andreas Schwab]

Glenn Morris <rgm@gnu.org> writes:

> Paul Eggert wrote:
>
>> It'd be nice if Emacs 'configure' allowed arbitrary source-directory
>> names.
>
> s/Emacs/any project using autoconf/

Not necessarily, those not using abs_srcdir could go along with it.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Whitespace in `${srcdir}' during `configure'

[Eli Zaretskii]

> Date: Sun, 09 Nov 2014 23:20:30 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> As I understand it the next version of GNU Make will address this issue

Indeed.  But that version is a long way before release.

Re: Whitespace in `${srcdir}' during `configure'

[Yuri Khan]

On Mon, Nov 10, 2014 at 2:26 PM, Glenn Morris <rgm@gnu.org> wrote:

> But really, the only case I can think of is a user whose home directory
> contains spaces and is unable to use /tmp. In which case, I can only
> assume that their sysadmin hates them and wants them to be miserable.
> Compiling software is probably the least of their problems. :)

This case is *the* norm on Windows, starting with 2000 (where /home is
"C:\Documents and Settings") and up to but not including Vista (where
it was finally renamed "C:\Users"). Also, temporary directories (both
user’s and public) are within this subtree.

Re: Whitespace in `${srcdir}' during `configure'

[Eli Zaretskii]

> Date: Mon, 10 Nov 2014 21:28:07 +0700
> From: Yuri Khan <yuri.v.khan@gmail.com>
> Cc: Alexander Shukaev <haroogan@gmail.com>, Paul Eggert <eggert@cs.ucla.edu>,
> 	emacs-devel <emacs-devel@gnu.org>
> 
> On Mon, Nov 10, 2014 at 2:26 PM, Glenn Morris <rgm@gnu.org> wrote:
> 
> > But really, the only case I can think of is a user whose home directory
> > contains spaces and is unable to use /tmp. In which case, I can only
> > assume that their sysadmin hates them and wants them to be miserable.
> > Compiling software is probably the least of their problems. :)
> 
> This case is *the* norm on Windows, starting with 2000 (where /home is
> "C:\Documents and Settings") and up to but not including Vista (where
> it was finally renamed "C:\Users"). Also, temporary directories (both
> user’s and public) are within this subtree.

This all is true, but it is not really relevant, since these
directories are rarely if ever seen in Emacs build process on Windows
(because we use MSYS for that, which has different ideas about these
directories.

Re: Whitespace in `${srcdir}' during `configure'

[Alexander Shukaev]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

>
> This all is true, but it is not really relevant, since these
> directories are rarely if ever seen in Emacs build process on Windows
> (because we use MSYS for that, which has different ideas about these
> directories.
>

It is relevant. Yesterday I've tried to build it in a directory
"C:\Users\Haroogan\Projects\Bitbucket\Emacs for Windows", where name
corresponds to the name of the project on Bitbucket. Yes, this case occurs
not too often, but directories with spaces are generally allowed and
therefore should be supported. It's an element of build robustness after
all.

My patch at least prevents:

configure: line 3557: ...: No such file or directory


And leads to the official error:

checking whether build environment is sane... configure: error: unsafe
> srcdir value: ...


and this limitation might be gone someday. So it is still worth applying I
believe.

[-- Attachment #2: Type: text/html, Size: 1877 bytes --]

Re: Whitespace in `${srcdir}' during `configure'

[Eli Zaretskii]

> Date: Mon, 10 Nov 2014 17:03:12 +0100
> From: Alexander Shukaev <haroogan@gmail.com>
> Cc: Yuri Khan <yuri.v.khan@gmail.com>, Glenn Morris <rgm@gnu.org>, eggert@cs.ucla.edu, 
> 	emacs-devel <emacs-devel@gnu.org>
> 
>     This all is true, but it is not really relevant, since these
>     directories are rarely if ever seen in Emacs build process on Windows
>     (because we use MSYS for that, which has different ideas about these
>     directories.
> 
> It is relevant. Yesterday I've tried to build it in a directory
> "C:\Users\Haroogan\Projects\Bitbucket\Emacs for Windows", where name
> corresponds to the name of the project on Bitbucket.

All I said is that the claim that almost every Windows user will have
these problems because Windows puts HOME in a directory with
whitespace is not relevant to the issue at hand.

I didn't say that directories with whitespace cannot happen.  I just
said that if they happen, they do because the _user_ have chosen to
build in such a directory, not because Windows did that behind the
user's back.  IOW, there's no difference between Windows and Unix in
this regard.

> My patch at least prevents:
> 
>     configure: line 3557: ...: No such file or directory
> 
> And leads to the official error:
> 
>     checking whether build environment is sane... configure: error: unsafe
>     srcdir value: ...
> 
> and this limitation might be gone someday. So it is still worth applying I
> believe.

I don't think your patch was rejected.  I think Paul encouraged you to
submit a more radical patch, so as to solve this problem in a more
complete way.

Re: Whitespace in `${srcdir}' during `configure'

[Glenn Morris]

Alexander Shukaev wrote:

> It is relevant. Yesterday I've tried to build it in a directory
> "C:\Users\Haroogan\Projects\Bitbucket\Emacs for Windows", where name
> corresponds to the name of the project on Bitbucket. Yes, this case occurs
> not too often, but directories with spaces are generally allowed and
> therefore should be supported. It's an element of build robustness after
> all.

Just to be clear: building Emacs in a directory with spaces works fine.
(Tested with 24.4 tarfile just now on GNU/Linux.
There could be some MS Windows specific issue I don't know about.)

What doesn't work is passing an explicit srcdir containing spaces to
configure. This is something that configure itself rejects.
This would be an (odd) out-of-tree build.
IMO this is of very minor interest.

> My patch at least prevents:
>
> configure: line 3557: ...: No such file or directory
>
> And leads to the official error:
>
>    checking whether build environment is sane... configure: error:
>    unsafe srcdir value: ...

But line 3557, which in Emacs 24.4 is

  . $srcdir/nt/mingw-cfg.site

comes _after_ the check for an unsafe srcdir, so how is this sequence possible?

But yes, we could certainly quote that one srcdir if it somehow gets
accessed before the spaces check.

(I see that most of your patch is entirely stylistic, changing $var for
${var}.)

> and this limitation might be gone someday.

I think we should wait till that day arrives in autotools and make,
before worrying about it in Emacs.

Re: Whitespace in `${srcdir}' during `configure'

[Alexander Shukaev]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

>
> But line 3557, which in Emacs 24.4 is
>
>   . $srcdir/nt/mingw-cfg.site
>
> comes _after_ the check for an unsafe srcdir, so how is this sequence
> possible?
>

Well, that's what happened to me.

(I see that most of your patch is entirely stylistic, changing $var for
> ${var}.)
>

My patch adds double quotes around `${srcdir}' in at least 6 places.

I think we should wait till that day arrives in autotools and make,
> before worrying about it in Emacs.
>

Orders are orders.

[-- Attachment #2: Type: text/html, Size: 1235 bytes --]

Re: Whitespace in `${srcdir}' during `configure'

[Glenn Morris]

Oh now, I see.

I guess you were using Emacs 24.4 or emacs-24.

In that branch

  . $srcdir/nt/mingw-cfg.site

indeed comes before "checking whether build environment is sane" (via
AM_SANITY_CHECK from AM_INIT_AUTOMAKE). In trunk, it comes after.

So in that branch, we should indeed either quote that one reference to
srcdir, or move it after AM_INIT_AUTOMAKE (that change was already made
in trunk 2014-06-10).

PS Quoting is not always necessary, eg it's not needed in case
arguments and on the RHS of assignments:

https://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Shell-Substitutions.html

So AFAICS none of the other changes you suggest would be necessary (even
if srcdir could contain spaces) apart from the gdbinit one. But by that
stage, we are sure srcdir has no spaces.

Re: Whitespace in `${srcdir}' during `configure'

[Glenn Morris]

Please test this backport of Paul's 2014-06-10 change:

*** configure.ac	2014-10-22 13:10:41 +0000
--- configure.ac	2014-11-10 19:31:16 +0000
***************
*** 24,46 ****
  AC_PREREQ(2.65)
  AC_INIT(emacs, 24.4.51)
  
- dnl We get MINGW64 with MSYS2
- if test "x$MSYSTEM" = "xMINGW32" -o "x$MSYSTEM" = "xMINGW64"
- then
-   . $srcdir/nt/mingw-cfg.site
- 
-   case $srcdir in
-     /* | ?:*)
-       # srcdir is an absolute path.  In this case, force the format
-       # "/c/foo/bar", to simplify later conversions to native Windows
-       # format ("c:/foo/bar")
-       srcdir=`cd "${srcdir}" && pwd -W`
-       # 'eval' pacifies strict POSIX non-MinGW shells (Bug#18612).
-       eval 'srcdir="/${srcdir:0:1}${srcdir:2}"'
-       ;;
-   esac
- fi
- 
  dnl Set emacs_config_options to the options of 'configure', quoted for the shell,
  dnl and then quoted again for a C string.  Separate options with spaces.
  dnl Add some environment variables, if they were passed via the environment
--- 24,29 ----
***************
*** 102,107 ****
--- 85,112 ----
  dnl Fairly arbitrary, older versions might work too.
  AM_INIT_AUTOMAKE(1.11)
  
+ dnl Canonicalize the configuration name.
+ AC_CANONICAL_HOST
+ canonical=$host
+ configuration=${host_alias-${build_alias-$host}}
+ 
+ dnl We get MINGW64 with MSYS2, MINGW32 with MSYS.
+ case $canonical in
+  *-mingw*)
+   . $srcdir/nt/mingw-cfg.site
+ 
+   case $srcdir in
+     /* | ?:*)
+       # srcdir is an absolute path.  In this case, force the format
+       # "/c/foo/bar", to simplify later conversions to native Windows
+       # format ("c:/foo/bar").
+       srcdir=`cd "${srcdir}" && pwd -W`
+       # 'eval' pacifies strict POSIX non-MinGW shells (Bug#18612).
+       eval 'srcdir="/${srcdir:0:1}${srcdir:2}"'
+       ;;
+   esac;;
+ esac
+ 
  dnl Support for --program-prefix, --program-suffix and
  dnl --program-transform-name options
  AC_ARG_PROGRAM
***************
*** 452,463 ****
  		[Show Gtk+/Gdk deprecation warnings for Gtk+ >= 3.0])],
  [ac_enable_gtk_deprecation_warnings="${enableval}"],[])
  
- ### Canonicalize the configuration name.
- 
- AC_CANONICAL_HOST
- canonical=$host
- configuration=${host_alias-${build_alias-$host}}
- 
  dnl This used to use changequote, but, apart from `changequote is evil'
  dnl per the autoconf manual, we can speed up autoconf somewhat by quoting
  dnl the great gob of text.  Thus it's not processed for possible expansion.
--- 457,462 ----

Re: Whitespace in `${srcdir}' during `configure'

[Glenn Morris]

A reminder about this.
I don't intend to do any more on this, but IMO this is The Right Thing
to do.

Glenn Morris wrote:

> Please test this backport of Paul's 2014-06-10 change:
>
> *** configure.ac	2014-10-22 13:10:41 +0000
> --- configure.ac	2014-11-10 19:31:16 +0000
> ***************
> *** 24,46 ****
>   AC_PREREQ(2.65)
>   AC_INIT(emacs, 24.4.51)
>   
> - dnl We get MINGW64 with MSYS2
> - if test "x$MSYSTEM" = "xMINGW32" -o "x$MSYSTEM" = "xMINGW64"
> - then
> -   . $srcdir/nt/mingw-cfg.site
> - 
> -   case $srcdir in
> -     /* | ?:*)
> -       # srcdir is an absolute path.  In this case, force the format
> -       # "/c/foo/bar", to simplify later conversions to native Windows
> -       # format ("c:/foo/bar")
> -       srcdir=`cd "${srcdir}" && pwd -W`
> -       # 'eval' pacifies strict POSIX non-MinGW shells (Bug#18612).
> -       eval 'srcdir="/${srcdir:0:1}${srcdir:2}"'
> -       ;;
> -   esac
> - fi
> - 
>   dnl Set emacs_config_options to the options of 'configure', quoted for the shell,
>   dnl and then quoted again for a C string.  Separate options with spaces.
>   dnl Add some environment variables, if they were passed via the environment
> --- 24,29 ----
> ***************
> *** 102,107 ****
> --- 85,112 ----
>   dnl Fairly arbitrary, older versions might work too.
>   AM_INIT_AUTOMAKE(1.11)
>   
> + dnl Canonicalize the configuration name.
> + AC_CANONICAL_HOST
> + canonical=$host
> + configuration=${host_alias-${build_alias-$host}}
> + 
> + dnl We get MINGW64 with MSYS2, MINGW32 with MSYS.
> + case $canonical in
> +  *-mingw*)
> +   . $srcdir/nt/mingw-cfg.site
> + 
> +   case $srcdir in
> +     /* | ?:*)
> +       # srcdir is an absolute path.  In this case, force the format
> +       # "/c/foo/bar", to simplify later conversions to native Windows
> +       # format ("c:/foo/bar").
> +       srcdir=`cd "${srcdir}" && pwd -W`
> +       # 'eval' pacifies strict POSIX non-MinGW shells (Bug#18612).
> +       eval 'srcdir="/${srcdir:0:1}${srcdir:2}"'
> +       ;;
> +   esac;;
> + esac
> + 
>   dnl Support for --program-prefix, --program-suffix and
>   dnl --program-transform-name options
>   AC_ARG_PROGRAM
> ***************
> *** 452,463 ****
>   		[Show Gtk+/Gdk deprecation warnings for Gtk+ >= 3.0])],
>   [ac_enable_gtk_deprecation_warnings="${enableval}"],[])
>   
> - ### Canonicalize the configuration name.
> - 
> - AC_CANONICAL_HOST
> - canonical=$host
> - configuration=${host_alias-${build_alias-$host}}
> - 
>   dnl This used to use changequote, but, apart from `changequote is evil'
>   dnl per the autoconf manual, we can speed up autoconf somewhat by quoting
>   dnl the great gob of text.  Thus it's not processed for possible expansion.
> --- 457,462 ----