Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix git-bisect when show-branch is configured to run with pager #1003

Closed
wants to merge 1 commit into from

Conversation

oded-ist
Copy link

@oded-ist oded-ist commented Jul 26, 2021

Signed-off-by: Oded Shimon oded@istraresearch.com

Thanks for taking the time to contribute to Git! Please be advised that the
Git community does not use github.com for their contributions. Instead, we use
a mailing list (git@vger.kernel.org) for code submissions, code reviews, and
bug reports. Nevertheless, you can use GitGitGadget (https://gitgitgadget.github.io/)
to conveniently send your Pull Requests commits to our mailing list.

Please read the "guidelines for contributing" linked above!

cc: Christian Couder christian.couder@gmail.com

cc: Christian Couder christian.couder@gmail.com

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 26, 2021

Welcome to GitGitGadget

Hi @oded-ist, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that your Pull Request has a good description, as it will be used as cover letter.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

  • the lines should not exceed 76 columns,
  • the first line should be like a header and typically start with a prefix like "tests:" or "revisions:" to state which subsystem the change is about, and
  • the commit messages' body should be describing the "why?" of the change.
  • Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form /allow. A good way to find other contributors is to locate recent pull requests where someone has been /allowed:

Both the person who commented /allow and the PR author are able to /allow you.

An alternative is the channel #git-devel on the Libera Chat IRC network:

<newcontributor> I've just created my first PR, could someone please /allow me? https://github.com/gitgitgadget/git/pull/12345
<veteran> newcontributor: it is done
<newcontributor> thanks!

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

If you want to see what email(s) would be sent for a /submit request, add a PR comment /preview to have the email(s) sent to you. You must have a public GitHub email address for this.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail).

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the (raw) link), then import it into your mail program. If you use GMail, you can do this via:

curl -g --user "<EMailAddress>:<Password>" \
    --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

To iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description):

Changes since v1:
- Fixed a typo in the commit message (found by ...)
- Added a code comment to ... as suggested by ...
...

To send a new iteration, just add another PR comment with the contents: /submit.

Need help?

New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join.

You may also be able to find help in real time in the developer IRC channel, #git-devel on Libera Chat. Remember that IRC does not support offline messaging, so if you send someone a private message and log out, they cannot respond to you. The scrollback of #git-devel is archived, though.

@dscho
Copy link
Member

dscho commented Jul 26, 2021

/allow

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 26, 2021

User oded-ist is now allowed to use GitGitGadget.

WARNING: oded-ist has no public email address set on GitHub

@oded-ist
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 26, 2021

Error: Could not determine full name of oded-ist

@oded-ist
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 26, 2021

Submitted as pull.1003.git.1627311659384.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git pr-1003/oded-ist/master-v1

To fetch this version to local tag pr-1003/oded-ist/master-v1:

git fetch --no-tags https://github.com/gitgitgadget/git tag pr-1003/oded-ist/master-v1

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 26, 2021

On the Git mailing list, Christian Couder wrote (reply to this):

(Sorry, I forgot to keep the mailing list in Cc, in my first reply, so
I am resending it...)

On Mon, Jul 26, 2021 at 5:03 PM Oded S via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Oded Shimon <oded@istraresearch.com>

Could you explain a bit more here what is the unwanted behavior this
patch fixes?

> Signed-off-by: Oded Shimon <oded@istraresearch.com>
> ---
>     Fix git-bisect when show-branch is configured to run with pager

Usually subjects are something like:

bisect: make sure show-branch doesn't use a pager

Otherwise your patch looks good to me!

Thanks!

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 26, 2021

User Christian Couder <christian.couder@gmail.com> has been added to the cc: list.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 26, 2021

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Oded S via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Subject: Re: [PATCH] Fix git-bisect when show-branch is configured to run with pager

Perhaps

    Subject: bisect: disable pager while invoking show-branch

cf. Documentation/SubmittingPatches (describe-changes)

> From: Oded Shimon <oded@istraresearch.com>

Here is the space for you to answer these potential questions by
future readers of your code in "git log -p" output in advance:

 * The title says "fix", but how is it broken?  If the user prefers
   to run show-branch with their pager, why would it be a good
   change to unilaterally countermand that preference?

 * When is show-branch invoked in the "git bisect" session?  Perhaps
   that justifies the unilateral disabling of the pager, but it is
   not explained here so we cannot tell why the author of this
   change thought it was a good idea.

 * We see in the patch context that "checkout" is also invoked
   somewhere in the same program, but it does not gain the
   "--no-pager" option.  Why?  If those who prefer show-branch to
   page have trouble using "bisect", wouldn't those who prefer
   "checkout" to page have the same trouble?

