If refs were ever packed in the middle of bisection, the bisect
refs were not removed from the "packed-refs" file.

This patch fixes this problem by using "git update-ref -d $ref $hash"
in "bisect_clean_state".

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-bisect.sh               |   11 ++++++++++-
 t/t6030-bisect-porcelain.sh |   12 ++++++++++++
 2 files changed, 22 insertions(+), 1 deletions(-)

	Ooops, there was a problem with the previous patch
	if "git bisect reset" was used when not bisecting.

	Sorry.

diff --git a/git-bisect.sh b/git-bisect.sh
index 1ed44e5..584906f 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -351,7 +351,16 @@ bisect_reset() {
 
 bisect_clean_state() {
 	rm -fr "$GIT_DIR/refs/bisect"
-	rm -f "$GIT_DIR/refs/heads/bisect"
+
+	# There may be some refs packed during bisection.
+	git for-each-ref --format='%(refname) %(objectname)' \
+		"refs/bisect/*" | while read ref hash
+	do
+		git update-ref -d $ref $hash
+	done
+
+	hash=$(git show-ref --hash refs/heads/bisect)
+	test -n "$hash" && git update-ref -d refs/heads/bisect $hash
 	rm -f "$GIT_DIR/BISECT_LOG"
 	rm -f "$GIT_DIR/BISECT_NAMES"
 	rm -f "$GIT_DIR/BISECT_RUN"
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 53956c0..f09db62 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -71,6 +71,18 @@ test_expect_success 'bisect start with one bad and good' '
 	git bisect next
 '
 
+test_expect_success 'bisect reset removes packed refs' '
+	git bisect reset &&
+	git bisect start &&
+	git bisect good $HASH1 &&
+	git bisect bad $HASH3 &&
+	git pack-refs --all --prune &&
+	git bisect next &&
+	git bisect reset &&
+	test -z "$(git for-each-ref "refs/bisect/*")" &&
+	test -z "$(git for-each-ref "refs/heads/bisect")"
+'
+
 # $HASH1 is good, $HASH4 is bad, we skip $HASH3
 # but $HASH2 is bad,
 # so we should find $HASH2 as the first bad commit
-- 
1.5.3.5.722.g789fd-dirty

-

[ message continues ]

Thanks.  Just a few nits.

On top of your patch...

 - You forgot to remove one "removal of filesystem refs";
 - for-each-ref takes more than one patterns.

 git-bisect.sh |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 584906f..21ed02f 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -350,17 +350,12 @@ bisect_reset() {
 }
 
 bisect_clean_state() {
-	rm -fr "$GIT_DIR/refs/bisect"
-
 	# There may be some refs packed during bisection.
-	git for-each-ref --format='%(refname) %(objectname)' \
-		"refs/bisect/*" | while read ref hash
+	git for-each-ref --format='%(refname) %(objectname)' refs/bisect/\* refs/heads/bisect |
+	while read ref hash
 	do
 		git update-ref -d $ref $hash
 	done
-
-	hash=$(git show-ref --hash refs/heads/bisect)
-	test -n "$hash" && git update-ref -d refs/heads/bisect $hash
 	rm -f "$GIT_DIR/BISECT_LOG"
 	rm -f "$GIT_DIR/BISECT_NAMES"
 	rm -f "$GIT_DIR/BISECT_RUN"
-

[ message continues ]

If you also swap %(refname) and %(objectname), then this is also not prone 
to whitespace in refnames. (Yes, I know, there shouldn't be such, but...)

-- Hannes

-

[ message continues ]

What's this obsession with whitespace in refnames?  Twice in like
two days people are talking about whitespace in refnames.

WHITESPACE IS NOT PERMITTED IN REFNAMES.

Do we need to apply the following patch, to keep people from creating
refs by hand with whitespace in them?  Is this really that common?

	git rev-parse HEAD >'.git/refs/heads/..i have spaces hah!'


diff --git a/refs.c b/refs.c
index aff02cd..b95bf83 100644
--- a/refs.c
+++ b/refs.c
@@ -246,6 +246,7 @@ static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)
 			struct stat st;
 			int flag;
 			int namelen;
+			int check;
 
 			if (de->d_name[0] == '.')
 				continue;
@@ -261,6 +262,9 @@ static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)
 				list = get_ref_dir(ref, list);
 				continue;
 			}
+			check = check_ref_format(ref);
+			if (check != 0 && check != -2)
+				continue;
 			if (!resolve_ref(ref, sha1, 1, &flag)) {
 				error("%s points nowhere!", ref);
 				continue;

-- 
Shawn.
-

[ message continues ]

I even had Junio convinced at one point!

I am not actually creating such refs, but I think my brain was still
fried from the URL encoding discussion, and I was overly paranoid about
spaces.

-Peff
-

[ message continues ]

Hi,


Yeah, I'm really sorry I started that.  It was meant to do good, but did 
bad.  Oh, well.  The road to hell is paved with good intentions.

Ciao,
Dscho

-

[ message continues ]

Ooops.  'test -d "$GIT_DIR/refs/bisect"' is used as a signal
that we are bisecting for the rest of the code, so we cannot
lose that rm -fr there.

I think a longer term clean-up would be not to treat "bisect" as
a reserved branch name but use detached HEAD while bisecting.
But that is a larger topic.

-

[ message continues ]

We do not need to pipe "echo" to "sed" only to strip refs/heads/
from the beginning.  We are assuming not-so-ancient shells these
days.

Also there is no need to avoid assuming \012 is the LF; we do
not run on EBCDIC, sorry.  Other parts of the script already
uses tr to convert separator to LF that way.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-bisect.sh |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 46a7b8d..3a21033 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -71,7 +71,7 @@ bisect_start() {
 		;;
 	refs/heads/*)
 		[ -s "$GIT_DIR/head-name" ] && die "won't bisect on seeked tree"
-		echo "$head" | sed 's#^refs/heads/##' >"$GIT_DIR/head-name"
+		echo "${head#refs/heads/}" >"$GIT_DIR/head-name"
 		;;
 	*)
 		die "Bad HEAD - strange symbolic ref"
@@ -275,8 +275,7 @@ exit_if_skipped_commits () {
 	if expr "$_tried" : ".*[|].*" > /dev/null ; then
 		echo "There are only 'skip'ped commit left to test."
 		echo "The first bad commit could be any of:"
-		echo "$_tried" | sed -e 's/[|]/\
-/g'
+		echo "$_tried" | tr '[|]' '[\012]'
 		echo "We cannot bisect more!"
 		exit 2
 	fi
-- 
1.5.3.5.1780.gca2b

-

[ message continues ]

What's wrong with sed?

-Miles

-- 
[|nurgle|]  ddt- demonic? so quake will have an evil kinda setting? one that
            will  make every christian in the world foamm at the mouth?
[iddt]      nurg, that's the goal
-

[ message continues ]

Nothing, but using it means forking a new process unnecessarily, and  
the shorter form without sed is arguably more readable:

-		echo "$head" | sed 's#^refs/heads/##' >"$GIT_DIR/head-name"
+		echo "${head#refs/heads/}" >"$GIT_DIR/head-name"

Cheers,
Wincent



-

[ message continues ]

Er, I suppose -- if you are acquainted with that particular shell
variable syntax (I suspect knowledge of sed is far more widespread).

[personally, I know that syntax has something to do with replacing
something with something else, but really haven't much clue other than
that, and I always _thought_ it was bash-specific and so avoided using
any of that stuff.]

-miles
-- 
People who are more than casually interested in computers should have at
least some idea of what the underlying hardware is like.  Otherwise the
programs they write will be pretty weird.  -- Donald Knuth
-

[ message continues ]

It says "remove refs/heads/ from the beginning of the string pointed to
by $head".

It's not a bashism. Some extensions to that syntax are though (I think).
If you want to be sure of portability, use sed instead. git uses this
syntax often enough that it's worth using everywhere, but usually only
in porcelain commands which one can relatively safely assume are run on
at least decently up-to-date developer workstations.

You'll note that stuff that absolutely *has* to reside server-side are
entirely in C.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
-

[ message continues ]

Huh?  It is used throughout.  That's why "make install" will install
xxx.sh scripts as xxx after possibly replacing the initial #!/bin/sh
line with a shell known to be reasonably conformant on a particular
system.

-- 
David Kastrup

-

[ message continues ]

This removes the last instance of making a ref by hand with
"echo SHA1 >.git/refs/$refname" from the script and replaces it
with update-ref.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-bisect.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 3a21033..4b74a7b 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -130,7 +130,7 @@ bisect_write() {
 		good|skip)	tag="$state"-"$rev" ;;
 		*)		die "Bad bisect_write argument: $state" ;;
 	esac
-	echo "$rev" >"$GIT_DIR/refs/bisect/$tag"
+	git update-ref "refs/bisect/$tag" "$rev"
 	echo "# $state: "$(git show-branch $rev) >>"$GIT_DIR/BISECT_LOG"
 	test -z "$nolog" && echo "git-bisect $state $rev" >>"$GIT_DIR/BISECT_LOG"
 }
-- 
1.5.3.5.1780.gca2b

-

[ message continues ]

When switching to a new rev, we first made "new-bisect" branch to
point at the chosen commit, attempt to switch to it, and then
finally renamed the new-bisect branch to bisect by hand when
successful.  This is so that we can catch checkout failure (your
local modification may interfere with switching to the chosen
version) without losing information on which commit the next
attempt should be made.

Rewrite it using a more modern form but without breaking the
safety.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-bisect.sh |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 4b74a7b..dae8a8e 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -316,10 +316,9 @@ bisect_next() {
 	exit_if_skipped_commits "$bisect_rev"
 
 	echo "Bisecting: $bisect_nr revisions left to test after this"
-	echo "$bisect_rev" >"$GIT_DIR/refs/heads/new-bisect"
+	git branch -f new-bisect "$bisect_rev"
 	git checkout -q 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
+	git branch -M new-bisect bisect
 	git show-branch "$bisect_rev"
 }
 
-- 
1.5.3.5.1780.gca2b

-

[ message continues ]