Subject: bug#14991: distcheck passes --prefix to configure before *DISTCHECK_CONFIGURE_FLAGS

From: Stefano Lattarini <[email-masked]>
Date: Wed, 30 Oct 2013 22:02:28 +0000

Reference: <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=14991>

tags 14991 + patch
close 14991
thanks

On 07/31/2013 08:37 AM, Akim Demaille wrote:
> Hi!
Hi Akim, thanks for the report, and sorry for the awful delay.

> Admittedly, what prompts this report is arguably a bug in
> a package: It passes _all_ the configure flags to
> AM_DISTCHECK_CONFIGURE_FLAGS.  Not a bright idea I guess,
> but simple.
> 
> Unfortunately distcheck reads:
> 
> # This target untars the dist file and tries a VPATH configuration.  Then
> # it guarantees that the distribution is self-contained by making another
> # tarfile.
> distcheck: dist
> 	case '$(DIST_ARCHIVES)' in \
> 	*.tar.gz*) \
> …
> 	  && ../configure --srcdir=.. --prefix="$$dc_install_base" \
> 	    $(AM_DISTCHECK_CONFIGURE_FLAGS) \
> 	    $(DISTCHECK_CONFIGURE_FLAGS) \
> 	  && $(MAKE) $(AM_MAKEFLAGS) \
> 
> So the "local" flags, --srcdir and especially --prefix, are
> provided by distcheck, and then overridden by the user's
> DISTCHECK_CONFIGURE_FLAGS.  Since I use --prefix, boom.
> 
> Very much agreed, --prefix should not have made it into
> (AM_)DISTCHECK_CONFIGURE_FLAGS; however it seems simple to
> just pass --srcdir and --prefix last to be protected against
> such cases.
>
I agree.  The code in "make distcheck" implicitly assumes that the
values for --srcdir and --prefix it defines cannot be changed, so
it makes no sense to allow them to be overridden.

I've fixed the issue with the two attached patches, that will appear
in Automake 1.14.1 (someday when I'll actually get around to release
it ;-).  I will wait some time before pushing the patches out, so a
review is welcome.

Regards,
  Stefano
>From 608d1a7908893b2896f5efd2a4ed22d7901262ed Mon Sep 17 00:00:00 2001
Message-Id: <608d1a790[email-masked]>
From: Stefano Lattarini <[email-masked]>
Date: Wed, 30 Oct 2013 21:02:14 +0000
Subject: [PATCH 1/2] tests: expose bug#14991 (relates to 'distcheck')

* t/distcheck-no-prefix-or-srcdir-override.sh: New, expose the bug.
* t/list-of-tests.mk (handwritten_TESTS, XFAIL_TESTS): Add it.

Signed-off-by: Stefano Lattarini <[email-masked]>
---
 t/distcheck-no-prefix-or-srcdir-override.sh | 60 +++++++++++++++++++++++++++++
 t/list-of-tests.mk                          |  2 +
 2 files changed, 62 insertions(+)
 create mode 100644 t/distcheck-no-prefix-or-srcdir-override.sh