> Signed-off-by: Oded Shimon <oded@istraresearch.com>

Thanks for trying to make Git better.  I cannot quite tell without
these questions (and there may be others) answered in the proposed
log message if the proposed change is a good one.

Also, in the longer term, I suspect that we probably should stop
calling show-branch from this codepath and here is why.

If we look at "git show v1.5.3:git-bisect.sh" and look for
invocation of show-branch, and look for show-branch in the current
codebase wrt bisect, i.e.

    $ git grep show-branch bisect.c git-bisect.sh builtin/bisect--helper.c

we notice that we used to call the command in many more places to
write into BISECT_LOG and also after checking out the commit to be
tested.  In today's code, the latter is the only place that still
uses show-branch.  What happend to the use of the other one?  IOW,
how do we write BISECT_LOG these days in such a way that it is
compatible/comparable to the output we got from show-branch in olden
times?  Would it easy to emulate it so that we do not use show-branch
at all?  If so, reusing how the part that writes BISECT_LOG does to
show the revision after checking it out may be a good clean-up,
regardless of "show-branch pages" issue.

In any case, if the "for those with show-branch configured to page,
the current behaviour of bisect needs fixing" is true, then I think
your patch may even be worth applying before such a longer-term
change.

Thanks.


>     Fix git-bisect when show-branch is configured to run with pager
>     
>     Signed-off-by: Oded Shimon oded@istraresearch.com
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1003%2Foded-ist%2Fmaster-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1003/oded-ist/master-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1003
>
>  bisect.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/bisect.c b/bisect.c
> index af2863d044b..c02bcc3359f 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -23,7 +23,7 @@ static struct oid_array skipped_revs;
>  static struct object_id *current_bad_oid;
>  
>  static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
> -static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
> +static const char *argv_show_branch[] = {"-P", "show-branch", NULL, NULL};
>  
>  static const char *term_bad;
>  static const char *term_good;
> @@ -748,7 +748,7 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
>  			return -abs(res);
>  	}
>  
> -	argv_show_branch[1] = bisect_rev_hex;
> +	argv_show_branch[2] = bisect_rev_hex;
>  	res = run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
>  	/*
>  	 * Errors in `run_command()` itself, signaled by res < 0,
>
> base-commit: eb27b338a3e71c7c4079fbac8aeae3f8fbb5c687

git-bisect uses show-branch for logging during the bisect process. If the user
sets an interactive pager for show-branch, this makes bisect hang (wait for
user input) unexpectedly - so we disable pager with -P.

It's possible that the user would set a pager for git-checkout as well, but an
interactive pager there would break many more scripts.

Signed-off-by: Oded Shimon <oded@istraresearch.com>
@oded-ist
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 27, 2021

Submitted as pull.1003.v2.git.1627373560881.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git pr-1003/oded-ist/master-v2

To fetch this version to local tag pr-1003/oded-ist/master-v2:

git fetch --no-tags https://github.com/gitgitgadget/git tag pr-1003/oded-ist/master-v2

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 27, 2021

On the Git mailing list, Junio C Hamano wrote (reply to this):

Junio C Hamano <gitster@pobox.com> writes:

> Also, in the longer term, I suspect that we probably should stop
> calling show-branch from this codepath and here is why.

I wonder if it is just a simple matter of a few lines of code, like
this?

---- >8 ------- >8 ------- >8 ------- >8 ------- >8 ----
Subject: [PATCH] bisect: do not run show-branch just to show the current  commit

In scripted versions of "git bisect", we used "git show-branch" to
describe single commit in the bisect log and also to the interactive
user after checking out the next version to be tested.  

The former use of "git show-branch" was lost when the helper
function that wrote bisect log entries was rewritten at 0f30233a
(bisect--helper: `bisect_write` shell function in C, 2019-01-02) in
C

But we've kept the latter ever since 0871984d (bisect: make "git
bisect" use new "--next-all" bisect-helper function, 2009-05-09)
started using the faithful C-rewrite introduced at ef24c7ca
(bisect--helper: add "--next-exit" to output bisect results,
2009-04-19).

Showing "[<full hex>] <subject>" is simple enough with our helper
pretty.c::format_commit_message() and spawning show-branch is an
overkill.  Let's lose one external process.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 bisect.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/bisect.c b/bisect.c
index af2863d044..2b8b6546e9 100644
--- a/bisect.c
+++ b/bisect.c
@@ -23,7 +23,6 @@ static struct oid_array skipped_revs;
 static struct object_id *current_bad_oid;
 
 static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
-static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
 
 static const char *term_bad;
 static const char *term_good;
@@ -729,6 +728,9 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
 {
 	char bisect_rev_hex[GIT_MAX_HEXSZ + 1];
 	enum bisect_error res = BISECT_OK;
+	struct commit *commit;
+	struct pretty_print_context pp = {0};
+	struct strbuf commit_msg = STRBUF_INIT;
 
 	oid_to_hex_r(bisect_rev_hex, bisect_rev);
 	update_ref(NULL, "BISECT_EXPECTED_REV", bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
@@ -748,13 +750,11 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
 			return -abs(res);
 	}
 
-	argv_show_branch[1] = bisect_rev_hex;
-	res = run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
-	/*
-	 * Errors in `run_command()` itself, signaled by res < 0,
-	 * and errors in the child process, signaled by res > 0
-	 * can both be treated as regular BISECT_FAILURE (-1).
-	 */
+	commit = lookup_commit_reference(the_repository, bisect_rev);
+	format_commit_message(commit, "[%H] %s%n", &commit_msg, &pp);
+	fputs(commit_msg.buf, stdout);
+	strbuf_release(&commit_msg);
+
 	return -abs(res);
 }
 
