Re: [PATCH] cherry-pick: allow to pick to unborn branches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]