diff --git a/t/distcheck-no-prefix-or-srcdir-override.sh b/t/distcheck-no-prefix-or-srcdir-override.sh
new file mode 100644
index 0000000..9b9a56f
--- /dev/null
+++ b/t/distcheck-no-prefix-or-srcdir-override.sh
@@ -0,0 +1,60 @@
+#! /bin/sh
+# Copyright (C) 2013 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that "make distcheck" overrides any --srcdir or --prefix flag
+# (mistakenly) defined in $(AM_DISTCHECK_CONFIGURE_FLAGS) or
+# $(DISTCHECK_CONFIGURE_FLAGS).  See automake bug#14991.
+
+. test-init.sh
+
+echo AC_OUTPUT >> configure.ac
+
+orig_cwd=$(pwd); export orig_cwd
+
+cat > Makefile.am << 'END'
+# configure should choke on non-absolute prefix or non-existent
+# srcdir.  We'll sanity-check that later.
+AM_DISTCHECK_CONFIGURE_FLAGS = --srcdir am-src --prefix am-pfx
+END
+
+# Same comments as above applies.
+DISTCHECK_CONFIGURE_FLAGS='--srcdir user-src --prefix user-pfx'
+export DISTCHECK_CONFIGURE_FLAGS
+
+$ACLOCAL
+$AUTOMAKE
+$AUTOCONF
+
+# Sanity check: configure should choke on non-absolute prefix
+# or non-existent srcdir.
+./configure --prefix foobar 2>stderr && { cat stderr >&2; exit 99; }
+cat stderr >&2
+grep "expected an absolute directory name for --prefix" stderr || exit 99
+./configure --srcdir foobar 2>stderr && { cat stderr >&2; exit 99; }
+cat stderr >&2
+grep "cannot find sources.* in foobar" stderr || exit 99
+
+./configure
+run_make -E -O distcheck
+test ! -s stderr
+# Sanity check: the flags have been actually seen.
+$PERL -e 'undef $/; $_ = <>; s/ \\\n/ /g; print;' <stdout >t
+grep '/configure .* --srcdir am-src'   t || exit 99
+grep '/configure .* --prefix am-pfx'   t || exit 99
+grep '/configure .* --srcdir user-src' t || exit 99
+grep '/configure .* --prefix user-pfx' t || exit 99
+
+:
diff --git a/t/list-of-tests.mk b/t/list-of-tests.mk
index 9069b08..8bdbdc0 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -35,6 +35,7 @@ t/gcj6.sh \
 t/override-conditional-2.sh \
 t/override-conditional-pr13940.sh \
 t/dist-pr109765.sh \
+t/distcheck-no-prefix-or-srcdir-override.sh \
 t/instdir-cond2.sh \
 t/java-nobase.sh \
 t/objext-pr10128.sh \
@@ -422,6 +423,7 @@ t/distcheck-hook2.sh \
 t/distcheck-writable-srcdir.sh \
 t/distcheck-missing-m4.sh \
 t/distcheck-outdated-m4.sh \
+t/distcheck-no-prefix-or-srcdir-override.sh \
 t/distcheck-override-infodir.sh \
 t/distcheck-pr9579.sh \
 t/distcheck-pr10470.sh \
-- 
1.8.3.1.605.g85318f5

>From cab362eee5f19c02ed6719449d1d27644f8ffc93 Mon Sep 17 00:00:00 2001
Message-Id: <cab362eee[email-masked]>
In-Reply-To: <608d1a790[email-masked]>
References: <608d1a790[email-masked]>
From: Stefano Lattarini <[email-masked]>
Date: Wed, 30 Oct 2013 21:41:39 +0000
Subject: [PATCH 2/2] distcheck: don't allow overriding of --prefix and
 --srcdir by the user

Not trough AM_DISTCHECK_FLAGS, nor through DISTCHECK_FLAGS.  Apparently,
some packages got in the habit of relaying all the options passed to the
original ./configure invocation through to the configure invocations
in "make distcheck".  This was causing problems, because it also passed
through the original --srcdir and --prefix options.

Fixes: expose bug#14991 (relates to 'distcheck')

* lib/am/distdir.am (distcheck): Pass the hard-coded --srcdir and
--prefix options *after* both the developer-defined options in
$(AM_DISTCHECK_FLAGS) and the user-defined options in $(DISTCHECK_FLAGS).
* t/list-of-tests.mk (XFAIL_TESTS): Remove the now-passing test
'distcheck-no-destdist-or-srcdir-override.sh'.
* doc/automake.texi (Checking the Distribution): Update.
* NEWS: Likewise.

Signed-off-by: Stefano Lattarini <[email-masked]>
---
 NEWS               |  3 +++
 doc/automake.texi  | 17 ++++++++++++-----
 lib/am/distdir.am  | 11 +++++++----
 t/list-of-tests.mk |  1 -
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index aaec7c0..77dcbbc 100644
--- a/NEWS
+++ b/NEWS
@@ -108,6 +108,9 @@ New in 1.14.1:
 
 * Bugs fixed:
 
