threads / patch / 25415

patch, 3 partsbisect: improve error msg of 'bisect reset' when original HEAD is deleted

Subject: [PATCH 2/3] bisect: improve error msg of 'bisect reset' when original HEAD is deleted

## tl;dr

8 messages between Oct 10, 2010 and Oct 14, 2010. Diffs are folded; open one to read it.

replies: 7people: 4as markdown or json

SZEDER Gábor· Oct 10, 2010, 21:48 UTC · lore

[PATCH 1/3] bisect: improve error message of 'bisect log' while not bisecting

'git bisect log' is implemented by a direct invocation of 'cat "$GIT_DIR/BISECT_LOG"', without any sanity checks. Consequently, running 'git bisect log' while not bisecting leads to an error, because the bisect logfile doesn't exists. The accompanying error message

  cat: /path/to/repo/.git/BISECT_LOG: No such file or directory
is neither very helpful nor very friendly.

Instead of blindly trying to cat the log file, first check whether there is a bisection going on (i.e. the bisect logfile exists), and die with a more appropriate error message when not.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 git-bisect.sh |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
Show changes to git-bisect.sh +5 −1
diff --git a/git-bisect.sh b/git-bisect.sh
index 6e2acb8..3a4bf81 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -412,6 +412,10 @@ bisect_run () {
     done
 }
 
+bisect_log () {
+	test -s "$GIT_DIR/BISECT_LOG" || die "We are not bisecting."
+	cat "$GIT_DIR/BISECT_LOG"
+}
 
 case "$#" in
 0)
@@ -438,7 +442,7 @@ case "$#" in
     replay)
 	bisect_replay "$@" ;;
     log)
-	cat "$GIT_DIR/BISECT_LOG" ;;
+	bisect_log ;;
     run)
         bisect_run "$@" ;;
     *)

-- 
1.7.3.1.148.g2fffa
SZEDER Gábor· Oct 10, 2010, 21:48 UTC · re: SZEDER Gábor · lore

'git bisect reset' (without the optional <commit> argument) returns to the original HEAD from where the bisection was started. However, when, for whatever reason, the user deleted the original HEAD before invoking 'git bisect reset', then all he gets is an error message from 'git checkout':

  fatal: invalid reference: somebranch

Let's try to be more helpful with an error message better describing what went wrong and a suggestion about how to resolve the situation:

  Could not check out original HEAD 'somebranch'. Try 'git bisect reset <commit>'.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 git-bisect.sh |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
Show changes to git-bisect.sh +6 −1
diff --git a/git-bisect.sh b/git-bisect.sh
index 3a4bf81..68fcff6 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -316,7 +316,12 @@ bisect_reset() {
 	*)
 	    usage ;;
 	esac