-- 
2.32.0-555-g350b5add0b

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 27, 2021

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Oded S via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Oded Shimon <oded@istraresearch.com>
>
> git-bisect uses show-branch for logging during the bisect process. If the user
> sets an interactive pager for show-branch, this makes bisect hang (wait for
> user input) unexpectedly - so we disable pager with -P.
>
> It's possible that the user would set a pager for git-checkout as well, but an
> interactive pager there would break many more scripts.
>
> Signed-off-by: Oded Shimon <oded@istraresearch.com>
> ---

Nicely described.  Now we can discuss if the thought process behind
this change makes sense or not with such a clear description.

I do not know if "unexpectedly" is truly unexpected for those who
configure show-branch to page, though.  After all they wanted their
pager to kick in.

In any case, such users are probably better off configuring their
pager not to prompt and wait when the output is less than pageful
(e.g. "less" has "--quit-if-one-screen" option and 'F' in $LESS
environemnt variable triggers this behaviour).

The patch looks good to me.

Thanks.



> diff --git a/bisect.c b/bisect.c
> index af2863d044b..c02bcc3359f 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -23,7 +23,7 @@ static struct oid_array skipped_revs;
>  static struct object_id *current_bad_oid;
>  
>  static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
> -static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
> +static const char *argv_show_branch[] = {"-P", "show-branch", NULL, NULL};
>  
>  static const char *term_bad;
>  static const char *term_good;
> @@ -748,7 +748,7 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
>  			return -abs(res);
>  	}
>  
> -	argv_show_branch[1] = bisect_rev_hex;
> +	argv_show_branch[2] = bisect_rev_hex;
>  	res = run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
>  	/*
>  	 * Errors in `run_command()` itself, signaled by res < 0,
>
> base-commit: eb27b338a3e71c7c4079fbac8aeae3f8fbb5c687

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 27, 2021

This branch is now known as os/bisect-runs-show-branch-without-pager.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 27, 2021

This patch series was integrated into seen via git@5315a5f.

@gitgitgadget gitgitgadget bot added the seen label Jul 27, 2021
@gitgitgadget
Copy link

gitgitgadget bot commented Jul 28, 2021

On the Git mailing list, Christian Couder wrote (reply to this):

On Tue, Jul 27, 2021 at 8:26 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Also, in the longer term, I suspect that we probably should stop
> > calling show-branch from this codepath and here is why.
>
> I wonder if it is just a simple matter of a few lines of code, like
> this?

Yeah, I also think it's a good idea.

> ---- >8 ------- >8 ------- >8 ------- >8 ------- >8 ----
> Subject: [PATCH] bisect: do not run show-branch just to show the current  commit
>
> In scripted versions of "git bisect", we used "git show-branch" to
> describe single commit in the bisect log and also to the interactive

s/single/ a single/

> user after checking out the next version to be tested.

[...]

> diff --git a/bisect.c b/bisect.c
> index af2863d044..2b8b6546e9 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -23,7 +23,6 @@ static struct oid_array skipped_revs;
>  static struct object_id *current_bad_oid;
>
>  static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
> -static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
>
>  static const char *term_bad;
>  static const char *term_good;
> @@ -729,6 +728,9 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
>  {
>         char bisect_rev_hex[GIT_MAX_HEXSZ + 1];
>         enum bisect_error res = BISECT_OK;
> +       struct commit *commit;
> +       struct pretty_print_context pp = {0};
> +       struct strbuf commit_msg = STRBUF_INIT;
>
>         oid_to_hex_r(bisect_rev_hex, bisect_rev);
>         update_ref(NULL, "BISECT_EXPECTED_REV", bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
> @@ -748,13 +750,11 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
>                         return -abs(res);
>         }
>
> -       argv_show_branch[1] = bisect_rev_hex;
> -       res = run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
> -       /*
> -        * Errors in `run_command()` itself, signaled by res < 0,
> -        * and errors in the child process, signaled by res > 0
> -        * can both be treated as regular BISECT_FAILURE (-1).
> -        */
> +       commit = lookup_commit_reference(the_repository, bisect_rev);
> +       format_commit_message(commit, "[%H] %s%n", &commit_msg, &pp);
> +       fputs(commit_msg.buf, stdout);
> +       strbuf_release(&commit_msg);
> +
>         return -abs(res);

