Re: [PATCH] bisect: use "$GIT_DIR/BISECT_START" to check if we are bisecting

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] bisect: use "$GIT_DIR/BISECT_START" to check if we are bisecting

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:39

Christian Couder [off-list ref] writes:
It seems simpler and safer to use the BISECT_START file everywhere
to decide if we are bisecting or not, instead of using it in some
places and BISECT_NAMES in other places.

In commit 6459c7c6786aa9bda0c7a095c9db66c36da0e5f0 (Nov 18 2007,
Bisect: use "$GIT_DIR/BISECT_NAMES" to check if we are bisecting.),
we decided to use BISECT_NAMES but code changed a lot and we now
have to check BISECT_START first in the "bisect_start" function
anyway.

This patch also makes things a little bit safer by creating
the BISECT_START file first and deleting it last, and also by
adding checks in "bisect_clean_state".

Signed-off-by: Christian Couder <redacted>
What's the breakage scenario that this patch fixes?
quoted hunk
diff --git a/git-bisect.sh b/git-bisect.sh
index 4bcbace..991b2ef 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -44,7 +44,7 @@ sq() {
 }
 
 bisect_autostart() {
-	test -f "$GIT_DIR/BISECT_NAMES" || {
+	test -s "$GIT_DIR/BISECT_START" || {
The reason you ignore an existing but empty BISECT_START file is...?

Re: [PATCH] bisect: use "$GIT_DIR/BISECT_START" to check if we are bisecting

From: Christian Couder <hidden>
Date: 2016-06-15 22:44:39

Le mercredi 28 mai 2008, Junio C Hamano a écrit :
Christian Couder [off-list ref] writes:
quoted
It seems simpler and safer to use the BISECT_START file everywhere
to decide if we are bisecting or not, instead of using it in some
places and BISECT_NAMES in other places.

In commit 6459c7c6786aa9bda0c7a095c9db66c36da0e5f0 (Nov 18 2007,
Bisect: use "$GIT_DIR/BISECT_NAMES" to check if we are bisecting.),
we decided to use BISECT_NAMES but code changed a lot and we now
have to check BISECT_START first in the "bisect_start" function
anyway.

This patch also makes things a little bit safer by creating
the BISECT_START file first and deleting it last, and also by
adding checks in "bisect_clean_state".

Signed-off-by: Christian Couder <redacted>
What's the breakage scenario that this patch fixes?
Before this patch, in "bisect_clean_state" we removed the BISECT_START file 
before the other files, so for example if the process is killed after 
having removed this file but not the others, then we are in an inconsistent 
state.

In this inconsistent state, if "git bisect reset" is called (perhaps again), 
then it would checkout the "master" branch (because the BISECT_START file 
does not exists, but the BISECT_NAMES file still exists).
quoted
diff --git a/git-bisect.sh b/git-bisect.sh
index 4bcbace..991b2ef 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -44,7 +44,7 @@ sq() {
 }

 bisect_autostart() {
-	test -f "$GIT_DIR/BISECT_NAMES" || {
+	test -s "$GIT_DIR/BISECT_START" || {
The reason you ignore an existing but empty BISECT_START file is...?
... that it should not happen, because this file is only written 
in "bisect_start" and there its content comes either from the current HEAD 
or from a previous not empty BISECT_START file.

We might add a check for an empty BISECT_START file and warn in this case 
that the file may have been corrupted, but that may be for another patch.

Thanks,
Christian.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help