[PATCH amend] git-bisect.sh: don't accidentally override existing branch "bisect"

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

 



If a branch named "bisect" or "new-bisect" already was created in the
repo by other means than git bisect, doing a git bisect used to override
the branch without a warning.  Now if the branch "bisect" or
"new-bisect" already exists, and it was not created by git bisect itself,
git bisect start fails with an appropriate error message.  Additionally,
if checking out a new bisect state fails due to a merge problem, git
bisect cleans up the temporary branch "new-bisect".

The accidental override has been noticed by Andres Salomon, reported
through
 http://bugs.debian.org/478647

Signed-off-by: Gerrit Pape <pape@xxxxxxxxxxx>
---

On Wed, Apr 30, 2008 at 11:30:18PM +0200, Christian Couder wrote:
> >     echo "Bisecting: $bisect_nr revisions left to test after this"
> >     git branch -f new-bisect "$bisect_rev"
> > -   git checkout -q new-bisect || exit
> > +   git checkout -q new-bisect || {
> > +           git branch -d new-bisect
> > +           exit
>
> Here we "exit 0" if "git branch -d new-bisect" succeeds.
> That seems wrong.

Thanks, I changed it to first delete the new-bisect branch, and then use
git checkout to create the branch and do the checkout.  So we have the
exit code from git branch if it fails.

On Thu, May 01, 2008 at 02:27:22PM +0200, Christian Couder wrote:
> Le jeudi 1 mai 2008, Richard Quirk a écrit :
> > Careful with that - it's a bashism and would fail if /bin/sh is
> > dash.
> > ie it would say that a branch called literally "{new-,}bisect" does
> > not exist, even if new-bisect and bisect do.
>
> You are right. Thanks.
> So what about a plain:
>
> git show-ref -q bisect new-bisect

I'm not sure we can rely on this, the exit code of that command if one
of the branches exists, but not the other, isn't documented.  The patch
now uses --verify -q on each branch, which is documented and should be
reliable.


 Documentation/git-bisect.txt |    2 +-
 git-bisect.sh                |   19 ++++++++++++-------
 t/t6030-bisect-porcelain.sh  |   18 ++++++++++++++++++
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 698ffde..1c7e38d 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -85,7 +85,7 @@ Oh, and then after you want to reset to the original head, do a
 $ git bisect reset
 ------------------------------------------------
 
-to get back to the master branch, instead of being in one of the
+to get back to the original branch, instead of being in one of the
 bisection branches ("git bisect start" will do that for you too,
 actually: it will reset the bisection state, and before it does that
 it checks that you're not using some old bisection branch).
diff --git a/git-bisect.sh b/git-bisect.sh
index d8d9bfd..f8c411a 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -69,14 +69,19 @@ bisect_start() {
 	head=$(GIT_DIR="$GIT_DIR" git symbolic-ref -q HEAD) ||
 	head=$(GIT_DIR="$GIT_DIR" git rev-parse --verify HEAD) ||
 	die "Bad HEAD - I need a HEAD"
+	#
+	# Check that we either already have BISECT_START, or that the
+	# branches bisect, new-bisect don't exist, to not override them.
+	#
+	test -s "$GIT_DIR/BISECT_START" ||
+		if git show-ref --verify -q refs/heads/bisect ||
+		    git show-ref --verify -q refs/heads/new-bisect; then
+			die 'The branches "bisect" and "new-bisect" must not exist.'
+		fi
 	start_head=''
 	case "$head" in
 	refs/heads/bisect)
-		if [ -s "$GIT_DIR/BISECT_START" ]; then
-		    branch=`cat "$GIT_DIR/BISECT_START"`
-		else
-		    branch=master
-		fi
+		branch=`cat "$GIT_DIR/BISECT_START"`
 		git checkout $branch || exit
 		;;
 	refs/heads/*|$_x40)
@@ -328,8 +333,8 @@ bisect_next() {
 	exit_if_skipped_commits "$bisect_rev"
 
 	echo "Bisecting: $bisect_nr revisions left to test after this"
-	git branch -f new-bisect "$bisect_rev"
-	git checkout -q new-bisect || exit
+	git branch -D new-bisect
+	git checkout -q -b new-bisect "$bisect_rev" || exit
 	git branch -M new-bisect bisect
 	git show-branch "$bisect_rev"
 }
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 5e3e544..05f1e15 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -284,6 +284,24 @@ test_expect_success 'bisect starting with a detached HEAD' '
 
 '
 
+test_expect_success 'bisect refuses to start if branch bisect exists' '
+	git bisect reset &&
+	git branch bisect &&
+	test_must_fail git bisect start &&
+	git branch -d bisect &&
+	git checkout -b bisect &&
+	test_must_fail git bisect start &&
+	git checkout master &&
+	git branch -d bisect
+'
+
+test_expect_success 'bisect refuses to start if branch new-bisect exists' '
+	git bisect reset &&
+	git branch new-bisect &&
+	test_must_fail git bisect start &&
+	git branch -d new-bisect
+'
+
 #
 #
 test_done
-- 
1.5.5.1

--
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]

  Powered by Linux