-	git checkout "$branch" -- && bisect_clean_state
+	if git checkout "$branch" -- ; then
+		bisect_clean_state
+	else
+		die "Could not check out original HEAD '$branch'." \
+				"Try 'git bisect reset <commit>'."
+	fi
 }
 
 bisect_clean_state() {

-- 
1.7.3.1.148.g2fffa
SZEDER Gábor· Oct 10, 2010, 21:48 UTC · re: SZEDER Gábor · lore

[PATCH 3/3] bisect: check for mandatory argument of 'bisect replay'

'git bisect replay' has a mandatory logfile argument, but the current implementation doesn't check whether the user has specified one. When the user omits the logfile argument, this leads to the following unhelpful error message:

  cannot read  for replaying

So, check for the mandatory argument first, and provide a more meaningful error message when it is omitted.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 git-bisect.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to git-bisect.sh +1 −0
diff --git a/git-bisect.sh b/git-bisect.sh
index 68fcff6..c21e33c 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -343,6 +343,7 @@ bisect_clean_state() {
 }
 
 bisect_replay () {
+	test "$#" -eq 1 || die "No logfile given"
 	test -r "$1" || die "cannot read $1 for replaying"
 	bisect_reset
 	while read git bisect command rev

-- 
1.7.3.1.148.g2fffa
Christian Couder· Oct 12, 2010, 02:35 UTC · re: SZEDER Gábor · lore

Re: [PATCH 3/3] bisect: check for mandatory argument of 'bisect replay'

On Sunday 10 October 2010 23:48:58 SZEDER Gábor wrote:
Show 27 quoted lines
> 'git bisect replay' has a mandatory logfile argument, but the current
> implementation doesn't check whether the user has specified one.  When
> the user omits the logfile argument, this leads to the following
> unhelpful error message:
> 
>   cannot read  for replaying
> 
> So, check for the mandatory argument first, and provide a more
> meaningful error message when it is omitted.
> 
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
>  git-bisect.sh |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/git-bisect.sh b/git-bisect.sh
> index 68fcff6..c21e33c 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -343,6 +343,7 @@ bisect_clean_state() {
>  }
> 
>  bisect_replay () {
> +	test "$#" -eq 1 || die "No logfile given"
>  	test -r "$1" || die "cannot read $1 for replaying"
>  	bisect_reset
>  	while read git bisect command rev
While at it perhaps you could do something like:
bisect_replay () {
+	test "$#" -lt 1 || die "No logfile given"
+	test "$#" -gt 1 || die "Too many argument. Please give only one logfile."
 	test -r "$1" || die "cannot read $1 for replaying"
 	bisect_reset
 	while read git bisect command rev

Thanks, Christian.

Christian Couder· Oct 12, 2010, 02:56 UTC · re: Christian Couder · lore

Re: [PATCH 3/3] bisect: check for mandatory argument of 'bisect replay'

On Tuesday 12 October 2010 04:35:11 Christian Couder wrote:
Show 41 quoted lines
> On Sunday 10 October 2010 23:48:58 SZEDER Gábor wrote:
> > 'git bisect replay' has a mandatory logfile argument, but the current
> > implementation doesn't check whether the user has specified one.  When
> > the user omits the logfile argument, this leads to the following
> > 
> > unhelpful error message:
> >   cannot read  for replaying
> > 
> > So, check for the mandatory argument first, and provide a more
> > meaningful error message when it is omitted.
> > 
> > Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> > ---
> > 
> >  git-bisect.sh |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/git-bisect.sh b/git-bisect.sh
> > index 68fcff6..c21e33c 100755
> > --- a/git-bisect.sh
> > +++ b/git-bisect.sh
> > @@ -343,6 +343,7 @@ bisect_clean_state() {
> > 
> >  }
> >  
> >  bisect_replay () {
> > 
> > +	test "$#" -eq 1 || die "No logfile given"
> > 
> >  	test -r "$1" || die "cannot read $1 for replaying"
> >  	bisect_reset
> >  	while read git bisect command rev
> 
> While at it perhaps you could do something like:
> 
> bisect_replay () {
> +	test "$#" -lt 1 || die "No logfile given"
> +	test "$#" -gt 1 || die "Too many argument. Please give only one logfile."
>  	test -r "$1" || die "cannot read $1 for replaying"
>  	bisect_reset
>  	while read git bisect command rev
I mean:
bisect_replay () {
+	test "$#" -lt 1 && die "No logfile given"
+	test "$#" -gt 1 && die "Too many argument. Please give only one logfile."
 	test -r "$1" || die "cannot read $1 for replaying"
 	bisect_reset
 	while read git bisect command rev
Sorry!

Best regards, Christian.

Junio C Hamano· Oct 13, 2010, 17:58 UTC · re: Christian Couder · lore

Re: [PATCH 3/3] bisect: check for mandatory argument of 'bisect replay'

Christian Couder <chriscool@tuxfamily.org> writes:
Show 25 quoted lines
>> >  bisect_replay () {
>> > 
>> > +	test "$#" -eq 1 || die "No logfile given"
>> > 
>> >  	test -r "$1" || die "cannot read $1 for replaying"
>> >  	bisect_reset
>> >  	while read git bisect command rev
>> 
>> While at it perhaps you could do something like:
>> 
>> bisect_replay () {
>> +	test "$#" -lt 1 || die "No logfile given"
>> +	test "$#" -gt 1 || die "Too many argument. Please give only one logfile."
>>  	test -r "$1" || die "cannot read $1 for replaying"
>>  	bisect_reset
>>  	while read git bisect command rev
>
> I mean:
>
> bisect_replay () {
> +	test "$#" -lt 1 && die "No logfile given"
> +	test "$#" -gt 1 && die "Too many argument. Please give only one logfile."
>  	test -r "$1" || die "cannot read $1 for replaying"
>  	bisect_reset
>  	while read git bisect command rev

That suggests that the original patch is buggy as well, in that it says "No logfile given" when it gets two.

How about checking with 1 and saying "Give me only one logfile" and be done with it?

Christian Couder· Oct 14, 2010, 07:55 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/3] bisect: check for mandatory argument of 'bisect replay'

On Wed, Oct 13, 2010 at 7:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 33 quoted lines
> Christian Couder <chriscool@tuxfamily.org> writes:
>
>>> >  bisect_replay () {
>>> >
>>> > +  test "$#" -eq 1 || die "No logfile given"
>>> >
>>> >    test -r "$1" || die "cannot read $1 for replaying"
>>> >    bisect_reset
>>> >    while read git bisect command rev
>>>
>>> While at it perhaps you could do something like:
>>>
>>> bisect_replay () {
>>> +    test "$#" -lt 1 || die "No logfile given"
>>> +    test "$#" -gt 1 || die "Too many argument. Please give only one logfile."
>>>      test -r "$1" || die "cannot read $1 for replaying"
>>>      bisect_reset
>>>      while read git bisect command rev
>>
>> I mean:
>>
>> bisect_replay () {
>> +     test "$#" -lt 1 && die "No logfile given"
>> +     test "$#" -gt 1 && die "Too many argument. Please give only one logfile."
>>       test -r "$1" || die "cannot read $1 for replaying"
>>       bisect_reset
>>       while read git bisect command rev
>
> That suggests that the original patch is buggy as well, in that it says
> "No logfile given" when it gets two.
>
> How about checking with 1 and saying "Give me only one logfile" and be
> done with it?
Yeah or: "Give me exactly one logfile as argument"

Best regards, Christian.

Ævar Arnfjörð Bjarmason· Oct 11, 2010, 06:44 UTC · re: SZEDER Gábor · lore

Re: [PATCH 1/3] bisect: improve error message of 'bisect log' while not bisecting

2010/10/10 SZEDER Gábor <szeder@ira.uka.de>:
Show 5 quoted lines
> 'git bisect log' is implemented by a direct invocation of 'cat
> "$GIT_DIR/BISECT_LOG"', without any sanity checks.  Consequently,
> running 'git bisect log' while not bisecting leads to an error,
> because the bisect logfile doesn't exists.  The accompanying error
> message
Nice, in case you want to fix this for git-rebase too:
Applying: gettextize: git-add "no files added" message
Applying: gettextize: git-add "Use -f if you really want" message
Applying: gettextize: git-add "pathspec [...] did not match" message
Applying: gettextize: git-add "remove '%s'" message
^C
v t ((5cab536...)|REBASE) $ git rebase --abort
cat: /home/avar/g/git/.git/rebase-apply/head-name: No such file or directory
cat: /home/avar/g/git/.git/rebase-apply/orig-head: No such file or directory
HEAD is now at 5cab536 gettextize: git-add "remove '%s'" message
v t ((5cab536...)) $
v t ((5cab536...)) $ git rebase --abort
No rebase in progress?
v t ((5cab536...)) $

← back to recent threads