+  - The user is no longer allowed to override the --srcdir nor the --prefix
+    configure options used by "make distcheck" (bug#14991).
+
   - Fixed a gross inefficiency in the recipes for installing byte-compiled
     python files, that was causing an O(N^2) performance on the number N of
     files, instead of the expected O(N) performance.  Note that this bug
diff --git a/doc/automake.texi b/doc/automake.texi
index 62728d4..cd33ad7 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -8556,11 +8556,18 @@ to supply additional flags to @command{configure}, define them in the
 @file{Makefile.am}.  The user can still extend or override the flags
 provided there by defining the @code{DISTCHECK_CONFIGURE_FLAGS} variable,
 on the command line when invoking @command{make}.
-
-Still, developers are encouraged to strive to make their code buildable
-without requiring any special configure option; thus, in general, you
-shouldn't define @code{AM_DISTCHECK_CONFIGURE_FLAGS}. However, there
-might be few scenarios in which the use of this variable is justified.
+@c See automake bug#14991 for more details about how the following holds.
+It's worth nothing that @command{make distcheck} needs complete control
+over the @command{configure} options @option{--srcdir} and
+@option{--prefix}, so those options cannot be overridden by
+@code{AM_DISTCHECK_CONFIGURE_FLAGS} nor by
+@code{DISTCHECK_CONFIGURE_FLAGS}.
+
+Also note that developers are encouraged to strive to make their code
+buildable without requiring any special configure option; thus, in
+general, you shouldn't define @code{AM_DISTCHECK_CONFIGURE_FLAGS}.
+However, there might be few scenarios in which the use of this variable
+is justified.
 GNU @command{m4} offers an example.  GNU @command{m4} configures by
 default with its experimental and seldom used "changeword" feature
 disabled; so in its case it is useful to have @command{make distcheck}
diff --git a/lib/am/distdir.am b/lib/am/distdir.am
index f354987..a8ad63c 100644
--- a/lib/am/distdir.am
+++ b/lib/am/distdir.am
@@ -452,13 +452,16 @@ distcheck: dist
 ## so be sure to 'cd' back to the original directory after this.
 	  && am__cwd=`pwd` \
 	  && $(am__cd) $(distdir)/_build \
-	  && ../configure --srcdir=.. --prefix="$$dc_install_base" \
+	  && ../configure \
 ?GETTEXT?	    --with-included-gettext \
-## Additional flags for configure.  Keep this last in the configure
-## invocation so the developer and user can override previous options,
-## and let the user's flags take precedence over the developer's ones.
+## Additional flags for configure.
 	    $(AM_DISTCHECK_CONFIGURE_FLAGS) \
 	    $(DISTCHECK_CONFIGURE_FLAGS) \
+## At the moment, the code doesn't actually support changes in these --srcdir
+## and --prefix values, so don't allow them to be overridden by the user or
+## the developer.  That used to be allowed, and caused issues in practice
+## (in corner-case usages); see automake bug#14991.
+	    --srcdir=.. --prefix="$$dc_install_base" \
 	  && $(MAKE) $(AM_MAKEFLAGS) \
 	  && $(MAKE) $(AM_MAKEFLAGS) dvi \
 	  && $(MAKE) $(AM_MAKEFLAGS) check \
diff --git a/t/list-of-tests.mk b/t/list-of-tests.mk
index 8bdbdc0..75f303a 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -35,7 +35,6 @@ t/gcj6.sh \
 t/override-conditional-2.sh \
 t/override-conditional-pr13940.sh \
 t/dist-pr109765.sh \
-t/distcheck-no-prefix-or-srcdir-override.sh \
 t/instdir-cond2.sh \
 t/java-nobase.sh \
 t/objext-pr10128.sh \
-- 
1.8.3.1.605.g85318f5