Junio C Hamano venit, vidit, dixit 06.06.2016 22:06: > Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes: > >> Currently, cherry-pick allows tp pick single commits to an empty HEAD >> but not multiple commits. >> >> Allow the multiple commit case, too. >> >> Reported-by: Fabrizio Cucci <fabrizio.cucci@xxxxxxxxx> >> Signed-off-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> >> --- >> sequencer.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) > > Thanks; we'd probably want a few tests in somewhere near 3503 or > 3505. I'll try my best. >> diff --git a/sequencer.c b/sequencer.c >> index 4687ad4..c6362d6 100644 >> --- a/sequencer.c >> +++ b/sequencer.c >> @@ -888,6 +888,10 @@ static int sequencer_rollback(struct replay_opts *opts) >> git_path_head_file()); >> goto fail; >> } >> + if (is_null_sha1(sha1)) { >> + error(_("cannot abort from a branch yet to be born")); >> + goto fail; >> + } > > Is this error-fail desirable? How do we get here? Did a user start > a cherry-pick on an unborn branch and then told us to abort that? > Shouldn't we be taking the user back to the "orphan" state if that > is the case? Here and below, I'm mimicking/copying the behavior that we have right now already. I asked myself the same question - rolling back to orphan state shouldn't be that hard after all. But that would be a change in behavior that - if considered a fix/improvement - would be orthogonal to the multi-pick fix. >> if (reset_for_rollback(sha1)) >> goto fail; >> remove_sequencer_state(); >> @@ -1086,11 +1090,8 @@ int sequencer_pick_revisions(struct replay_opts *opts) >> walk_revs_populate_todo(&todo_list, opts); >> if (create_seq_dir() < 0) >> return -1; >> - if (get_sha1("HEAD", sha1)) { >> - if (opts->action == REPLAY_REVERT) >> - return error(_("Can't revert as initial commit")); >> - return error(_("Can't cherry-pick into empty head")); >> - } >> + if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT)) >> + return error(_("Can't revert as initial commit")); > > Not a new issue, but I cannot quite fathom what this error message > wants to say. "Can't revert an initial commit"? Cannot create a "revert commit" as the initial commit on a yet unborn branch. Maybe "nothing to revert" would be clearer, but then another user might ask: If I say "git revert deabeef" and there is a commit "deadbeef" then why is there "nothing to revert"? Applying the reverse of a patch to an empty tree should result in an empty tree, and creating a commit with that empty tree as "This reverts commit deadbeef" is what we are refusing here, unless I'm confused. >> save_head(sha1_to_hex(sha1)); >> save_opts(opts); >> return pick_commits(todo_list, opts); > -- 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