# [PATCH 2/3] bisect: improve error msg of 'bisect reset' when original HEAD is deleted 8 messages from 2010-10-10 to 2010-10-14. Participants: SZEDER Gábor, Ævar Arnfjörð Bjarmason, Christian Couder, Junio C Hamano. Thread: https://gitlist.dev/t/25415 ## SZEDER Gábor, 2010-10-10 21:48 Subject: [PATCH 1/3] bisect: improve error message of 'bisect log' while not bisecting Message-ID: <1286747338-8521-1-git-send-email-szeder@ira.uka.de> URL: https://gitlist.dev/e/1286747338-8521-1-git-send-email-szeder%40ira.uka.de ``` '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 --- git-bisect.sh | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) 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, 2010-10-10 21:48 Subject: [PATCH 2/3] bisect: improve error msg of 'bisect reset' when original HEAD is deleted Message-ID: <1286747338-8521-2-git-send-email-szeder@ira.uka.de> URL: https://gitlist.dev/e/1286747338-8521-2-git-send-email-szeder%40ira.uka.de In-Reply-To: <1286747338-8521-1-git-send-email-szeder@ira.uka.de> ``` 'git bisect reset' (without the optional 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 '. Signed-off-by: SZEDER Gábor --- git-bisect.sh | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) 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 '." + fi } bisect_clean_state() { -- 1.7.3.1.148.g2fffa ``` ## SZEDER Gábor, 2010-10-10 21:48 Subject: [PATCH 3/3] bisect: check for mandatory argument of 'bisect replay' Message-ID: <1286747338-8521-3-git-send-email-szeder@ira.uka.de> URL: https://gitlist.dev/e/1286747338-8521-3-git-send-email-szeder%40ira.uka.de In-Reply-To: <1286747338-8521-1-git-send-email-szeder@ira.uka.de> ``` '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 --- 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 -- 1.7.3.1.148.g2fffa ``` ## Ævar Arnfjörð Bjarmason, 2010-10-11 06:44 Subject: Re: [PATCH 1/3] bisect: improve error message of 'bisect log' while not bisecting Message-ID: URL: https://gitlist.dev/e/AANLkTi%3DKZyhJrYw%2BL2U6gsu%3DjNcBe8eRJTG4jSuPTYsT%40mail.gmail.com In-Reply-To: <1286747338-8521-1-git-send-email-szeder@ira.uka.de> ``` 2010/10/10 SZEDER Gábor : > '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...)) $ ``` ## Christian Couder, 2010-10-12 02:35 Subject: Re: [PATCH 3/3] bisect: check for mandatory argument of 'bisect replay' Message-ID: <201010120435.11903.chriscool@tuxfamily.org> URL: https://gitlist.dev/e/201010120435.11903.chriscool%40tuxfamily.org In-Reply-To: <1286747338-8521-3-git-send-email-szeder@ira.uka.de> ``` 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 > --- > 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, 2010-10-12 02:56 Subject: Re: [PATCH 3/3] bisect: check for mandatory argument of 'bisect replay' Message-ID: <201010120456.35411.chriscool@tuxfamily.org> URL: https://gitlist.dev/e/201010120456.35411.chriscool%40tuxfamily.org In-Reply-To: <201010120435.11903.chriscool@tuxfamily.org> ``` On Tuesday 12 October 2010 04:35:11 Christian Couder wrote: > 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 > > --- > > > > 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, 2010-10-13 17:58 Subject: Re: [PATCH 3/3] bisect: check for mandatory argument of 'bisect replay' Message-ID: <7viq16m0tk.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7viq16m0tk.fsf%40alter.siamese.dyndns.org In-Reply-To: <201010120456.35411.chriscool@tuxfamily.org> ``` Christian Couder 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? ``` ## Christian Couder, 2010-10-14 07:55 Subject: Re: [PATCH 3/3] bisect: check for mandatory argument of 'bisect replay' Message-ID: URL: https://gitlist.dev/e/AANLkTi%3DHCHy0q4atzVYUGO0hQ3FBigZcFDsFLv9iZ%3Dh-%40mail.gmail.com In-Reply-To: <7viq16m0tk.fsf@alter.siamese.dyndns.org> ``` On Wed, Oct 13, 2010 at 7:58 PM, Junio C Hamano wrote: > Christian Couder 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. ```