Nice! Now, the above line can be simplified to:

         return BISECT_OK;

And the declaration of the `res` variable can be moved into the else
clause where it is used.
>  }

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 28, 2021

User Christian Couder <christian.couder@gmail.com> has been added to the cc: list.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 28, 2021

On the Git mailing list, Junio C Hamano wrote (reply to this):

Christian Couder <christian.couder@gmail.com> writes:

>> I wonder if it is just a simple matter of a few lines of code, like
>> this?
>
> Yeah, I also think it's a good idea.
>
>> ---- >8 ------- >8 ------- >8 ------- >8 ------- >8 ----
>> Subject: [PATCH] bisect: do not run show-branch just to show the current  commit
>>
>> In scripted versions of "git bisect", we used "git show-branch" to
>> describe single commit in the bisect log and also to the interactive
>
> s/single/ a single/

Thanks.

>>         enum bisect_error res = BISECT_OK;
>> +       struct commit *commit;
>> +       struct pretty_print_context pp = {0};
>> +       struct strbuf commit_msg = STRBUF_INIT;
>> ...
>> +       commit = lookup_commit_reference(the_repository, bisect_rev);
>> +       format_commit_message(commit, "[%H] %s%n", &commit_msg, &pp);
>> +       fputs(commit_msg.buf, stdout);
>> +       strbuf_release(&commit_msg);
>> +
>>         return -abs(res);
>
> Nice! Now, the above line can be simplified to:
>
>          return BISECT_OK;
>
> And the declaration of the `res` variable can be moved into the else
> clause where it is used.

Thanks, again.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 28, 2021

On the Git mailing list, Junio C Hamano wrote (reply to this):

I'll fix the missing 'a' in the log message, but the "res"
simplification is probably better done as a separate patch on top.

How does this look?

Thanks.

---- >8 ------- >8 ------- >8 ------- >8 ------- >8 ------- >8 ----
Subject: [PATCH] bisect: simplify return code from bisect_checkout()

The function was designed to return only BISECT_OK (0) or
BISECT_FAILED (-1) and no other values, but there were two issues:

 - The comment misspelled BISECT_FAILED as BISECT_FAILURE, even
   though the logic it described (i.e. any non-zero return should be
   reported as a single BISECT_FAILED) was correct.

 - It took the return value from run_command_v_opt(), and assumed it
   was either -1 or 1 upon error, which is not the case; it can relay
   errors from wait_or_whine(), which can report exit status of the
   child process.

