git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: "H.Merijn Brand" <h.m.brand@xs4all.nl>,
	Andreas Ericsson <ae@op5.se>,
	git@vger.kernel.org, Sam Vilain <sam@vilain.net>
Subject: [PATCH] Do not rely on the exit status of "unset" for unset variables
Date: Tue, 4 Dec 2007 22:45:16 +0000 (GMT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0712042242310.27959@racer.site> (raw)
In-Reply-To: <7vve7e49or.fsf@gitster.siamese.dyndns.org>


From: H.Merijn Brand <h.m.brand@xs4all.nl>

POSIX says that exit status "0" means that "unset" successfully unset
the variable.  However, it is kind of ambiguous if an environment
variable which was not set could be successfully unset.

At least the default shell on HP-UX insists on reporting an error in
such a case, so just ignore the exit status of "unset".

[Dscho: extended the patch to git-submodule.sh, as Junio realized that
 this is the only other place where we check the exit status of "unset".]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Tue, 4 Dec 2007, Junio C Hamano wrote:

	> "H.Merijn Brand" <h.m.brand@xs4all.nl> writes:
	> 
	> > On Tue, 4 Dec 2007 15:39:47 +0000 (GMT), Johannes Schindelin
	> > ...
	> > I found it! unset returns false
	> > ...
	> > I must leave now.
	> 
	> Thanks, you two.
	> 
	> I do not see "unset VAR... &&" outside t0001 test, but there are
	> instances of "unset VAR... &&" in git-submodule implementations 
	> as well.
	> 
	> In short, not too many places to fix.

	You're right.  I grepped for "unset" in t/*.sh, but that catches 
	only false positives other than t0001.

	Merijn, maybe you want to have your sign-off in the commit 
	message?

 git-submodule.sh |   10 +++++-----
 t/t0001-init.sh  |   16 ++++++++--------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 82ac28f..ad9fe62 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -158,7 +158,7 @@ module_add()
 	die "'$path' already exists in the index"
 
 	module_clone "$path" "$realrepo" || exit
-	(unset GIT_DIR && cd "$path" && git checkout -q ${branch:+-b "$branch" "origin/$branch"}) ||
+	(unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "origin/$branch"}) ||
 	die "Unable to checkout submodule '$path'"
 	git add "$path" ||
 	die "Failed to add submodule '$path'"
@@ -228,14 +228,14 @@ modules_update()
 			module_clone "$path" "$url" || exit
 			subsha1=
 		else
-			subsha1=$(unset GIT_DIR && cd "$path" &&
+			subsha1=$(unset GIT_DIR; cd "$path" &&
 				git rev-parse --verify HEAD) ||
 			die "Unable to find current revision in submodule path '$path'"
 		fi
 
 		if test "$subsha1" != "$sha1"
 		then
-			(unset GIT_DIR && cd "$path" && git-fetch &&
+			(unset GIT_DIR; cd "$path" && git-fetch &&
 				git-checkout -q "$sha1") ||
 			die "Unable to checkout '$sha1' in submodule path '$path'"
 
@@ -246,7 +246,7 @@ modules_update()
 
 set_name_rev () {
 	revname=$( (
-		unset GIT_DIR &&
+		unset GIT_DIR
 		cd "$1" && {
 			git describe "$2" 2>/dev/null ||
 			git describe --tags "$2" 2>/dev/null ||
@@ -285,7 +285,7 @@ modules_list()
 		else
 			if test -z "$cached"
 			then
-				sha1=$(unset GIT_DIR && cd "$path" && git rev-parse --verify HEAD)
+				sha1=$(unset GIT_DIR; cd "$path" && git rev-parse --verify HEAD)
 				set_name_rev "$path" "$sha1"
 			fi
 			say "+$sha1 $path$revname"
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index b14b3ec..c015405 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -25,7 +25,7 @@ check_config () {
 
 test_expect_success 'plain' '
 	(
-		unset GIT_DIR GIT_WORK_TREE &&
+		unset GIT_DIR GIT_WORK_TREE
 		mkdir plain &&
 		cd plain &&
 		git init
@@ -35,7 +35,7 @@ test_expect_success 'plain' '
 
 test_expect_success 'plain with GIT_WORK_TREE' '
 	if (
-		unset GIT_DIR &&
+		unset GIT_DIR
 		mkdir plain-wt &&
 		cd plain-wt &&
 		GIT_WORK_TREE=$(pwd) git init
@@ -48,7 +48,7 @@ test_expect_success 'plain with GIT_WORK_TREE' '
 
 test_expect_success 'plain bare' '
 	(
-		unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&
+		unset GIT_DIR GIT_WORK_TREE GIT_CONFIG
 		mkdir plain-bare-1 &&
 		cd plain-bare-1 &&
 		git --bare init
@@ -58,7 +58,7 @@ test_expect_success 'plain bare' '
 
 test_expect_success 'plain bare with GIT_WORK_TREE' '
 	if (
-		unset GIT_DIR GIT_CONFIG &&
+		unset GIT_DIR GIT_CONFIG
 		mkdir plain-bare-2 &&
 		cd plain-bare-2 &&
 		GIT_WORK_TREE=$(pwd) git --bare init
@@ -72,7 +72,7 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '
 test_expect_success 'GIT_DIR bare' '
 
 	(
-		unset GIT_CONFIG &&
+		unset GIT_CONFIG
 		mkdir git-dir-bare.git &&
 		GIT_DIR=git-dir-bare.git git init
 	) &&
@@ -82,7 +82,7 @@ test_expect_success 'GIT_DIR bare' '
 test_expect_success 'GIT_DIR non-bare' '
 
 	(
-		unset GIT_CONFIG &&
+		unset GIT_CONFIG
 		mkdir non-bare &&
 		cd non-bare &&
 		GIT_DIR=.git git init
@@ -93,7 +93,7 @@ test_expect_success 'GIT_DIR non-bare' '
 test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '
 
 	(
-		unset GIT_CONFIG &&
+		unset GIT_CONFIG
 		mkdir git-dir-wt-1.git &&
 		GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-1.git git init
 	) &&
@@ -103,7 +103,7 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '
 test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '
 
 	if (
-		unset GIT_CONFIG &&
+		unset GIT_CONFIG
 		mkdir git-dir-wt-2.git &&
 		GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-2.git git --bare init
 	)
-- 
1.5.3.7.2139.g2a5a3

  parent reply	other threads:[~2007-12-04 22:46 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-04 13:09 Building git-1.5.3.7 on HP-UX 11.00 H.Merijn Brand
2007-12-04 13:44 ` Johannes Schindelin
2007-12-04 14:03   ` H.Merijn Brand
2007-12-04 14:40     ` Johannes Schindelin
2007-12-04 15:01       ` H.Merijn Brand
2007-12-04 15:14         ` Andreas Ericsson
2007-12-04 15:22           ` H.Merijn Brand
2007-12-04 15:39             ` Johannes Schindelin
2007-12-04 15:56               ` H.Merijn Brand
2007-12-04 16:28                 ` Johannes Schindelin
2007-12-05 10:49                   ` H.Merijn Brand
2007-12-04 18:05                 ` Junio C Hamano
2007-12-04 22:25                   ` H.Merijn Brand
2007-12-04 22:46                     ` Johannes Schindelin
2007-12-05  8:08                       ` H.Merijn Brand
2007-12-04 22:45                   ` Johannes Schindelin [this message]
2007-12-05  8:01                     ` [PATCH] Do not rely on the exit status of "unset" for unset variables H.Merijn Brand
2007-12-10 14:51                   ` Building git-1.5.3.7 on HP-UX 11.00 H.Merijn Brand
2007-12-11  8:26                     ` Junio C Hamano
2007-12-11  8:54                       ` Shawn O. Pearce
2007-12-11 12:57                         ` H.Merijn Brand
2007-12-11  9:20                       ` Johannes Sixt
2007-12-11 10:42                         ` H.Merijn Brand
2007-12-11 11:01                           ` Johannes Sixt
2007-12-11 13:33                       ` H.Merijn Brand
2007-12-11 13:53                         ` Johannes Sixt
2007-12-04 15:11       ` H.Merijn Brand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0712042242310.27959@racer.site \
    --to=johannes.schindelin@gmx.de \
    --cc=ae@op5.se \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=h.m.brand@xs4all.nl \
    --cc=sam@vilain.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).