From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Git List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>,
Christian Couder <chriscool@tuxfamily.org>,
Daniel Barkalow <barkalow@iabervon.org>
Subject: Re: [PATCH 17/17] revert: Introduce --continue to continue the operation
Date: Sun, 17 Jul 2011 21:41:34 +0530 [thread overview]
Message-ID: <CALkWK0==A1a-kzqXuEkjS+FMR1CAzJox8MBdkQGMLukoU15L9w@mail.gmail.com> (raw)
In-Reply-To: <20110712204650.GG14909@elie>
Hi Jonathan,
Jonathan Nieder writes:
> Ramkumar Ramachandra wrote:
>> Using the information in ".git/sequencer", it is now possible to
>> continue a cherry-pick or continue after a conflict. To do this, we
>> have to parse the information in ".git/sequencer/opts" into the
>> replay_opts structure and
> [...]
>
> Might be simpler to say:
>
> Introduce a new "git cherry-pick --continue" command which uses
> the information in ".git/sequencer" to continue a cherry-pick that
> stopped because of a conflict or other error. It works by dropping
> the first instruction from .git/sequencer/todo and performing the
> remaining cherry-picks listed there, with options (think "-s" and
> "-X") from the initial command listed in .git/sequencer/opts.
>
> So now you can do:
>
> $ git cherry-pick -Xpatience foo..bar
> ... description conflict in commit moo ...
> $ git cherry-pick --continue
> error: 'cherry-pick' is not possible because you have unmerged files.
> fatal: failed to resume cherry-pick
> $ echo resolved >conflictingfile
> $ git add conflictingfile && git commit
> $ git cherry-pick --continue; # resumes with the commit after "moo"
Yep, thanks for the pretty commit message :)
>> During the "git commit" stage, CHERRY_PICK_HEAD will aid by providing
>> the commit message from the conflicting "moo" commit. Note that the
>> cherry-pick mechanism has no control at this stage, so the user is
>> free to violate anything that was specified during the first
>> cherry-pick invocation. For example, if "-x" was specified during the
>> first cherry-pick invocation, the user is free to edit out the message
>> during commit time. One glitch to note is that the "--signoff" option
>> specified at cherry-pick invocation time is not reflected in the
>> commit message provided by CHERRY_PICK_HEAD; the user must take care
>> to add "--signoff" during the "git commit" invocation.
>
> The -s thing doesn't have much to do with this change. But is it a
> bug or not? If it's not a bug, then this is not so much a glitch to
> note as an important feature to ensure people don't sign off on a
> conflict resolution without thinking about it. (I guess I think it's
> a bug. It's hard to decide.)
Bug, definitely. It happens because unlike "-x" where the
cherry-picking machinery appends to the commit message, "-s" is
handled at commit-time (when it spawns `git commit`). Ofcourse, if I
were never to write the sequencing features, this would never been
seen as a bug -- hence the term "glitch"; an implementation detail
that doesn't suit our future plans (namely, this series) very well.
>> --- a/t/t3510-cherry-pick-sequence.sh
>> +++ b/t/t3510-cherry-pick-sequence.sh
>> @@ -101,4 +101,97 @@ test_expect_success '--reset cleans up sequencer state' '
> [...]
>> +test_expect_success '--continue complains when no cherry-pick is in progress' '
>> + pristine_detach initial &&
>> + test_must_fail git cherry-pick --continue >actual 2>&1 &&
>> + test_i18ngrep "error" actual
>
> This would start to fail if the message ever changed from "error" to
> "fatal", right? I don't think that's a good thing.
Okay, removed this and all similar checks.
>> +test_expect_success '--continue complains when there are unresolved conflicts' '
>> + pristine_detach initial &&
>> + head=$(git rev-parse HEAD) &&
>> + test_must_fail git cherry-pick base..picked &&
>> + test_must_fail git cherry-pick --continue &&
>> + git cherry-pick --reset
>> +'
>> +
>> +test_expect_success '--continue continues after conflicts are resolved' '
>> + pristine_detach initial &&
>> + head=$(git rev-parse HEAD) &&
>
> How is $head used?
Stray line -- copy-pasting from an older version on my own tests.
Removed everywhere.
>> + test_must_fail git cherry-pick base..anotherpick &&
>> + echo "c" >foo &&
>> + git add foo &&
>> + git commit &&
>> + git cherry-pick --continue &&
>> + test_path_is_missing .git/sequencer &&
>> + {
>> + git rev-list HEAD |
>> + git diff-tree --root --stdin |
>> + sed "s/[0-9a-f]\{40\}/OBJID/g"
>> + } >actual &&
>
> $_x40 is idiomatic and safer with old seds.
I see. I actually thought '[0-9a-f]\{40\}' would be more traditional,
and that's why I picked in the first place :p
>> +test_expect_success '--continue respects opts' '
>> + pristine_detach initial &&
>> + head=$(git rev-parse HEAD) &&
>> + test_must_fail git cherry-pick -s -x base..anotherpick &&
>> + echo "c" >foo &&
>> + git add foo &&
>> + git commit -s &&
>> + git cherry-pick --continue &&
>> + test_path_is_missing .git/sequencer &&
>> + git cat-file commit HEAD >anotherpick_msg &&
>> + git cat-file commit HEAD~1 >picked_msg &&
>> + git cat-file commit HEAD~2 >unrelatedpick_msg &&
>> + git cat-file commit HEAD~3 >initial_msg &&
>> + test_must_fail test_i18ngrep "Signed-off-by:" initial_msg &&
>
> This will break when GETTEXT_POISON is set --- test_i18ngrep
> automatically succeeds in that case.
>
> Is "Signed-off-by" meant to be translated anyway? I would use
>
> ! grep
>
> if testing that.
My bad. Replaced with grep. Thanks.
> By the way, that probably should go in a separate test assertion
> ("-s is not automatically propagated to resolved conflict") to make
> it easier to change the behavior later.
Separated into another test. Thanks.
-- Ram
next prev parent reply other threads:[~2011-07-17 16:12 UTC|newest]
Thread overview: 99+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-11 14:53 [GSoC update] Sequencer for inclusion Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 01/17] advice: Introduce error_resolve_conflict Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 02/17] revert: Inline add_message_to_msg function Ramkumar Ramachandra
2011-07-12 16:53 ` Jonathan Nieder
2011-07-13 6:00 ` Ramkumar Ramachandra
2011-07-13 6:42 ` Jonathan Nieder
2011-07-19 16:36 ` Ramkumar Ramachandra
2011-07-19 16:52 ` Junio C Hamano
2011-07-19 17:08 ` Ramkumar Ramachandra
2011-07-19 19:36 ` Jonathan Nieder
2011-07-20 5:32 ` Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 03/17] revert: Don't check lone argument in get_encoding Ramkumar Ramachandra
2011-07-12 16:59 ` Jonathan Nieder
2011-07-13 6:14 ` Ramkumar Ramachandra
2011-07-13 6:30 ` Jonathan Nieder
2011-07-11 14:53 ` [PATCH 04/17] revert: Rename no_replay to record_origin Ramkumar Ramachandra
2011-07-12 17:02 ` Jonathan Nieder
2011-07-13 7:35 ` Ramkumar Ramachandra
2011-07-17 19:36 ` Jonathan Nieder
2011-07-11 14:53 ` [PATCH 05/17] revert: Propogate errors upwards from do_pick_commit Ramkumar Ramachandra
2011-07-12 17:32 ` Jonathan Nieder
2011-07-17 10:46 ` Ramkumar Ramachandra
2011-07-17 16:59 ` Jonathan Nieder
2011-07-19 10:09 ` Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 06/17] revert: Eliminate global "commit" variable Ramkumar Ramachandra
2011-07-12 17:45 ` Jonathan Nieder
2011-07-13 6:57 ` Ramkumar Ramachandra
2011-07-13 7:10 ` Jonathan Nieder
2011-07-13 8:33 ` Ramkumar Ramachandra
2011-07-13 16:40 ` Jonathan Nieder
2011-07-11 14:53 ` [PATCH 07/17] revert: Introduce struct to keep command-line options Ramkumar Ramachandra
2011-07-12 18:05 ` Jonathan Nieder
2011-07-13 7:56 ` Ramkumar Ramachandra
2011-07-11 14:53 ` [PATCH 08/17] revert: Separate cmdline parsing from functional code Ramkumar Ramachandra
2011-07-12 18:20 ` Jonathan Nieder
2011-07-18 20:53 ` Ramkumar Ramachandra
2011-07-18 21:03 ` Jonathan Nieder
2011-07-11 14:54 ` [PATCH 09/17] revert: Don't create invalid replay_opts in parse_args Ramkumar Ramachandra
2011-07-11 20:44 ` Junio C Hamano
2011-07-12 5:57 ` Ramkumar Ramachandra
2011-07-12 18:29 ` Jonathan Nieder
2011-07-17 11:56 ` Ramkumar Ramachandra
2011-07-17 18:43 ` Jonathan Nieder
2011-07-11 14:54 ` [PATCH 10/17] sequencer: Announce sequencer state location Ramkumar Ramachandra
2011-07-12 18:56 ` Jonathan Nieder
2011-07-13 12:10 ` Sverre Rabbelier
2011-07-17 16:23 ` Ramkumar Ramachandra
2011-07-17 19:19 ` Jonathan Nieder
2011-07-18 19:44 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 11/17] revert: Save data for continuing after conflict resolution Ramkumar Ramachandra
2011-07-11 21:01 ` Junio C Hamano
2011-07-11 21:31 ` Junio C Hamano
2011-07-12 5:43 ` Ramkumar Ramachandra
2011-07-12 19:37 ` Jonathan Nieder
2011-07-17 11:48 ` Ramkumar Ramachandra
2011-07-17 18:40 ` Jonathan Nieder
2011-07-18 19:31 ` Ramkumar Ramachandra
2011-07-19 12:21 ` Jonathan Nieder
2011-07-19 12:34 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 12/17] revert: Save command-line options for continuing operation Ramkumar Ramachandra
2011-07-11 21:15 ` Junio C Hamano
2011-07-12 5:56 ` Ramkumar Ramachandra
2011-07-12 19:52 ` Jonathan Nieder
2011-07-18 20:18 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 13/17] revert: Introduce a layer of indirection over pick_commits Ramkumar Ramachandra
2011-07-12 20:03 ` Jonathan Nieder
2011-07-18 21:24 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 14/17] reset: Make hard reset remove the sequencer state Ramkumar Ramachandra
2011-07-12 20:15 ` Jonathan Nieder
2011-07-17 16:40 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 15/17] revert: Remove sequencer state when no commits are pending Ramkumar Ramachandra
2011-07-11 19:58 ` Junio C Hamano
2011-07-12 6:26 ` Ramkumar Ramachandra
2011-07-11 14:54 ` [PATCH 16/17] revert: Introduce --reset to remove sequencer state Ramkumar Ramachandra
2011-07-12 20:30 ` Jonathan Nieder
2011-07-17 17:10 ` Ramkumar Ramachandra
2011-07-17 19:25 ` Jonathan Nieder
2011-07-11 14:54 ` [PATCH 17/17] revert: Introduce --continue to continue the operation Ramkumar Ramachandra
2011-07-12 20:46 ` Jonathan Nieder
2011-07-17 16:11 ` Ramkumar Ramachandra [this message]
2011-07-17 18:32 ` Jonathan Nieder
2011-07-18 20:00 ` Ramkumar Ramachandra
2011-07-18 20:09 ` Jonathan Nieder
2011-07-11 17:17 ` [GSoC update] Sequencer for inclusion Jonathan Nieder
2011-07-11 17:57 ` Ramkumar Ramachandra
2011-07-11 20:05 ` Ramkumar Ramachandra
2011-07-11 20:11 ` Jonathan Nieder
2011-07-12 7:05 ` Ramkumar Ramachandra
2011-07-11 20:07 ` Junio C Hamano
2011-07-11 22:14 ` Jeff King
2011-07-12 6:41 ` Ramkumar Ramachandra
2011-07-12 6:47 ` Jeff King
2011-07-13 9:41 ` Ramkumar Ramachandra
2011-07-13 19:07 ` Jeff King
2011-07-18 21:37 ` [RFC PATCH] config: Introduce functions to write non-standard file Ramkumar Ramachandra
2011-07-18 23:54 ` Junio C Hamano
2011-07-19 8:52 ` Ramkumar Ramachandra
2011-07-12 5:58 ` [GSoC update] Sequencer for inclusion Jonathan Nieder
2011-07-12 6:28 ` Miles Bader
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='CALkWK0==A1a-kzqXuEkjS+FMR1CAzJox8MBdkQGMLukoU15L9w@mail.gmail.com' \
--to=artagnon@gmail.com \
--cc=barkalow@iabervon.org \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@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).