Translate any error return from run_command_v_opt() to BISECT_FAILED,
and simplify the resulting code by losing the 'res' variable that is
no longer needed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 bisect.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/bisect.c b/bisect.c
index 2b8b6546e9..888949fba6 100644
--- a/bisect.c
+++ b/bisect.c
@@ -727,7 +727,6 @@ static int is_expected_rev(const struct object_id *oid)
 static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int no_checkout)
 {
 	char bisect_rev_hex[GIT_MAX_HEXSZ + 1];
-	enum bisect_error res = BISECT_OK;
 	struct commit *commit;
 	struct pretty_print_context pp = {0};
 	struct strbuf commit_msg = STRBUF_INIT;
@@ -740,14 +739,13 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
 		update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0,
 			   UPDATE_REFS_DIE_ON_ERR);
 	} else {
-		res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
-		if (res)
+		if (run_command_v_opt(argv_checkout, RUN_GIT_CMD))
 			/*
 			 * Errors in `run_command()` itself, signaled by res < 0,
 			 * and errors in the child process, signaled by res > 0
-			 * can both be treated as regular BISECT_FAILURE (-1).
+			 * can both be treated as regular BISECT_FAILED (-1).
 			 */
-			return -abs(res);
+			return BISECT_FAILED;
 	}
 
 	commit = lookup_commit_reference(the_repository, bisect_rev);
@@ -755,7 +753,7 @@ static enum bisect_error bisect_checkout(const struct object_id *bisect_rev, int
 	fputs(commit_msg.buf, stdout);
 	strbuf_release(&commit_msg);
 
-	return -abs(res);
+	return BISECT_OK;
 }
 
 static struct commit *get_commit_reference(struct repository *r,
-- 
2.32.0-561-g6177dfa0d2

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 28, 2021

This patch series was integrated into seen via git@b189b04.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 29, 2021

There was a status update in the "New Topics" section about the branch os/bisect-runs-show-branch-without-pager on the Git mailing list:

When "git bisect" spawns "git show-branch" to pretty-print the
title of the commit to be tested, it could have invoked user's
pager if user configured to run pager while running show-branch.
The invocation of show-branch has been changed to disable pager,
even if one is configured.

Will discard.
This probably is unnecessary with the rewrite to get rid of the
show-branch invocation altogether.

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 29, 2021

On the Git mailing list, Christian Couder wrote (reply to this):

On Wed, Jul 28, 2021 at 7:07 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> I'll fix the missing 'a' in the log message, but the "res"
> simplification is probably better done as a separate patch on top.

Ok.

> How does this look?
>
> ---- >8 ------- >8 ------- >8 ------- >8 ------- >8 ------- >8 ----
> Subject: [PATCH] bisect: simplify return code from bisect_checkout()

[...]

This LGTM, thanks!

@gitgitgadget
Copy link

gitgitgadget bot commented Jul 30, 2021

This patch series was integrated into seen via git@11c75b8.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 2, 2021

There was a status update in the "Cooking" section about the branch os/bisect-runs-show-branch-without-pager on the Git mailing list:

When "git bisect" spawns "git show-branch" to pretty-print the
title of the commit to be tested, it could have invoked user's
pager if user configured to run pager while running show-branch.
The invocation of show-branch has been changed to disable pager,
even if one is configured.

Will discard.
This probably is unnecessary with the rewrite to get rid of the
show-branch invocation altogether.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 2, 2021

This patch series was integrated into seen via git@0a69d39.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 4, 2021

This patch series was integrated into seen via git@81e1205.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 4, 2021

This patch series was integrated into seen via git@77ded63.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 4, 2021

This patch series was integrated into seen via git@06c1fed.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 4, 2021

There was a status update in the "Discarded" section about the branch os/bisect-runs-show-branch-without-pager on the Git mailing list:

When "git bisect" spawns "git show-branch" to pretty-print the
title of the commit to be tested, it could have invoked user's
pager if user configured to run pager while running show-branch.
The invocation of show-branch has been changed to disable pager,
even if one is configured.

Will discard.
This probably is unnecessary with the rewrite to get rid of the
show-branch invocation altogether.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 4, 2021

This patch series was integrated into seen via git@23fb45c.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 4, 2021

This patch series was integrated into seen via git@8f9bc8f.

@dscho
Copy link
Member

dscho commented Aug 5, 2021

There was a status update in the "New Topics" section about the branch os/bisect-runs-show-branch-without-pager on the Git mailing list:

When "git bisect" spawns "git show-branch" to pretty-print the
title of the commit to be tested, it could have invoked user's
pager if user configured to run pager while running show-branch.
The invocation of show-branch has been changed to disable pager,
even if one is configured.

Will discard.
This probably is unnecessary with the rewrite to get rid of the
show-branch invocation altogether.

@oded-ist so I guess this PR can be closed?

@oded-ist oded-ist closed this Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants