[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Fix to bootstrap
Re: Fix to bootstrap
Thu, 19 Apr 2012 16:09:40 +0200
Reuben Thomas wrote:
> On 19 April 2012 14:28, Jim Meyering <address@hidden> wrote:
>> Thanks for tracking that down.
>> However, rather than handling just that one "cd",
>> how about ensuring that no use of "cd" in this script
>> can cause such trouble? To do that, you would unset CDPATH
>> somewhere near the top, with a comment explaining its purpose.
> That would not deal with "cd -". It also assumes that CDPATH would
> never be relied on.
bootstrap will never use "cd -",
and portable scripts should not rely on CDPATH.
> Currently there are two uses of cd in bootstrap, only one of which
> (the one I fixed) has its output used.
The idea is to make the code a little more future/maintenance proof.
If we leave it so "cd" might in some unusual cases (like yours)
generate output, a new use of "cd" might also be in a place
that can cause malfunction. Unsetting CDPATH protects against that,
while addressing only the particular "cd" that hurts today does not.
> To make the intention clear, it would seem more sensible to have some
> sort of wrapper (function? script?) for cd which never produces
> output. Or is that overkill? (Arguably, it's a misfeature of cd.)
> Anyway, I leave those more complex issues to you; you can take my fix
> for now or adapt it as you see fit.
I've fixed it with this:
>From 28c2a36d502603738c26d59f7d9bd16f27010835 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 19 Apr 2012 16:02:23 +0200
Subject: [PATCH] bootstrap: don't let a user's CDPATH setting affect this
When CDPATH is set, cd will sometimes generate output.
When "cd" is run in a subshell whose output matters, that
surprising-to-some output can cause malfunction.
Unsetting CDPATH turns off this shell "feature."
* build-aux/bootstrap (CDPATH): Unset.
Reported by Reuben Thomas in:
and inspired by his patch here:
ChangeLog | 13 +++++++++++++
build-aux/bootstrap | 6 +++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index c980aa3..693ad24 100644
@@ -1,3 +1,16 @@
+2012-04-19 Jim Meyering <address@hidden>
+ bootstrap: don't let a user's CDPATH setting affect this script
+ When CDPATH is set, cd will sometimes generate output.
+ When "cd" is run in a subshell whose output matters, that
+ surprising-to-some output can cause malfunction.
+ Unsetting CDPATH turns off this shell "feature."
+ * build-aux/bootstrap (CDPATH): Unset.
+ Reported by Reuben Thomas in:
+ and inspired by his patch here:
2012-04-16 Akim Demaille <address@hidden>
and Jim Meyering <address@hidden>
diff --git a/build-aux/bootstrap b/build-aux/bootstrap
index 16dc15c..4afc6c4 100755
@@ -1,6 +1,6 @@
# Print a version string.
-scriptversion=2012-04-16.16; # UTC
+scriptversion=2012-04-19.14; # UTC
# Bootstrap this package from checked-out sources.
@@ -36,6 +36,10 @@ nl='
+# Ensure that CDPATH is not set. Otherwise, the output from cd
+# would cause trouble in at least one use below.
+(unset CDPATH) >/dev/null 2>&1 && unset CDPATH