From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Phillip Wood <phillip.wood@dunelm.org.uk>
Cc: Alban Gruin <alban.gruin@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org
Subject: Re: [PATCH v3 0/2] rebase -i: extend rebase.missingCommitsCheck
Date: Fri, 6 Dec 2019 15:30:51 +0100 (CET) [thread overview]
Message-ID: <nycvar.QRO.7.76.6.1912061529270.31080@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <bc1fdd40-0490-eb65-247c-189b16865db5@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6607 bytes --]
Hi Alban,
On Fri, 6 Dec 2019, Phillip Wood wrote:
> On 05/12/2019 23:15, Alban Gruin wrote:
> >
> > Le 04/12/2019 à 22:51, Junio C Hamano a écrit :
> > > Alban Gruin <alban.gruin@gmail.com> writes:
> > >
> > > > To prevent mistakes when editing a branch, rebase features a knob,
> > > > rebase.missingCommitsCheck, to warn the user if a commit was dropped.
> > > > Unfortunately, this check is only effective for the initial edit, which
> > > > means that if you edit the todo list at a later point of the rebase and
> > > > drop a commit, no warnings or errors would be issued.
> > > > ...
> > > > rebase-interactive.c | 57 ++++++++++++++++++++----
> > > > rebase-interactive.h | 2 +
> > > > sequencer.c | 53 ++++++----------------
> > > > sequencer.h | 1 -
> > > > t/t3404-rebase-interactive.sh | 83 +++++++++++++++++++++++++++++++++++
> > > > 5 files changed, 147 insertions(+), 49 deletions(-)
> > >
> > > This passes the self-test when tested by itself, but when merged
> > > near the tip of 'pu', it breaks t3404.116, it seems.
> > >
> >
> > After a quick investigation, it comes from
> > pw/post-commit-from-sequencer. Since then, tests are expected to setup
> > the editor and run the commands using it in a subshell. So the fix is
> > straightforward.
> >
> > Perhaps I should take ag/sequencer-todo-updates, merge
> > pw/post-commit-from-sequencer, rebase this series onto the result, fix
> > the issue, and reroll the series?
>
> If the issue is just using a subshell to set the editor then (assuming you're
> only adding new tests) I don't think you need to rebase - just change your
> tests and it should be fine when Junio merges it into pu. I'm sorry I've not
> looked at the latest version yet, I'll try and get round to it next week.
Just squash this in:
-- snipsnap --
Subject: [PATCH] fixup??? rebase-interactive: warn if commit is dropped with
`rebase --edit-todo'
This is required to appease the interaction with
`pw/post-commit-from-sequencer`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t3404-rebase-interactive.sh | 74 +++++++++++++++++++----------------
1 file changed, 40 insertions(+), 34 deletions(-)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index e3f64bc2a59..72718d0d839 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1453,10 +1453,12 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '
test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = ignore' '
test_config rebase.missingCommitsCheck ignore &&
rebase_setup_and_clean missing-commit &&
- set_fake_editor &&
- FAKE_LINES="break 1 2 3 4 5" git rebase -i --root &&
- FAKE_LINES="1 2 3 4" git rebase --edit-todo 2>actual &&
- git rebase --continue 2>actual &&
+ (
+ set_fake_editor &&
+ FAKE_LINES="break 1 2 3 4 5" git rebase -i --root &&
+ FAKE_LINES="1 2 3 4" git rebase --edit-todo 2>actual &&
+ git rebase --continue 2>actual
+ ) &&
test D = $(git cat-file commit HEAD | sed -ne \$p) &&
test_i18ngrep \
"Successfully rebased and updated refs/heads/missing-commit" \
@@ -1474,18 +1476,20 @@ test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = wa
tail -n4 expect >expect.2 &&
test_config rebase.missingCommitsCheck warn &&
rebase_setup_and_clean missing-commit &&
- set_fake_editor &&
- test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \
- git rebase -i --root &&
- cp .git/rebase-merge/git-rebase-todo.backup orig &&
- FAKE_LINES="2 3 4" git rebase --edit-todo 2>actual.2 &&
- head -n5 actual.2 >actual &&
- test_i18ncmp expect actual &&
- cp orig .git/rebase-merge/git-rebase-todo &&
- FAKE_LINES="1 2 3 4" git rebase --edit-todo 2>actual.2 &&
- head -n4 actual.2 >actual &&
- test_i18ncmp expect.2 actual &&
- git rebase --continue 2>actual &&
+ (
+ set_fake_editor &&
+ test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \
+ git rebase -i --root &&
+ cp .git/rebase-merge/git-rebase-todo.backup orig &&
+ FAKE_LINES="2 3 4" git rebase --edit-todo 2>actual.2 &&
+ head -n5 actual.2 >actual &&
+ test_i18ncmp expect actual &&
+ cp orig .git/rebase-merge/git-rebase-todo &&
+ FAKE_LINES="1 2 3 4" git rebase --edit-todo 2>actual.2 &&
+ head -n4 actual.2 >actual &&
+ test_i18ncmp expect.2 actual &&
+ git rebase --continue 2>actual
+ ) &&
test D = $(git cat-file commit HEAD | sed -ne \$p) &&
test_i18ngrep \
"Successfully rebased and updated refs/heads/missing-commit" \
@@ -1509,24 +1513,26 @@ test_expect_success 'rebase --edit-todo respects rebase.missingCommitsCheck = er
tail -n10 expect >expect.2 &&
test_config rebase.missingCommitsCheck error &&
rebase_setup_and_clean missing-commit &&
- set_fake_editor &&
- test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \
- git rebase -i --root &&
- cp .git/rebase-merge/git-rebase-todo.backup orig &&
- test_must_fail env FAKE_LINES="2 3 4" \
- git rebase --edit-todo 2>actual &&
- test_i18ncmp expect actual &&
- test_must_fail git rebase --continue 2>actual &&
- test_i18ncmp expect actual &&
- cp orig .git/rebase-merge/git-rebase-todo &&
- test_must_fail env FAKE_LINES="1 2 3 4" \
- git rebase --edit-todo 2>actual &&
- test_i18ncmp expect.2 actual &&
- test_must_fail git rebase --continue 2>actual &&
- test_i18ncmp expect.2 actual &&
- cp orig .git/rebase-merge/git-rebase-todo &&
- FAKE_LINES="1 2 3 4 drop 5" git rebase --edit-todo &&
- git rebase --continue 2>actual &&
+ (
+ set_fake_editor &&
+ test_must_fail env FAKE_LINES="bad 1 2 3 4 5" \
+ git rebase -i --root &&
+ cp .git/rebase-merge/git-rebase-todo.backup orig &&
+ test_must_fail env FAKE_LINES="2 3 4" \
+ git rebase --edit-todo 2>actual &&
+ test_i18ncmp expect actual &&
+ test_must_fail git rebase --continue 2>actual &&
+ test_i18ncmp expect actual &&
+ cp orig .git/rebase-merge/git-rebase-todo &&
+ test_must_fail env FAKE_LINES="1 2 3 4" \
+ git rebase --edit-todo 2>actual &&
+ test_i18ncmp expect.2 actual &&
+ test_must_fail git rebase --continue 2>actual &&
+ test_i18ncmp expect.2 actual &&
+ cp orig .git/rebase-merge/git-rebase-todo &&
+ FAKE_LINES="1 2 3 4 drop 5" git rebase --edit-todo &&
+ git rebase --continue 2>actual
+ ) &&
test D = $(git cat-file commit HEAD | sed -ne \$p) &&
test_i18ngrep \
"Successfully rebased and updated refs/heads/missing-commit" \
--
2.24.0.windows.2.611.ge9aced84530
next prev parent reply other threads:[~2019-12-06 14:31 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-17 14:39 [RFC PATCH 0/9] rebase -i: extend rebase.missingCommitsCheck to `--edit-todo' and co Alban Gruin
2019-07-17 14:39 ` [RFC PATCH 1/9] t3404: demonstrate that --edit-todo does not check for dropped commits Alban Gruin
2019-07-18 18:31 ` Junio C Hamano
2019-07-19 18:12 ` Alban Gruin
2019-07-19 19:51 ` Junio C Hamano
2019-07-17 14:39 ` [RFC PATCH 2/9] t3429: demonstrate that rebase exec " Alban Gruin
2019-07-17 14:39 ` [RFC PATCH 3/9] sequencer: update `total_nr' when adding an item to a todo list Alban Gruin
2019-07-18 19:52 ` Junio C Hamano
2019-07-19 18:12 ` Alban Gruin
2019-07-17 14:39 ` [RFC PATCH 4/9] sequencer: update `done_nr' when skipping commands in " Alban Gruin
2019-07-18 19:55 ` Junio C Hamano
2019-07-19 18:13 ` Alban Gruin
2019-07-17 14:39 ` [RFC PATCH 5/9] sequencer: move the code writing total_nr on the disk to a new function Alban Gruin
2019-07-18 20:04 ` Junio C Hamano
2019-07-19 18:14 ` Alban Gruin
2019-07-17 14:39 ` [RFC PATCH 6/9] sequencer: add a parameter to sequencer_continue() to accept a todo list Alban Gruin
2019-07-17 14:39 ` [RFC PATCH 7/9] rebase-interactive: todo_list_check() also uses the done list Alban Gruin
2019-07-17 14:39 ` [RFC PATCH 8/9] rebase-interactive: warn if commit is dropped with --edit-todo Alban Gruin
2019-07-17 14:39 ` [RFC PATCH 9/9] sequencer: have read_populate_todo() check for dropped commits Alban Gruin
2019-07-24 13:29 ` [RFC PATCH 0/9] rebase -i: extend rebase.missingCommitsCheck to `--edit-todo' and co Phillip Wood
2019-07-25 9:01 ` Johannes Schindelin
2019-07-25 20:26 ` Alban Gruin
2019-07-29 9:38 ` Phillip Wood
2019-09-24 20:15 ` Alban Gruin
2019-11-04 9:54 ` [RFC PATCH v2 0/2] rebase -i: extend rebase.missingCommitsCheck to `--edit-todo' Alban Gruin
2019-11-04 9:54 ` [RFC PATCH v2 1/2] t3404: demonstrate that --edit-todo does not check for dropped commits Alban Gruin
2019-11-04 9:54 ` [RFC PATCH v2 2/2] rebase-interactive: warn if commit is dropped with --edit-todo Alban Gruin
2019-11-05 14:20 ` Phillip Wood
2019-12-02 23:47 ` [PATCH v3 0/2] rebase -i: extend rebase.missingCommitsCheck Alban Gruin
2019-12-02 23:47 ` [PATCH v3 1/2] sequencer: move check_todo_list_from_file() to rebase-interactive.c Alban Gruin
2019-12-06 14:38 ` Johannes Schindelin
2019-12-02 23:47 ` [PATCH v3 2/2] rebase-interactive: warn if commit is dropped with `rebase --edit-todo' Alban Gruin
2019-12-04 19:19 ` Junio C Hamano
2019-12-09 16:00 ` Phillip Wood
2020-01-09 21:13 ` Alban Gruin
2020-01-10 17:13 ` Phillip Wood
2020-01-10 21:31 ` Alban Gruin
2020-01-11 14:44 ` Phillip Wood
2019-12-09 16:08 ` Phillip Wood
2019-12-04 21:51 ` [PATCH v3 0/2] rebase -i: extend rebase.missingCommitsCheck Junio C Hamano
2019-12-05 23:15 ` Alban Gruin
2019-12-06 10:41 ` Phillip Wood
2019-12-06 14:30 ` Johannes Schindelin [this message]
2020-01-11 17:39 ` [PATCH v4 " Alban Gruin
2020-01-11 17:39 ` [PATCH v4 1/2] sequencer: move check_todo_list_from_file() to rebase-interactive.c Alban Gruin
2020-01-11 17:39 ` [PATCH v4 2/2] rebase-interactive: warn if commit is dropped with `rebase --edit-todo' Alban Gruin
2020-01-19 16:28 ` Phillip Wood
2020-01-25 15:17 ` Alban Gruin
2020-01-25 17:54 ` [PATCH v5 0/2] rebase -i: extend rebase.missingCommitsCheck Alban Gruin
2020-01-25 17:54 ` [PATCH v5 1/2] sequencer: move check_todo_list_from_file() to rebase-interactive.c Alban Gruin
2020-01-25 17:54 ` [PATCH v5 2/2] rebase-interactive: warn if commit is dropped with `rebase --edit-todo' Alban Gruin
2020-01-26 10:04 ` [PATCH v5 0/2] rebase -i: extend rebase.missingCommitsCheck Johannes Schindelin
2020-01-27 21:39 ` Alban Gruin
2020-01-28 22:46 ` Johannes Schindelin
2020-01-28 21:12 ` [PATCH v6 " Alban Gruin
2020-01-28 21:12 ` [PATCH v6 1/2] sequencer: move check_todo_list_from_file() to rebase-interactive.c Alban Gruin
2020-01-28 21:12 ` [PATCH v6 2/2] rebase-interactive: warn if commit is dropped with `rebase --edit-todo' Alban Gruin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=nycvar.QRO.7.76.6.1912061529270.31080@tvgsbejvaqbjf.bet \
--to=johannes.schindelin@gmx.de \
--cc=alban.gruin@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.