Is reserving the branch name "bisect" a good thing?

Is reserving the branch name "bisect" a good thing?

[linux]

Just wondering... most of the "magic" references are in $GIT_DIR
directly, and ALL_CAPS.  "git bisect start" begins with
"rm -f $GIT_DIR/refs/heads/bisect", which could catch someone
trying to implement a bisection algorithm in their own code.

Would it be better if "git bisect" followed that rule as well?
Otherwise, we really should document the reserved word.

Either that, or use refs/bisect/current and avoid the issue entirely.

Something like (untested):

diff --git a/git-bisect.sh b/git-bisect.sh
index 68838f3..19a8f36 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -50,7 +50,7 @@ bisect_start() {
 	head=$(GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD) ||
 	die "Bad HEAD - I need a symbolic ref"
 	case "$head" in
-	refs/heads/bisect*)
+	BISECT*)
 		git checkout master || exit
 		;;
 	refs/heads/*)
@@ -63,7 +63,7 @@ bisect_start() {
 	#
 	# Get rid of any old bisect state
 	#
-	rm -f "$GIT_DIR/refs/heads/bisect"
+	rm -f "$GIT_DIR/BISECT"
 	rm -rf "$GIT_DIR/refs/bisect/"
 	mkdir "$GIT_DIR/refs/bisect"
 	{
@@ -146,10 +146,10 @@ bisect_next() {
 	fi
 	nr=$(eval "git-rev-list $rev $good -- $(cat $GIT_DIR/BISECT_NAMES)" | wc -l) || exit
 	echo "Bisecting: $nr revisions left to test after this"
-	echo "$rev" > "$GIT_DIR/refs/heads/new-bisect"
-	git checkout new-bisect || exit
-	mv "$GIT_DIR/refs/heads/new-bisect" "$GIT_DIR/refs/heads/bisect" &&
-	GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD refs/heads/bisect
+	echo "$rev" > "$GIT_DIR/NEW-BISECT"
+	git checkout NEW-BISECT || exit
+	mv "$GIT_DIR/NEW-BISECT" "$GIT_DIR/BISECT" &&
+	GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD BISECT
 	git-show-branch "$rev"
 }
 
@@ -172,7 +172,7 @@ bisect_reset() {
 	esac
 	git checkout "$branch" &&
 	rm -fr "$GIT_DIR/refs/bisect"
-	rm -f "$GIT_DIR/refs/heads/bisect"
+	rm -f "$GIT_DIR/BISECT"
 	rm -f "$GIT_DIR/BISECT_LOG"
 }
 

Re: Is reserving the branch name "bisect" a good thing?

[Junio C Hamano]

 <linux <at> horizon.com> writes:

> Would it be better if "git bisect" followed that rule as well?
> Otherwise, we really should document the reserved word.

I wonder if you broke "git bisect visualize" with that patch.

Re: Is reserving the branch name "bisect" a good thing?

[linux]

>> Would it be better if "git bisect" followed that rule as well?
>> Otherwise, we really should document the reserved word.

> I wonder if you broke "git bisect visualize" with that patch.

I don't know, but I sure broke git-bisect.

The problem is that git-checkout won't switch branches if the
ref given is not in $GIT_DIR/refs/heads.  (Try to include a "heads/"
prefix on a non-selected existing branch to see.)

Without changing this policy in git-checkout, or replicating most
of git-checkout's code in git-bisect, I can't get away from using
a head name in refs/heads/.  :-(

Stepping back, the problem is that
- git has a policy against allowing checkins against a ref not in refs/heads/
- git-commit doesn't have a concept of an unwriteable HEAD, so
- git-checkout refuses to set HEAD to an unwriteable ref (not in refs/heads/)
But "git bisect" wants exactly this sort of historical snapshot.

Actually, this leads to a question... suppose I want to manually
check out some old revision (like "v.2.6.12") for some reason
(performance testing, say).  How do I do that?
Do I have to create a branch just to do that?

It might be nicer to allow such a checkout and do the checking in
git-commit, telling you to "git-checkout -b <new_branch>" before
you check in your edits.


In git-bisect, I finally managed to do a bit of a hack, but it's
a bit annoying for doing the above from the command line.
(Error handling could also probably use improvement.)

See the "@@ -146,10 +160,12 @@ bisect_next() {" chunk, third
from the end, for the meat.


diff --git a/git-bisect.sh b/git-bisect.sh
index 68838f3..c8b9d7b 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,9 @@
 #!/bin/sh
 . git-sh-setup
 
+# Put arguments in single quotes to they can be re-interpreted by the shell.
+# (To put a single quote in a single-quoted string, you need to write
+# 'wasn'\''t' = 'wasn' + \' + 't'.)
 sq() {
 	perl -e '
 		for (@ARGV) {
@@ -42,6 +45,18 @@ bisect_autostart() {
 	}
 }
 
+# Not generally needed, but provide a cleanup function
+bisect_stop() {
+	case "$(cat "$GIT_DIR/HEAD")" in
+	refs/bisect/*)
+		echo "Resetting HEAD to master"
+		git checkout master || exit
+		;;
+	esac
+	rm -rf "$GIT_DIR/refs/bisect/"
+	rm -f "$GIT_DIR/BISECT_LOG" "$GIT_DIR/BISECT_NAMES"
+}
+
 bisect_start() {
 	#
 	# Verify HEAD. If we were bisecting before this, reset to the
@@ -50,7 +65,7 @@ bisect_start() {
 	head=$(GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD) ||
 	die "Bad HEAD - I need a symbolic ref"
 	case "$head" in
-	refs/heads/bisect*)
+	refs/bisect/*)
 		git checkout master || exit
 		;;
 	refs/heads/*)
@@ -63,7 +78,6 @@ bisect_start() {
 	#
 	# Get rid of any old bisect state
 	#
-	rm -f "$GIT_DIR/refs/heads/bisect"
 	rm -rf "$GIT_DIR/refs/bisect/"
 	mkdir "$GIT_DIR/refs/bisect"
 	{
@@ -146,10 +160,12 @@ bisect_next() {
 	fi
 	nr=$(eval "git-rev-list $rev $good -- $(cat $GIT_DIR/BISECT_NAMES)" | wc -l) || exit
 	echo "Bisecting: $nr revisions left to test after this"
-	echo "$rev" > "$GIT_DIR/refs/heads/new-bisect"
-	git checkout new-bisect || exit
-	mv "$GIT_DIR/refs/heads/new-bisect" "$GIT_DIR/refs/heads/bisect" &&
-	GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD refs/heads/bisect
+	next="$(TMPDIR="$GIT_DIR/refs/heads" tempfile -p bisect)"
+	echo "$rev" > "$next"
+	# checkout refuses to deal with a head name not in refs/heads...
+	git checkout $(basename "$next")
+	mv "$next" "$GIT_DIR/refs/bisect/current" &&
+	GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD refs/bisect/current
 	git-show-branch "$rev"
 }
 
@@ -172,7 +188,6 @@ bisect_reset() {
 	esac
 	git checkout "$branch" &&
 	rm -fr "$GIT_DIR/refs/bisect"
-	rm -f "$GIT_DIR/refs/heads/bisect"
 	rm -f "$GIT_DIR/BISECT_LOG"
 }
 
@@ -217,6 +232,8 @@ case "$#" in
     case "$cmd" in
     start)
         bisect_start "$@" ;;
+    stop)
+        bisect_stop "$@" ;;
     bad)
         bisect_bad "$@" ;;
     good)

Re: Is reserving the branch name "bisect" a good thing?

[Junio C Hamano]

linux@horizon.com writes:

> Without changing this policy in git-checkout, or replicating most
> of git-checkout's code in git-bisect, I can't get away from using
> a head name in refs/heads/.  :-(

I tend to think the above "restriction" a conscious design decision.

> Actually, this leads to a question... suppose I want to manually
> check out some old revision (like "v.2.6.12") for some reason
> (performance testing, say).  How do I do that?
> Do I have to create a branch just to do that?

Absolutely, and that is deliberately so.  You do not want to:

	git checkout v2.6.12
        work work work
        git commit

and move tag v2.6.12 to something that is not v2.6.12.  As you
outlined, that "not moving the tag" check can be done in git
commit and have the user do:

	git checkout v2.6.12
        work work work
        git commit ;# fails
	git checkout -b oops-i-needed-a-branch-after-all
	git commit

but that feels ugly and wrong.

Branches are cheap (just a single file which is a pointer), so
if you prefer neatness:

	git checkout -b test-build v2.6.12
        work work work
        git checkout master ;# when you are done
        git branch -d test-build

Or if you are lazy like me, just keep a single throwaway branch
around, and whenever you feel like:

	git branch -f ta v2.6.12
        git checkout ta
        work work work
        git checkout master ;# when you are done

To test and possibly further develop the git-daemon update topic
branch, which is two revs before the tip of proposed updates
branch:

	git branch -f daemon-updates pu~2
        git checkout daemon-updates
        make
	work work work, find problems and do enhancements
        git commit

then you can say "Hey, Junio I found problem in your daemon
updates topic branch, and if you want my fixes you can pull from
my daemon-updates branch".

> It might be nicer to allow such a checkout and do the checking in
> git-commit, telling you to "git-checkout -b <new_branch>" before
> you check in your edits.

That may be, but it is too confusing.  Everybody needs to be
aware HEAD can point at random place, not necessarily on some
branch.  You happen to have noticed git-commit wants HEAD to be
pointing at a branch and no other random place, but are you
absolutely sure no other tools care?  I don't.

> +# Not generally needed, but provide a cleanup function
> +bisect_stop() {

Why isn't this part of bisect reset?