Re: [PATCH 1/2] rebase -i: support --root without --onto

From: Chris Webb <chris@arachsys.com>
Date: Jun 26, 2012, 19:38 UTC
Message-ID: <20120626193817.GD30779@arachsys.com>
In-Reply-To: <7vtxxxc22x.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 29 quoted lines

> I am not quite sure what is going on in this "then" clause.
> 
> Chris Webb <chris@arachsys.com> writes:
>
> > +		git commit --allow-empty --allow-empty-message --amend \
> > +			   --no-post-rewrite -n -q -C $1 &&
> 
> At this point, nobody touched the empty sentinel root yet; you
> rewrite its log message and authorship using the picked commit.
> 
> > +			pick_one -n $1 &&
> 
> And then you create a new commit that records the update "$1" does
> relative to its parent (this hopefully only contains additions -- is
> it sensible to die-with-patch if it doesn't?), making sure that it
> does not fast-forward.  Does this always make the result a root commit?
> If "$1" has parents, wouldn't it become a child of the commits its
> parents were rewritten to (if any) in pick_one_preserving_merges()
> that is called from pick_one?
> 
> > +			git commit --allow-empty --allow-empty-message \
> > +				   --amend --no-post-rewrite -n -q -C $1 ||
> 
> And then you rewrite the log and authorship of that one.
> 
> In short, my questions are:
> 
>  (1) what is the purpose of the first "commit --amend" to update the
>      sentinel root commit?

This first commit --amend isn't supposed to change the empty tree in the commit: the tree and index should be unchanged at this point. I'm only running it to set the commit message and author.

The idea here is that I want the author and commit message already in place if cherry-pick (and hence pick_one -n) fails so that we drop out for the user to resolve conflicts.

This seems to be the way git cherry-pick or git merge behave when they need conflicts resolving, and I wanted the behaviour to be consistent to avoid surprises.

Is there a way of explicitly writing my commit --amend to make this intention clearer? Would a -o without paths spell this out, or would it just make thing more confusing?

>  (2) Is the purpose of "pick_one -n" done here to create a root
>      commit?  Does it always do so correctly?

pick_one -n cherry-picks the changes without actually making a commit. It's already used in the squash case, so should be well-tested.

Following this, the second commit --amend actually commits those changes, amending the sentinel. I don't change the message at all at this stage because it's already correct.

Similar question to before: is there a clearer way to ask commit --amend to leave the author and commit message unchanged rather than supply them explicitly all over again, or shall I just comment to explain the intention? EDITOR=: perhaps?

> This makes "git rebase --root" without $onto imply "-i", which makes
> sense, but it was a bit unexpected (it wasn't in the proposed log
> message).

Yes, sorry, you're quite right. It's only mentioned in the tests patch.

Would you prefer me to make this change in a separate patch, and generate an error for this case in the initial commit, or just explain it properly in the log message to go with the original combined patch?

Best wishes,

Chris.