| Message ID | 48745298-f12b-8efb-4e48-90d2c22a8349@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | status: fix branch shown when not only bisecting | expand |
Rubén Justo <rjusto@gmail.com> writes: > In 83c750acde (wt-status.*: better advice for git status added, > 2012-06-05), git-status received new informative messages to describe > the ongoing work in a worktree. > > Multiple operations can be performed concurrently in a worktree. For > example, during a rebase, the user can also perform a cherry-pick. Hmph ... > In > that situation, git-status only shows information about one of them, > prioritizing which one, in order: merge, am, rebase, cherry-pick. ... I have to wonder if it is a bug that "cherry-pick" proceeds when there is an ongoing "rebase" going on, though. When a sequencer is driving a cherry-pick of master..topic1 and the user gets control back in the middle, perhaps due to a conflict, should the user be allowed to do "cherry-pick master..topic2", splicing these commits from the other topic in the middle of the first cherry-pick session the user started?
On 31-jul-2023 16:45:56, Junio C Hamano wrote: > Rubén Justo <rjusto@gmail.com> writes: > > > In 83c750acde (wt-status.*: better advice for git status added, > > 2012-06-05), git-status received new informative messages to describe > > the ongoing work in a worktree. > > > > Multiple operations can be performed concurrently in a worktree. For > > example, during a rebase, the user can also perform a cherry-pick. > > Hmph ... > > > In > > that situation, git-status only shows information about one of them, > > prioritizing which one, in order: merge, am, rebase, cherry-pick. > > ... I have to wonder if it is a bug that "cherry-pick" proceeds when > there is an ongoing "rebase" going on, though. Doing an interactive rebase, stop at some point, cherry-pick some commit and let the rebase continue, it's not optimal but does not seem to me to be bad workflow. > When a sequencer is > driving a cherry-pick of master..topic1 and the user gets control > back in the middle, perhaps due to a conflict, should the user be > allowed to do "cherry-pick master..topic2", splicing these commits > from the other topic in the middle of the first cherry-pick session > the user started? We already prevent this to happen. Maybe because we do not want to support multiple .git/CHERRY_PICK_HEAD files. Anyway, to me, sounds like a reasonable thing to have: that nesting limit of 1. The same for the other operations.
Rubén Justo <rjusto@gmail.com> writes: >> When a sequencer is >> driving a cherry-pick of master..topic1 and the user gets control >> back in the middle, perhaps due to a conflict, should the user be >> allowed to do "cherry-pick master..topic2", splicing these commits >> from the other topic in the middle of the first cherry-pick session >> the user started? > > We already prevent this to happen. Maybe because we do not want to > support multiple .git/CHERRY_PICK_HEAD files. Anyway, to me, sounds > like a reasonable thing to have: that nesting limit of 1. The same for > the other operations. OK, as long as we prevent such kinds of questionable combinations ("two multi-commit cherry-picks" was merely an example---I did not mean that is the only problematic case), I do not see much problem with it. In any case, teaching "status" how to show such a state with less information loss, which is the theme of this patch, is not making things worse---even if some of the states may be nonsense and should be prevented, "git status" is not the place to do so anyway. I didn't see if the proposed output from the command makes sense (yet), but somebody else may already have done so and writing their reviews on their findings. Let's see if we get any positive reviews and move it to 'next' after that. Will queue in the meantime not to lose it in 'seen'. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > I didn't see if the proposed output from the command makes sense > (yet), but somebody else may already have done so and writing their > reviews on their findings. Let's see if we get any positive reviews > and move it to 'next' after that. And unfortunately nothing has happened. Granted that summer in the northern hemisphere is a slow season, but it is a bit disappointing.
diff --git a/branch.c b/branch.c index cdf70b0ef0..81c03b81b7 100644 --- a/branch.c +++ b/branch.c @@ -420,9 +420,9 @@ static void prepare_checked_out_branches(void) wt_status_state_free_buffers(&state); if (wt_status_check_bisect(wt, &state) && - state.branch) { + state.bisecting_from) { struct strbuf ref = STRBUF_INIT; - strbuf_addf(&ref, "refs/heads/%s", state.branch); + strbuf_addf(&ref, "refs/heads/%s", state.bisecting_from); old = strmap_put(¤t_checked_out_branches, ref.buf, xstrdup(wt->path)); diff --git a/ref-filter.c b/ref-filter.c index f0a8e55270..526eebffad 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1894,7 +1894,7 @@ char *get_head_description(void) state.detached_from); } else if (state.bisect_in_progress) strbuf_addf(&desc, _("(no branch, bisect started on %s)"), - state.branch); + state.bisecting_from); else if (state.detached_from) { if (state.detached_at) strbuf_addf(&desc, _("(HEAD detached at %s)"), diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index c2ab8a444a..bbb8e9b8b0 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -692,6 +692,36 @@ EOF ' +test_expect_success 'status when bisecting while rebasing' ' + git reset --hard main && + FAKE_LINES="exec_exit_15" && + export FAKE_LINES && + test_when_finished "git rebase --abort" && + ONTO=$(git rev-parse --short HEAD^) && + test_must_fail git rebase -i HEAD^ && + git checkout -b bisect_while_rebasing && + test_when_finished "git bisect reset" && + git bisect start && + TGT=$(git rev-parse --short two_bisect) && + cat >expected <<EOF && +On branch bisect_while_rebasing +Last command done (1 command done): + exec exit 15 +No commands remaining. +You are currently editing a commit while rebasing branch '\''bisect'\'' on '\''$ONTO'\''. + (use "git commit --amend" to amend the current commit) + (use "git rebase --continue" once you are satisfied with your changes) + +You are currently bisecting, started from branch '\''bisect_while_rebasing'\''. + (use "git bisect reset" to get back to the original branch) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no >actual && + test_cmp expected actual +' + + test_expect_success 'status when rebase --apply conflicts with statushints disabled' ' git reset --hard main && git checkout -b statushints_disabled && diff --git a/worktree.c b/worktree.c index b8cf29e6a1..360e2b1866 100644 --- a/worktree.c +++ b/worktree.c @@ -395,9 +395,9 @@ int is_worktree_being_bisected(const struct worktree *wt, memset(&state, 0, sizeof(state)); found_bisect = wt_status_check_bisect(wt, &state) && - state.branch && + state.bisecting_from && skip_prefix(target, "refs/heads/", &target) && - !strcmp(state.branch, target); + !strcmp(state.bisecting_from, target); wt_status_state_free_buffers(&state); return found_bisect; } diff --git a/wt-status.c b/wt-status.c index 5b1378965c..0eb2af63b6 100644 --- a/wt-status.c +++ b/wt-status.c @@ -861,6 +861,7 @@ void wt_status_state_free_buffers(struct wt_status_state *state) FREE_AND_NULL(state->branch); FREE_AND_NULL(state->onto); FREE_AND_NULL(state->detached_from); + FREE_AND_NULL(state->bisecting_from); } static void wt_longstatus_print_unmerged(struct wt_status *s) @@ -1569,10 +1570,10 @@ static void show_revert_in_progress(struct wt_status *s, static void show_bisect_in_progress(struct wt_status *s, const char *color) { - if (s->state.branch) + if (s->state.bisecting_from) status_printf_ln(s, color, _("You are currently bisecting, started from branch '%s'."), - s->state.branch); + s->state.bisecting_from); else status_printf_ln(s, color, _("You are currently bisecting.")); @@ -1733,7 +1734,7 @@ int wt_status_check_bisect(const struct worktree *wt, if (!stat(worktree_git_path(wt, "BISECT_LOG"), &st)) { state->bisect_in_progress = 1; - state->branch = get_branch(wt, "BISECT_START"); + state->bisecting_from = get_branch(wt, "BISECT_START"); return 1; } return 0; diff --git a/wt-status.h b/wt-status.h index ab9cc9d8f0..819dcad723 100644 --- a/wt-status.h +++ b/wt-status.h @@ -94,6 +94,7 @@ struct wt_status_state { char *branch; char *onto; char *detached_from; + char *bisecting_from; struct object_id detached_oid; struct object_id revert_head_oid; struct object_id cherry_pick_head_oid;
In 83c750acde (wt-status.*: better advice for git status added, 2012-06-05), git-status received new informative messages to describe the ongoing work in a worktree. Multiple operations can be performed concurrently in a worktree. For example, during a rebase, the user can also perform a cherry-pick. In that situation, git-status only shows information about one of them, prioritizing which one, in order: merge, am, rebase, cherry-pick. However, when a bisect is also in progress, git-status includes, in addition to the description of any other ongoing operation, the description of the bisect. This means that, in this case, it shows description for two ongoing operations. In 0722c805d6 (status: show the branch name if possible in in-progress info, 2013-02-03), the messages introduced in 83c750acde were improved to show, if possible, the branch in which the described operation was initiated. Unfortunately, git-status only records one branch related to one operation. However, in the situation described above, when a bisect is combined with another operation, it is necessary to record two branches, which, it is important to note, may be different. Therefore, when that happens, we show incorrect information: $ git checkout -b bisect-branch $ git bisect start $ git checkout -b rebase-branch $ GIT_SEQUENCE_EDITOR='echo break >>' git rebase -i HEAD~ $ git status interactive rebase in progress ... ... You are currently editing a commit while rebasing branch 'bisect-branch' on '...'. You are currently bisecting, started from branch 'bisect-branch'. ... Days after 0722c805d6, this bisect circumstance hit us again; a leak was introduced in 8b87cfd000 (wt-status: move strbuf into read_and_strip_branch(), 2013-03-16). Let's fix git-status to display accurate information and also fix the leak, considering the branch where the bisect was started separately from the branch related to other ongoing operations. Signed-off-by: Rubén Justo <rjusto@gmail.com> --- branch.c | 4 ++-- ref-filter.c | 2 +- t/t7512-status-help.sh | 30 ++++++++++++++++++++++++++++++ worktree.c | 4 ++-- wt-status.c | 7 ++++--- wt-status.h | 1 + 6 files changed, 40 insertions(+), 8 deletions(-)