Re: [PATCH 2/2] sh-setup: make require_clean_work_tree() work on orphan branches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 24.11.2015 um 15:45 schrieb SZEDER Gábor:
git-sh-setup's require_clean_work_tree() always exits with error on an
orphan branch, even when the index and worktree are both clean.  The
reason is that require_clean_work_tree() starts off with verifying
HEAD, to make sure that it can safely pass HEAD to 'git diff-index'
later when it comes to checking the cleanness of the index, and errors
out finding the invalid HEAD of the orphan branch.

There are scripts requiring a clean worktree that should work on an
orphan branch as well, and those should be able to use this function
instead of duplicating its functionality and nice error reporting in a
way that handles orphan branches.

Fixing this is easy: the index should be compared to the empty tree
while on an orphan branch, and to HEAD otherwise.

However, just fixing require_clean_work_tree() this way is also
dangerous, because scripts must take care to work properly on orphan
branches.  Currently a script calling require_clean_work_tree() would
exit on a clean orphan branch, but with the simple fix it would
continue executing and who knows what the consequences might be if
the script is not prepared for orphan branches.

Let scripts tell git-sh-setup that they are capable of dealing with
orphan branches by setting the ORPHAN_OK variable, similar to how the
ability to run in a subdirectory must be signalled by setting
SUBDIRECTORY_OK.  Only if ORPHAN_OK is set while on an orphan branch
will require_clean_work_tree() go on and compare the index and
worktree to the empty tree, otherwise it will exit with error even
when the index and worktree are clean, i.e the default exit behavior
doesn't change.

Also provide an error message in the orphan branch/invalid HEAD case
that is consistent in style with the function's other error messages
instead of the error message coming straight from 'git rev-parse
--verify'.

Signed-off-by: SZEDER Gábor <szeder@xxxxxxxxxx>
---
  Documentation/git-sh-setup.txt     |  3 ++-
  git-sh-setup.sh                    | 16 ++++++++++++++--
  t/t2301-require-clean-work-tree.sh | 29 +++++++++++++++++++++++++++++
  3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 4f67c4cde6..bccaa2488f 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -25,7 +25,8 @@ Before sourcing it, your script should set up a few variables;
  `USAGE` (and `LONG_USAGE`, if any) is used to define message
  given by `usage()` shell function.  `SUBDIRECTORY_OK` can be set
  if the script can run from a subdirectory of the working tree
-(some commands do not).
+(some commands do not).  `ORPHAN_OK` can be set if the script can
+work on orphan branches.

  The scriptlet sets `GIT_DIR` and `GIT_OBJECT_DIRECTORY` shell
  variables, but does *not* export them to the environment.
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 4691fbcb64..f45b69386e 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -200,7 +200,19 @@ require_work_tree () {
  }

  require_clean_work_tree () {
-	git rev-parse --verify HEAD >/dev/null || exit 1
+	if git rev-parse --verify HEAD >/dev/null 2>/dev/null
+	then
+		compare_to=HEAD
+	else
+		if [ -z "$ORPHAN_OK" ]
+		then
+			echo >&2 "Cannot $1: Your current branch does not have any commits yet."
+			exit 1
+		else
+			# SHA1 of an empty tree
+			compare_to=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+		fi
+	fi

It is worrysome that this now throws away any error message of rev-parse. A more conservative approach would be to test for -z "$ORPHAN_OK" first and entail new behavior only for the "$ORPHAN_OK" case.

  	git update-index -q --ignore-submodules --refresh
  	err=0

@@ -210,7 +222,7 @@ require_clean_work_tree () {
  		err=1
  	fi

-	if ! git diff-index --cached --quiet --ignore-submodules HEAD --
+	if ! git diff-index --cached --quiet --ignore-submodules $compare_to --
  	then
  		if [ $err = 0 ]
  		then
diff --git a/t/t2301-require-clean-work-tree.sh b/t/t2301-require-clean-work-tree.sh
index 1bb41b59a5..6d0957981e 100755
--- a/t/t2301-require-clean-work-tree.sh
+++ b/t/t2301-require-clean-work-tree.sh
@@ -60,4 +60,33 @@ test_expect_success 'error on dirty index and work tree while on orphan branch'
  	test_must_fail run_require_clean_work_tree
  '

+test_expect_success 'ORPHAN_OK - success on clean index and worktree while on orphan branch' '
+	test_when_finished "git checkout master" &&
+	git checkout --orphan orphan &&
+	git reset --hard &&
+	(
+		ORPHAN_OK=Yes &&
+		run_require_clean_work_tree
+	)
+'
+
+test_expect_success 'ORPHAN_OK - error on dirty index while on orphan branch' '
+	test_when_finished "git checkout master" &&
+	git checkout --orphan orphan &&
+	(
+		ORPHAN_OK=Yes &&
+		test_must_fail run_require_clean_work_tree
+	)
+'
+
+test_expect_success 'ORPHAN_OK - error on dirty index and worktree while on orphan branch' '
+	test_when_finished "git checkout master" &&
+	git checkout --orphan orphan &&
+	echo dirty >file &&
+	(
+		ORPHAN_OK=Yes &&
+		test_must_fail run_require_clean_work_tree
+	)
+'

Tests with ORPHAN_OK=Yes, but being on a non-orphaned branch are missing.

-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]