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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html