From: Thomas Gummerer <t.gummerer@gmail.com>
To: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
Cc: git@vger.kernel.org, newren@gmail.com, phillip.wood123@gmail.com,
martin.agren@gmail.com, jrnieder@gmail.com, gitster@pobox.com
Subject: Re: [GSoC][PATCH v4 0/4] [GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commits
Date: Mon, 17 Jun 2019 09:39:09 +0100 [thread overview]
Message-ID: <20190617083909.GB15217@hank.intra.tgummerer.com> (raw)
In-Reply-To: <20190616082040.9440-1-rohit.ashiwal265@gmail.com>
On 06/16, Rohit Ashiwal wrote:
> Yet another iteration of my patch. We have changed the series a little bit. We
> now have a commit that rename `reset_for_rollback` to `reset_merge`. A lot of
> nit-picks were handled in this revision.
Thanks for your work! I allowed myself to nitpick a bit more at this
stage :)
One other thing I wanted to point out here is range-diff, which can be
helpful to include for the benefit of reviewers that saw this series
before. See 'man git-range-diff' or the --range-diff flag in 'git
format-patch' for more info on how it works. It helps seeing at a
glance what changed between versions of a series.
For the benefit of other reviewers that might find it helpful, here's
one I generated between v3 and v4 of the series::
1: 8f29142755 ! 1: 99279e617c sequencer: add advice for revert
@@ -25,8 +25,8 @@
- error(_("a cherry-pick or revert is already in progress"));
- advise(_("try \"git cherry-pick (--continue | --quit | --abort)\""));
+ enum replay_action action;
-+ const char *in_progress_advice;
+ const char *in_progress_error = NULL;
++ const char *in_progress_advice = NULL;
+
+ if (!sequencer_get_last_command(r, &action)) {
+ switch (action) {
@@ -41,7 +41,7 @@
+ _("try \"git cherry-pick (--continue | --abort | --quit)\"");
+ break;
+ default:
-+ BUG(_("the control must not reach here"));
++ BUG(_("unexpected action in create_seq_dir"));
+ }
+ }
+ if (in_progress_error) {
-: ---------- > 2: c64aabf2d2 sequencer: rename reset_for_rollback to reset_merge
2: 3bc8678df4 ! 3: 8b483815ca cherry-pick/revert: add --skip option
@@ -27,7 +27,7 @@
-'git cherry-pick' --continue
-'git cherry-pick' --quit
-'git cherry-pick' --abort
-+'git cherry-pick' --continue | --skip | --abort | --quit
++'git cherry-pick' (--continue | --skip | --abort | --quit)
DESCRIPTION
-----------
@@ -42,7 +42,7 @@
-'git revert' --continue
-'git revert' --quit
-'git revert' --abort
-+'git revert' --continue | --skip | --abort | --quit
++'git revert' (--continue | --skip | --abort | --quit)
DESCRIPTION
-----------
@@ -97,10 +97,11 @@
+++ b/sequencer.c
@@
- static int reset_for_rollback(const struct object_id *oid)
+ static int reset_merge(const struct object_id *oid)
{
- const char *argv[4]; /* reset --merge <arg> + NULL */
-+ struct argv_array argv = ARGV_ARRAY_INIT; /* reset --merge <arg> + NULL */
++ int ret;
++ struct argv_array argv = ARGV_ARRAY_INIT;
- argv[0] = "reset";
- argv[1] = "--merge";
@@ -112,34 +113,29 @@
+ if (!is_null_oid(oid))
+ argv_array_push(&argv, oid_to_hex(oid));
+
-+ return run_command_v_opt(argv.argv, RUN_GIT_CMD);
++ ret = run_command_v_opt(argv.argv, RUN_GIT_CMD);
++ argv_array_clear(&argv);
++
++ return ret;
}
--static int rollback_single_pick(struct repository *r)
-+static int rollback_single_pick(struct repository *r, unsigned int is_skip)
- {
- struct object_id head_oid;
-
- if (!file_exists(git_path_cherry_pick_head(r)) &&
-- !file_exists(git_path_revert_head(r)))
-+ !file_exists(git_path_revert_head(r)) && !is_skip)
- return error(_("no cherry-pick or revert in progress"));
- if (read_ref_full("HEAD", 0, &head_oid, NULL))
- return error(_("cannot resolve HEAD"));
-- if (is_null_oid(&head_oid))
-+ if (is_null_oid(&head_oid) && !is_skip)
- return error(_("cannot abort from a branch yet to be born"));
- return reset_for_rollback(&head_oid);
- }
+ static int rollback_single_pick(struct repository *r)
@@
- * If CHERRY_PICK_HEAD or REVERT_HEAD indicates
- * a single-cherry-pick in progress, abort that.
- */
-- return rollback_single_pick(r);
-+ return rollback_single_pick(r, 0);
- }
- if (!f)
- return error_errno(_("cannot open '%s'"), git_path_head_file());
+ return reset_merge(&head_oid);
+ }
+
++static int skip_single_pick(void)
++{
++ struct object_id head;
++
++ if (read_ref_full("HEAD", 0, &head, NULL))
++ return error(_("cannot resolve HEAD"));
++ return reset_merge(&head);
++}
++
+ int sequencer_rollback(struct repository *r, struct replay_opts *opts)
+ {
+ FILE *f;
@@
return -1;
}
@@ -149,13 +145,35 @@
+ enum replay_action action = -1;
+ sequencer_get_last_command(r, &action);
+
++ /*
++ * opts->action tells us which subcommand requested to skip
++ * the commit.
++ */
+ switch (opts->action) {
+ case REPLAY_REVERT:
++ /*
++ * If .git/REVERT_HEAD exists then we are sure that we are in
++ * the middle of a revert and we allow to skip the commit.
++ */
+ if (!file_exists(git_path_revert_head(r))) {
++ /*
++ * Check if the last instruction executed was related to
++ * revert. If so, we are sure that a revert is in progress.
++ *
++ * NB: single commit revert is also counted in this
++ * definition of "progress" (and was dealt with in the
++ * previous check).
++ */
+ if (action == REPLAY_REVERT) {
++ /*
++ * Check if the user has moved the HEAD, i.e.,
++ * already committed. In this case, we would like
++ * to advise instead of skipping.
++ */
+ if (!rollback_is_safe())
+ goto give_advice;
+ else
++ /* skip commit :) */
+ break;
+ }
+ return error(_("no revert in progress"));
@@ -173,10 +191,10 @@
+ }
+ break;
+ default:
-+ BUG("the control must not reach here");
++ BUG("unexpected action in sequencer_skip");
+ }
+
-+ if (rollback_single_pick(r, 1))
++ if (skip_single_pick())
+ return error(_("failed to skip the commit"));
+ if (!is_directory(git_path_seq_dir()))
+ return 0;
@@ -289,7 +307,7 @@
+ git commit -a &&
+ test_path_is_missing .git/CHERRY_PICK_HEAD &&
+ test_must_fail git cherry-pick --skip 2>advice &&
-+ test_cmp expect advice
++ test_i18ncmp expect advice
+'
+
+test_expect_success 'allow skipping commit but not abort for a new history' '
@@ -303,7 +321,7 @@
+ test_must_fail git cherry-pick anotherpick &&
+ test_must_fail git cherry-pick --abort 2>advice &&
+ git cherry-pick --skip &&
-+ test_cmp expect advice
++ test_i18ncmp expect advice
+'
+
+test_expect_success 'allow skipping stopped cherry-pick because of untracked file modifications' '
3: a3cd17540b ! 4: 98618e08f4 cherry-pick/revert: advise using --skip
@@ -42,8 +42,8 @@
+++ b/sequencer.c
@@
enum replay_action action;
- const char *in_progress_advice;
const char *in_progress_error = NULL;
+ const char *in_progress_advice = NULL;
+ unsigned int advise_skip = file_exists(git_path_revert_head(r)) ||
+ file_exists(git_path_cherry_pick_head(r));
@@ -62,7 +62,7 @@
+ _("try \"git cherry-pick (--continue | %s--abort | --quit)\"");
break;
default:
- BUG(_("the control must not reach here"));
+ BUG(_("unexpected action in create_seq_dir"));
@@
}
if (in_progress_error) {
@@ -80,7 +80,7 @@
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@
- test_cmp expect advice
+ test_i18ncmp expect advice
'
+test_expect_success 'selectively advise --skip while launching another sequence' '
@@ -92,7 +92,7 @@
+ EOF
+ test_must_fail git cherry-pick picked..yetanotherpick &&
+ test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
-+ test_cmp expect advice &&
++ test_i18ncmp expect advice &&
+ cat >expect <<-EOF &&
+ error: cherry-pick is already in progress
+ hint: try "git cherry-pick (--continue | --abort | --quit)"
@@ -100,7 +100,7 @@
+ EOF
+ git reset --merge &&
+ test_must_fail git cherry-pick picked..yetanotherpick 2>advice &&
-+ test_cmp expect advice
++ test_i18ncmp expect advice
+'
+
test_expect_success 'allow skipping commit but not abort for a new history' '
> Rohit Ashiwal (4):
> sequencer: add advice for revert
> sequencer: rename reset_for_rollback to reset_merge
> cherry-pick/revert: add --skip option
> cherry-pick/revert: advise using --skip
>
> Documentation/git-cherry-pick.txt | 4 +-
> Documentation/git-revert.txt | 4 +-
> Documentation/sequencer.txt | 4 +
> builtin/commit.c | 13 +--
> builtin/revert.c | 5 ++
> sequencer.c | 139 ++++++++++++++++++++++++++----
> sequencer.h | 1 +
> t/t3510-cherry-pick-sequence.sh | 122 ++++++++++++++++++++++++++
> 8 files changed, 266 insertions(+), 26 deletions(-)
>
> --
> 2.21.0
>
next prev parent reply other threads:[~2019-06-17 8:39 UTC|newest]
Thread overview: 95+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-08 19:19 [GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commits Rohit Ashiwal
2019-06-08 19:19 ` [GSoC][PATCH 1/3] sequencer: add advice for revert Rohit Ashiwal
2019-06-09 17:52 ` Phillip Wood
2019-06-10 5:13 ` Rohit Ashiwal
2019-06-10 10:39 ` Phillip Wood
2019-06-10 13:25 ` Rohit Ashiwal
2019-06-10 17:46 ` Phillip Wood
2019-06-10 16:34 ` Junio C Hamano
2019-06-08 19:19 ` [GSoC][PATCH 2/3] cherry-pick/revert: add --skip option Rohit Ashiwal
2019-06-09 8:37 ` Thomas Gummerer
2019-06-09 18:01 ` Phillip Wood
2019-06-10 5:57 ` Rohit Ashiwal
2019-06-10 10:40 ` Phillip Wood
2019-06-10 13:43 ` Rohit Ashiwal
2019-06-10 17:47 ` Phillip Wood
2019-06-08 19:19 ` [GSoC][PATCH 3/3] cherry-pick/revert: update hints Rohit Ashiwal
2019-06-09 8:42 ` Thomas Gummerer
2019-06-09 18:03 ` Phillip Wood
2019-06-10 5:28 ` Rohit Ashiwal
2019-06-10 10:40 ` Phillip Wood
2019-06-10 13:33 ` Rohit Ashiwal
2019-06-10 17:47 ` Phillip Wood
2019-06-09 9:02 ` [GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commits Thomas Gummerer
2019-06-09 10:06 ` Rohit Ashiwal
2019-06-09 20:00 ` Thomas Gummerer
2019-06-11 7:31 ` [GSoC][PATCH v2 " Rohit Ashiwal
2019-06-11 7:31 ` [GSoC][PATCH v2 1/3] sequencer: add advice for revert Rohit Ashiwal
2019-06-11 21:25 ` Junio C Hamano
2019-06-11 7:31 ` [GSoC][PATCH v2 2/3] cherry-pick/revert: add --skip option Rohit Ashiwal
2019-06-12 13:31 ` Phillip Wood
2019-06-12 18:11 ` Junio C Hamano
2019-06-12 18:57 ` Phillip Wood
2019-06-11 7:31 ` [GSoC][PATCH v2 3/3] cherry-pick/revert: advise using --skip Rohit Ashiwal
2019-06-12 15:16 ` Phillip Wood
2019-06-13 4:05 ` [GSoC][PATCH v3 0/3] Teach cherry-pick/revert to skip commits Rohit Ashiwal
2019-06-13 4:05 ` [GSoC][PATCH v3 1/3] sequencer: add advice for revert Rohit Ashiwal
2019-06-13 17:45 ` Phillip Wood
2019-06-13 19:21 ` Martin Ågren
2019-06-13 20:59 ` Junio C Hamano
2019-06-14 3:44 ` Rohit Ashiwal
2019-06-14 3:43 ` Rohit Ashiwal
2019-06-13 4:05 ` [GSoC][PATCH v3 2/3] cherry-pick/revert: add --skip option Rohit Ashiwal
2019-06-13 17:56 ` Junio C Hamano
2019-06-13 19:57 ` Junio C Hamano
2019-06-14 3:48 ` Rohit Ashiwal
2019-06-14 3:45 ` Rohit Ashiwal
2019-06-14 15:58 ` Junio C Hamano
2019-06-16 7:03 ` Rohit Ashiwal
2019-06-13 17:59 ` Phillip Wood
2019-06-13 4:05 ` [GSoC][PATCH v3 3/3] cherry-pick/revert: advise using --skip Rohit Ashiwal
2019-06-16 8:20 ` [GSoC][PATCH v4 0/4] [GSoC][PATCH 0/3] Teach cherry-pick/revert to skip commits Rohit Ashiwal
2019-06-16 8:20 ` [GSoC][PATCH v4 1/4] sequencer: add advice for revert Rohit Ashiwal
2019-06-17 5:51 ` Thomas Gummerer
2019-06-16 8:20 ` [GSoC][PATCH v4 2/4] sequencer: rename reset_for_rollback to reset_merge Rohit Ashiwal
2019-06-16 8:20 ` [GSoC][PATCH v4 3/4] cherry-pick/revert: add --skip option Rohit Ashiwal
2019-06-17 8:30 ` Thomas Gummerer
2019-06-16 8:20 ` [GSoC][PATCH v4 4/4] cherry-pick/revert: advise using --skip Rohit Ashiwal
2019-06-17 8:39 ` Thomas Gummerer [this message]
2019-06-18 17:06 ` [GSoC][PATCH v5 0/5] Teach cherry-pick/revert to skip commits Rohit Ashiwal
2019-06-18 17:06 ` [GSoC][PATCH v5 1/5] sequencer: add advice for revert Rohit Ashiwal
2019-06-18 17:06 ` [GSoC][PATCH v5 2/5] sequencer: rename reset_for_rollback to reset_merge Rohit Ashiwal
2019-06-18 17:06 ` [GSoC][PATCH v5 3/5] sequencer: use argv_array in reset_merge Rohit Ashiwal
2019-06-18 17:06 ` [GSoC][PATCH v5 4/5] cherry-pick/revert: add --skip option Rohit Ashiwal
2019-06-20 3:40 ` Junio C Hamano
2019-06-20 9:46 ` Rohit Ashiwal
2019-06-20 9:57 ` Phillip Wood
2019-06-20 20:09 ` Junio C Hamano
2019-06-20 10:02 ` Phillip Wood
2019-06-20 10:34 ` Rohit Ashiwal
2019-06-20 11:42 ` Phillip Wood
2019-06-21 7:47 ` Rohit Ashiwal
2019-06-18 17:06 ` [GSoC][PATCH v5 5/5] cherry-pick/revert: advise using --skip Rohit Ashiwal
2019-06-21 9:17 ` [GSoC][PATCH v6 0/5] Teach cherry-pick/revert to skip commits Rohit Ashiwal
2019-06-21 9:17 ` [GSoC][PATCH v6 1/5] sequencer: add advice for revert Rohit Ashiwal
2019-06-21 9:17 ` [GSoC][PATCH v6 2/5] sequencer: rename reset_for_rollback to reset_merge Rohit Ashiwal
2019-06-21 9:17 ` [GSoC][PATCH v6 3/5] sequencer: use argv_array in reset_merge Rohit Ashiwal
2019-06-21 9:17 ` [GSoC][PATCH v6 4/5] cherry-pick/revert: add --skip option Rohit Ashiwal
2019-06-21 9:18 ` [GSoC][PATCH v6 5/5] cherry-pick/revert: advise using --skip Rohit Ashiwal
2019-06-21 19:09 ` [GSoC][PATCH v6 0/5] Teach cherry-pick/revert to skip commits Junio C Hamano
2019-06-23 20:03 ` [GSoC][PATCH v7 0/6] " Rohit Ashiwal
2019-06-23 20:03 ` [GSoC][PATCH v7 1/6] advice: add sequencerInUse config variable Rohit Ashiwal
2019-06-25 9:18 ` Thomas Gummerer
2019-06-23 20:03 ` [GSoC][PATCH v7 2/6] sequencer: add advice for revert Rohit Ashiwal
2019-06-29 14:05 ` Phillip Wood
2019-06-23 20:03 ` [GSoC][PATCH v7 3/6] sequencer: rename reset_for_rollback to reset_merge Rohit Ashiwal
2019-06-23 20:03 ` [GSoC][PATCH v7 4/6] sequencer: use argv_array in reset_merge Rohit Ashiwal
2019-06-23 20:03 ` [GSoC][PATCH v7 5/6] cherry-pick/revert: add --skip option Rohit Ashiwal
2019-06-23 20:03 ` [GSoC][PATCH v7 6/6] cherry-pick/revert: advise using --skip Rohit Ashiwal
2019-07-02 9:11 ` [GSoC][PATCH v8 0/5] Teach cherry-pick/revert to skip commits Rohit Ashiwal
2019-07-02 9:11 ` [GSoC][PATCH v8 1/5] sequencer: add advice for revert Rohit Ashiwal
2019-07-02 9:11 ` [GSoC][PATCH v8 2/5] sequencer: rename reset_for_rollback to reset_merge Rohit Ashiwal
2019-07-02 9:11 ` [GSoC][PATCH v8 3/5] sequencer: use argv_array in reset_merge Rohit Ashiwal
2019-07-02 9:11 ` [GSoC][PATCH v8 4/5] cherry-pick/revert: add --skip option Rohit Ashiwal
2019-07-02 9:11 ` [GSoC][PATCH v8 5/5] cherry-pick/revert: advise using --skip Rohit Ashiwal
2019-07-02 15:26 ` [GSoC][PATCH v8 0/5] Teach cherry-pick/revert to skip commits Phillip Wood
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190617083909.GB15217@hank.intra.tgummerer.com \
--to=t.gummerer@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=martin.agren@gmail.com \
--cc=newren@gmail.com \
--cc=phillip.wood123@gmail.com \
--cc=rohit.ashiwal265@gmail.com \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).