Editing the root commit

Editing the root commit

[Chris Webb]

I've recently been polishing up some private code for release, using git
rebase --interactive to expand on some commit messages.

In a couple of cases, I wanted to edit the root commit. (Adding a COPYING
file, for example.) I've been successfully doing this with an explicit

  ROOT=$(git log --pretty=format:%H | tail -n 1)
  git checkout $ROOT
  git commit --amend --message='Initial import modified' # for example
  git rebase --onto HEAD $ROOT master

However, this brought two questions to mind.

The first is whether there's a clever symbolic way to refer to the root of
the current branch, rather than tailing git log output? gitrevisions(7)
doesn't obviously suggest one.

The second question is whether it's possible to use git rebase --interactive
to edit the root commit along with some subsequent ones in one fell swoop?

My fingers half-remember doing something like

  git checkout --orphan rewritten
  git rm -rf
  git rebase --interactive --root --onto rewritten master

a year or so ago, but this now fails (on git 1.7.10) with the somewhat
surprising error

  $ git rebase --root --onto rewritten master
  fatal: Needed a single revision
  Does not point to a valid commit: master

Am I misremembering the recipe here, or has the behaviour changed? It seems
to fail identically with or without --interactive.

Best wishes,

Chris.

Re: Editing the root commit

[Junio C Hamano]

Chris Webb <chris@arachsys.com> writes:

> The first is whether there's a clever symbolic way to refer to the root of
> the current branch, rather than tailing git log output? gitrevisions(7)
> doesn't obviously suggest one.

There is not, for a few good reasons.

The very first commit is no more special than any other commit in
the history.  Some projects may start with "the first working
version", in which case there may be some value to be able to refer
to it, while some other projects may start with "the initial
snapshot" whose specialness comes _only_ from it being the first
one.

"The first working version" specialness may deserve to be tagged,
and in that case you would already have a name to refer to it.  If
on the other hand the first one isn't even worth to be tagged, it
does not deserve to waste a short-hand to refer to it at the
machinery level.  So there is nothing listed in the gitrevisions(7)
for it.

Also there is a more important question: Which root commit?  There
does not necessarily have to be a single "the root" commit in the
history.

>   git checkout --orphan rewritten
>   git rm -rf
>   git rebase --interactive --root --onto rewritten master

The argument to "--onto" must be a commit, but "checkout --orphan"
is not a way to create a commit; it is a way to bring you to a state
without any commit.

I personally think "git rebase -i --root" should be made to just
work without requiring "--onto" and let you "edit" even the first
one in the history. It is understandable that nobody bothered, as
people are a lot less often rewriting near the very beginning of the
history than otherwise.

Even though I wouldn't bother doing this myself, I wouldn't mind
reviewing a patch series ;-)

Re: Editing the root commit

[Chris Webb]

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

[symbolic reference to root commit]
> Also there is a more important question: Which root commit?  There
> does not necessarily have to be a single "the root" commit in the
> history.

Ah yes, very good point. Anything with a subtree merge would have several
roots, for example.

I suppose the thing that makes root commits special is their lack of
parents, so the most direct way to get a list of root commits for the
current branch would just be

  git rev-list --max-parents=0 HEAD

making the recipe

  ROOT=$(git rev-list --max-parents=0 master)
  git checkout "$ROOT" -- # will fail if more than one root
  git commit --amend
  git rebase [--interactive] --onto HEAD "$ROOT" master

> I personally think "git rebase -i --root" should be made to just
> work without requiring "--onto" and let you "edit" even the first
> one in the history. It is understandable that nobody bothered, as
> people are a lot less often rewriting near the very beginning of the
> history than otherwise.
> 
> Even though I wouldn't bother doing this myself, I wouldn't mind
> reviewing a patch series ;-)

Okay, I'll take a look when I finish my current project!

Best wishes,

Chris.

Re: Editing the root commit

[jaseem abid]

On Tue, Jun 19, 2012 at 2:46 PM, Chris Webb <chris@arachsys.com> wrote:
>
> I've recently been polishing up some private code for release, using git
> rebase --interactive to expand on some commit messages.
>
> In a couple of cases, I wanted to edit the root commit. (Adding a COPYING
> file, for example.) I've been successfully doing this with an explicit
>
>  ROOT=$(git log --pretty=format:%H | tail -n 1)
>  git checkout $ROOT
>  git commit --amend --message='Initial import modified' # for example
>  git rebase --onto HEAD $ROOT master
>
> However, this brought two questions to mind.
>
> The first is whether there's a clever symbolic way to refer to the root of
> the current branch, rather than tailing git log output? gitrevisions(7)
> doesn't obviously suggest one.
>
> The second question is whether it's possible to use git rebase
> --interactive
> to edit the root commit along with some subsequent ones in one fell swoop?
>
> My fingers half-remember doing something like
>
>  git checkout --orphan rewritten
>  git rm -rf
>  git rebase --interactive --root --onto rewritten master
>
> a year or so ago, but this now fails (on git 1.7.10) with the somewhat
> surprising error
>
>  $ git rebase --root --onto rewritten master
>  fatal: Needed a single revision
>  Does not point to a valid commit: master
>
> Am I misremembering the recipe here, or has the behaviour changed? It
> seems
> to fail identically with or without --interactive.

I was trying to do something very similar yesterday. I was cleaning up
a lot of commits with a rebase -i when I figured out that I cant do
that on the root commit.

The man on interactive rebase says,

        Start it with the last commit you want to retain as-is:
        git rebase -i <after-this-commit>

which means root cant be included. Finally I had to settle for some
solution like what you mentioned.

If possible Junio, can a feature be added to include root commit also
to git rebase -i somehow so that a rewrite including root is easier
and more straightforward ?

--
Jaseem Abid
http://jaseemabid.github.com

Re: Editing the root commit

[Chris Webb]

Chris Webb <chris@arachsys.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
> > Even though I wouldn't bother doing this myself, I wouldn't mind
> > reviewing a patch series ;-)
> 
> Okay, I'll take a look when I finish my current project!

I had a bit of spare time this morning and had a quick look through
git-rebase--interactive.sh.

Apart from the validation, message and reflog code in git-rebase.sh and
git-rebase--interactive.sh that would need fixing up to know about this
case, the essence of this seems to be starting with an orphan commit instead
of a commit descended from $onto right at the end of --interactive.

I'd love to write something like

  git checkout ${onto:---orphan}

(or a variant) but can git be persuaded to have an orphan detached HEAD like
that?

An orphan branch appears to involve HEAD containing 'ref: refs/heads/foo'
where refs/heads/foo doesn't exist yet, but I get 'not a git repo' instead
of an orphan detached HEAD with either :>.git/HEAD or rm .git/HEAD.

Cheers,

Chris.

Re: Editing the root commit

[Junio C Hamano]

Chris Webb <chris@arachsys.com> writes:

> Chris Webb <chris@arachsys.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > Even though I wouldn't bother doing this myself, I wouldn't mind
>> > reviewing a patch series ;-)
>> 
>> Okay, I'll take a look when I finish my current project!
>
> I had a bit of spare time this morning and had a quick look through
> git-rebase--interactive.sh.
>
> Apart from the validation, message and reflog code in git-rebase.sh and
> git-rebase--interactive.sh that would need fixing up to know about this
> case, the essence of this seems to be starting with an orphan commit instead
> of a commit descended from $onto right at the end of --interactive.
>
> I'd love to write something like
>
>   git checkout ${onto:---orphan}
>
> (or a variant) but can git be persuaded to have an orphan detached HEAD like
> that?

For the root commit in the history, you check it out on the detached
HEAD.  Under "--interactive" if the insn sheet tells you to allow
the user to "edit/amend/reword" it, give control back the user after
you have detached HEAD at that commit.  The user experience should
be identical to the case you are replaying on an existing commit
after that point.

But lets step back a bit and look at the whole picture, to make sure
we are on the same page.

    $ git rebase [-i] frotz

looks at where you are, finds where you forked from 'frotz', and
replays everything you have done since you forked onto the tip of
'frotz'.

    $ git rebase [-i] --onto nitfol frotz

replays the same history onto the tip of 'nitfol' instead.

    $ git rebase [-i] --root --onto nitfol

looks at the entire history leading to where you are, and replays
everything you have done onto the tip of 'nitfol'.

What if we do not say --onto here?  I am not asking what the current
implementation does (we get an error message saying "I want 'onto'
specified").  What _should_ this command mean to a naïve user?

    $ git rebase [-i] --root

I think it should mean "replay all my history down to root".  The
original root commit should become the new root commit in the
rewritten history.

Re: Editing the root commit

[Jeff King]

On Wed, Jun 20, 2012 at 11:25:32AM -0700, Junio C Hamano wrote:

> What if we do not say --onto here?  I am not asking what the current
> implementation does (we get an error message saying "I want 'onto'
> specified").  What _should_ this command mean to a naïve user?
> 
>     $ git rebase [-i] --root
> 
> I think it should mean "replay all my history down to root".  The
> original root commit should become the new root commit in the
> rewritten history.

I think that is the only thing that makes sense. And it should be easy
with non-interactive rebase, because we always start with the root commit,
and it always applies cleanly.

For interactive rebase, though, it's a little trickier. Earlier you
said:

> For the root commit in the history, you check it out on the detached
> HEAD.  Under "--interactive" if the insn sheet tells you to allow
> the user to "edit/amend/reword" it, give control back the user after
> you have detached HEAD at that commit.  The user experience should
> be identical to the case you are replaying on an existing commit
> after that point.

That makes sense if the first instruction involves picking that first
commit. But if the first commit is deleted (or reordered), then it is
not appropriate to detach to the root; we must detach to the first
picked commit, which we can only do after we see the final instruction
sheet.

Git-rebase tries to do the rewind before dropping to run_specific_rebase.
However, I think we might be OK, as it seems that there is a special
exception for "interactive", and we drop to run_specific_rebase early in
that case.

-Peff

PS I have no clue how multiple roots would do. Without --preserve merges,
   I would except history to be flattened, and that should be OK (the root
   commit will become a non-root, just as it would if you were actually
   going --onto something else).  But I suspect --preserve-merges might be
   tricky, as you might have to create several root commits during the
   course of processing the instruction sheet.

Re: Editing the root commit

[Chris Webb]

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

> What if we do not say --onto here?  I am not asking what the current
> implementation does (we get an error message saying "I want 'onto'
> specified").  What _should_ this command mean to a na??ve user?
> 
>     $ git rebase [-i] --root
> 
> I think it should mean "replay all my history down to root".  The
> original root commit should become the new root commit in the
> rewritten history.

Yes, I definitely agree with everything you say here. That's exactly what
I'd expect from git rebase --root without --onto, and what I'd hope to be
able to implement in a patch series.

> For the root commit in the history, you check it out on the detached
> HEAD.  Under "--interactive" if the insn sheet tells you to allow
> the user to "edit/amend/reword" it, give control back the user after
> you have detached HEAD at that commit.  The user experience should
> be identical to the case you are replaying on an existing commit
> after that point.

I think it might a little more complicated than detecting when we have to do
a commit --amend on the root commit though? The user might have reordered
the first commit (introducing A and B, say) with the second commit
(introducing C and D), or dropped the original root commit entirely.

My understanding of the way --interactive works at the moment is that it
checks out the starting commit (whether given explicitly by --onto or taken
from <upstream>) and then 'plays out' the commits as described in the
instruction sheet.

I could re-use this unchanged if I could do a git checkout --orphan without
having to create a new branch, but I don't think this is allowed: I can have
a detached head or make an orphan checkout onto a new branch, but not both
at the same time? Would you prefer that I create a temporary branch just to
be able to git checkout --orphan onto it here, or that I add support for
this kind of 'detached HEAD with no parent' state, or is there a natural way
to rework --interactive without needing to do this which I'm missing?

Best wishes,

Chris.

Re: Editing the root commit

[Chris Webb]

Jeff King <peff@peff.net> writes:

> But if the first commit is deleted (or reordered), then it is not
> appropriate to detach to the root; we must detach to the first picked
> commit, which we can only do after we see the final instruction sheet.

It's worse than that isn't it? If you have

  A -- B -- C

and the sheet says drop A, pick B, pick C, you can't detach to B. You want
the commit B as a root (i.e. with no parent), not the commit B with parent
A. You need to have the patch from A to B replayed as the first commit on an
empty branch (only without the branch).

Cheers,

Chris.

Re: Editing the root commit

[Jeff King]

On Wed, Jun 20, 2012 at 08:39:23PM +0100, Chris Webb wrote:

> Jeff King <peff@peff.net> writes:
> 
> > But if the first commit is deleted (or reordered), then it is not
> > appropriate to detach to the root; we must detach to the first picked
> > commit, which we can only do after we see the final instruction sheet.
> 
> It's worse than that isn't it? If you have
> 
>   A -- B -- C
> 
> and the sheet says drop A, pick B, pick C, you can't detach to B. You want
> the commit B as a root (i.e. with no parent), not the commit B with parent
> A. You need to have the patch from A to B replayed as the first commit on an
> empty branch (only without the branch).

Oh, you're right. I think you would have to do some custom magic to make
the first commit without looking at HEAD, like:

  1. Erase the index and working tree.

  2. Apply the patch from B^ to B (where B is the first picked commit).

  3. git-commit-tree manually with no parents.

  4. Point HEAD to the newly made commit.

But there is a very good chance of step (2) causing conflicts, at which
point you would have to give control back to the user. And what is in
HEAD at that point that would give them a meaningful answer to "git diff
--cached" or similar?

I think the only thing you can do is make a fake sentinel commit (with
an empty tree) to put in HEAD, and then remove the sentinel immediately
after the first commit is put in place (making sure not to include it in
the first commit's parent list). Yuck.

-Peff

Re: Editing the root commit

[Chris Webb]

Jeff King <peff@peff.net> writes:

> I think the only thing you can do is make a fake sentinel commit (with
> an empty tree) to put in HEAD, and then remove the sentinel immediately
> after the first commit is put in place (making sure not to include it in
> the first commit's parent list). Yuck.

If I do this:

diff --git a/path.c b/path.c
index 6f2aa69..1b3b6f3 100644
--- a/path.c
+++ b/path.c
@@ -169,8 +169,9 @@ int validate_headref(const char *path)
 	int fd;
 	ssize_t len;
 
+	/* Allow HEAD to be entirely missing for detached orphan state */
 	if (lstat(path, &st) < 0)
-		return -1;
+		return errno == ENOENT ? 0 : -1;
 
 	/* Make sure it is a "refs/.." symlink */
 	if (S_ISLNK(st.st_mode)) {


to thwart the sanity check, I can do 'rm $GIT_DIR/HEAD' to put my HEAD into a
state where it is both detached and unborn, i.e. so that my next commit will
result in a detached HEAD pointing at a root commit.

Surprisingly, this check appears to be the only thing disallowing such a state,
and the result behaves as sanely as a normal git-checkout --orphan <branch>
does! Using a detached unborn HEAD like this would avoid any need for sentinel
commits or the like in generalising rebase: we'd just do

  git rm -rf .
  rm -f $GIT_DIR/index $GIT_DIR/HEAD

instead of git checkout $onto, and be away replaying the commits or executing
the instruction sheet as normal.

If I prepared a proper patch series with docs and tests, would allowing this be
acceptable? I don't want to work on it if there's an intentional design
decision to explicitly disallow it. However, apart from just rebase and
rebase--interactive, I suspect other scripts which operate on history will be
more easily generalised to work on history right up to the root commit if such
a state were allowed.

PS Whilst experimenting, I also noticed a (presumably unintentional) behaviour:

  $ git init .
  Initialized empty Git repository in /tmp/foo/.git/
  $ git checkout --detach
  $ touch bar
  $ git add bar
  $ git commit -m test
  [(null) (root-commit) 17b5bf9] test
   0 files changed
   create mode 100644 bar
  $ ls .git/refs/heads/
  (null)
  $

Here we've created a branch with the strange name '(null)' instead of actually
detaching, or refusing to detach because we're on an unborn branch.

Assuming this is a bug, I'll cook up a patch to fix it either way, either by
entering a detached unborn state if we're allowing that, or to refuse to detach
if we're not allowing that state.

Best wishes,

Chris.

Re: Editing the root commit

[Junio C Hamano]

Chris Webb <chris@arachsys.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> I think the only thing you can do is make a fake sentinel commit (with
>> an empty tree) to put in HEAD, and then remove the sentinel immediately
>> after the first commit is put in place (making sure not to include it in
>> the first commit's parent list). Yuck.
>
> If I do this:
>
> diff --git a/path.c b/path.c
> index 6f2aa69..1b3b6f3 100644
> --- a/path.c
> +++ b/path.c
> @@ -169,8 +169,9 @@ int validate_headref(const char *path)
>  	int fd;
>  	ssize_t len;
>  
> +	/* Allow HEAD to be entirely missing for detached orphan state */
>  	if (lstat(path, &st) < 0)
> -		return -1;
> +		return errno == ENOENT ? 0 : -1;
>  
>  	/* Make sure it is a "refs/.." symlink */
>  	if (S_ISLNK(st.st_mode)) {
>
>
> to thwart the sanity check, I can do 'rm $GIT_DIR/HEAD' to put my HEAD into a
> state where it is both detached and unborn, i.e. so that my next commit will
> result in a detached HEAD pointing at a root commit.

No thanks.  It will be too big a change to the fundamental invariant
for what a git directory is (and isn't).  It is simply unacceptable
to suddenly start treating a random directory that does not even
have HEAD as a git directory.

It would be a lot more palatable approach to teach "rebase -i" defer
its "detaching HEAD to the onto commit" step before starting to read
the insn sheet.  Would such a change be too involved for it to be
worth supporting "rebase --root -i"?

Re: Editing the root commit

[Chris Webb]

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

[detached unborn HEAD]
> No thanks.  It will be too big a change to the fundamental invariant
> for what a git directory is (and isn't).  It is simply unacceptable
> to suddenly start treating a random directory that does not even
> have HEAD as a git directory.

No problem. I imagine you wouldn't be keen on other representations of the
same state either, e.g. a null sha1 in HEAD, and would prefer to disallow
it altogether?

In that case, would you like me to do a test and a fix for the '(null)'
branch behaviour of

  git checkout --orphan dummy && git checkout --detach

? I assume that can't be intentional: looks from the code like it was
intended to tell me I'm on an unborn branch and can't do that.

> It would be a lot more palatable approach to teach "rebase -i" defer
> its "detaching HEAD to the onto commit" step before starting to read
> the insn sheet.  Would such a change be too involved for it to be
> worth supporting "rebase --root -i"?

I'm not sure as I don't really know the rebase shell scripts well at all,
but I'm happy to take a look and see. I imagine we wouldn't want to make
rebase -i much more complicated (and consequently harder to work on) just to
cover this rarely needed variant, but if it's relatively self-contained
(e.g. detect we're supposed to be making an orphan commit and do it by hand
instead of with git commit, maybe?) it would presumably be worthwhile?

Cheers,

Chris.

Re: Editing the root commit

[Chris Webb]

Chris Webb <chris@arachsys.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > It would be a lot more palatable approach to teach "rebase -i" defer
> > its "detaching HEAD to the onto commit" step before starting to read
> > the insn sheet.  Would such a change be too involved for it to be
> > worth supporting "rebase --root -i"?
> 
> I'm not sure as I don't really know the rebase shell scripts well at all,
> but I'm happy to take a look and see.

Ignoring the implementation, I think the nastiest bit here is what happens
for the user if there's a conflict, as Peff pointed out. Ideally, we want to
checkout a state we're going to replay the patch onto, so that if we drop
out because there's conflict (e.g. the patch modifies a file which doesn't
exist yet), git diff and git diff --cached do something sensible.

Without a detached orphan checkout, as Peff says, we'd have to put a
temporary empty commit in and then manually make the first commit avoiding
using it as a parent, which is very ugly. I'll take a look, but might be too
complex to be worth doing for this corner-case.

Cheers,

Chris.

Re: Editing the root commit

[Junio C Hamano]

Chris Webb <chris@arachsys.com> writes:

> Ignoring the implementation, I think the nastiest bit here is what happens
> for the user if there's a conflict, as Peff pointed out. Ideally, we want to
> checkout a state we're going to replay the patch onto, so that if we drop
> out because there's conflict (e.g. the patch modifies a file which doesn't
> exist yet), git diff and git diff --cached do something sensible.

Step back a bit.

If you are doing "rebase -i --root", what does it _mean_ to reorder
commits and move the root commit to somewhere else, or insert
something else that is _not_ the root to the beginning of the
sequence?

It does not make _any_ sense to me.  For one thing, the root commit
is to replay "addition of all these paths" to void (if you were doing
"rebase -i --root --onto $there", then $there may not be a void, but
it needs not to have overlapping paths for the result to make any
sense), and moving it to somewhere _other than_ the root location
does not make much sense.  For another, a non-root commit is mostly
replay "these changes to these existing paths", and replaying such a
change to a void does not make any sense either.

So in that sense, perhaps

	rebase -i --root

in a history

	A---B---C

should behave as if you did

	rebase -i A

got an insn sheet that looked like

	pick B
        pick C

and then you made it to look like

	exec false
        pick B
        pick C

to get the control back when the HEAD is detached at A, in order for
you to muck with the tree and "git commit --amend" to reword the
message.

Re: Editing the root commit

[Chris Webb]

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

> If you are doing "rebase -i --root", what does it _mean_ to reorder
> commits and move the root commit to somewhere else, or insert
> something else that is _not_ the root to the beginning of the
> sequence?
> 
> It does not make _any_ sense to me.  For one thing, the root commit
> is to replay "addition of all these paths" to void (if you were doing
> "rebase -i --root --onto $there", then $there may not be a void, but
> it needs not to have overlapping paths for the result to make any
> sense), and moving it to somewhere _other than_ the root location
> does not make much sense.  For another, a non-root commit is mostly
> replay "these changes to these existing paths", and replaying such a
> change to a void does not make any sense either.

As you say, it depends on the commits you're re-ordering, but in my case the
first few commits were entirely additions, and I wanted to add these files
in a different order, and add different batches at once, with more sensible
commit messages to explain to the reader what was going on. The addition of
COPYING from quite a long way forward needed to move right back into the
first commit too.

> So in that sense, perhaps
> 
> 	rebase -i --root
> 
> in a history
> 
> 	A---B---C
> 
> should behave as if you did
> 
> 	rebase -i A
> 
> got an insn sheet that looked like
> 
> 	pick B
>         pick C
> 
> and then you made it to look like
> 
> 	exec false
>         pick B
>         pick C
> 
> to get the control back when the HEAD is detached at A, in order for
> you to muck with the tree and "git commit --amend" to reword the
> message.

That would be easier to implement, certainly, but it makes --root --onto
inconsistent with --root without --onto, which does work 'in the standard
way' at present. It's also just a bit of syntactic sugar for something you
can already do with rebase -i at present, in exactly the way you describe
above.

Maybe it's the best I can sensibly do though. I'm away this weekend, but
will see what I can cook up one way or the other next week when the round
tuit supply is topped up!

Cheers,

Chris.

Re: Editing the root commit

[Martin von Zweigbergk]

On Wed, Jun 20, 2012 at 11:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>    $ git rebase [-i] --root --onto nitfol
>
> looks at the entire history leading to where you are, and replays
> everything you have done onto the tip of 'nitfol'.
>
> What if we do not say --onto here?

Or even leaving out the --onto for that matter. If I understand
correctly, "git rebase --root --onto nitfol" is (more-or-less) just
syntactic sugar for "git rebase nitfol", see
http://thread.gmane.org/gmane.comp.version-control.git/181024/focus=182054.

I have an old branch from around the time of that thread that make
this fact (that "--root --onto" is syntactic sugar) a little clearer
in the code. I'll try to finally send out some of those patches as
soon as I can.

>  What _should_ this command mean to a naïve user?
>
>    $ git rebase [-i] --root
>
> I think it should mean "replay all my history down to root".  The
> original root commit should become the new root commit in the
> rewritten history.

I agree, and in this case, the --root flag would not just be syntactic sugar.

Martin

Re: Editing the root commit

[Chris Webb]

Jeff King <peff@peff.net> writes:

> I think the only thing you can do is make a fake sentinel commit (with
> an empty tree) to put in HEAD, and then remove the sentinel immediately
> after the first commit is put in place (making sure not to include it in
> the first commit's parent list). Yuck.

I thought this would turn out far more horrible than it actually did in
practice.

The follow-up patch adds this empty sentinel and then automatically squashes
it into the root commit(s) as they're picked. The resulting rebase -i --root
can handle re-ordering of commits (picking a new root), editing of the root
commit, dropping the root commit, and so on.

In the second patch, I've written some simple tests to demonstrate this and
to cover cases that seem particularly likely to break, as well as removing a
test_must_fail for --root without --onto, because it no longer does!

Cheers,

Chris.

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

[Chris Webb]

Allow --root to be specified to rebase -i without --onto, making it
possible to edit and re-order all commits right back to the root(s).

If there is a conflict to be resolved when applying the first change,
the user will expect a sane index and working tree to get sensible
behaviour from git-diff and friends, so create a sentinel commit with an
empty tree to rebase onto. Automatically squash the sentinel with any
commits rebased directly onto it, so they end up as root commits in
their own right and retain their authorship and commit message.

Signed-off-by: Chris Webb <chris@arachsys.com>
---
 Documentation/git-rebase.txt |    9 +++++----
 git-rebase--interactive.sh   |   24 ++++++++++++++++++------
 git-rebase.sh                |   14 ++++++++++++--
 3 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 147fa1a..85b5e44 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git rebase' [-i | --interactive] [options] [--onto <newbase>]
 	[<upstream>] [<branch>]
-'git rebase' [-i | --interactive] [options] --onto <newbase>
+'git rebase' [-i | --interactive] [options] [--onto <newbase>]
 	--root [<branch>]
 'git rebase' --continue | --skip | --abort
 
@@ -348,10 +348,11 @@ idea unless you know what you are doing (see BUGS below).
 --root::
 	Rebase all commits reachable from <branch>, instead of
 	limiting them with an <upstream>.  This allows you to rebase
-	the root commit(s) on a branch.  Must be used with --onto, and
+	the root commit(s) on a branch.  When used with --onto, it
 	will skip changes already contained in <newbase> (instead of
-	<upstream>).  When used together with --preserve-merges, 'all'
-	root commits will be rewritten to have <newbase> as parent
+	<upstream>) whereas without --onto it will operate on every change.
+	When used together with both --onto and --preserve-merges,
+	'all' root commits will be rewritten to have <newbase> as parent
 	instead.
 
 --autosquash::
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c19b7c..ed5a6ba 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -417,6 +417,21 @@ record_in_rewritten() {
 	esac
 }
 
+do_pick () {
+	if test "$(git rev-parse HEAD)" = "$squash_onto"
+	then
+		git commit --allow-empty --allow-empty-message --amend \
+			   --no-post-rewrite -n -q -C $1 &&
+			pick_one -n $1 &&
+			git commit --allow-empty --allow-empty-message \
+				   --amend --no-post-rewrite -n -q -C $1 ||
+			die_with_patch $1 "Could not apply $1... $2"
+	else
+		pick_one $1 ||
+			die_with_patch $1 "Could not apply $1... $2"
+	fi
+}
+
 do_next () {
 	rm -f "$msg" "$author_script" "$amend" || exit
 	read -r command sha1 rest < "$todo"
@@ -428,16 +443,14 @@ do_next () {
 		comment_for_reflog pick
 
 		mark_action_done
-		pick_one $sha1 ||
-			die_with_patch $sha1 "Could not apply $sha1... $rest"
+		do_pick $sha1 "$rest"
 		record_in_rewritten $sha1
 		;;
 	reword|r)
 		comment_for_reflog reword
 
 		mark_action_done
-		pick_one $sha1 ||
-			die_with_patch $sha1 "Could not apply $sha1... $rest"
+		do_pick $sha1 "$rest"
 		git commit --amend --no-post-rewrite || {
 			warn "Could not amend commit after successfully picking $sha1... $rest"
 			warn "This is most likely due to an empty commit message, or the pre-commit hook"
@@ -451,8 +464,7 @@ do_next () {
 		comment_for_reflog edit
 
 		mark_action_done
-		pick_one $sha1 ||
-			die_with_patch $sha1 "Could not apply $sha1... $rest"
+		do_pick $sha1 "$rest"
 		warn "Stopped at $sha1... $rest"
 		exit_with_patch $sha1 0
 		;;
diff --git a/git-rebase.sh b/git-rebase.sh
index e616737..bde2be8 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -31,7 +31,7 @@ SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
 OPTIONS_SPEC="\
 git rebase [-i] [options] [--onto <newbase>] [<upstream>] [<branch>]
-git rebase [-i] [options] --onto <newbase> --root [<branch>]
+git rebase [-i] [options] [--onto <newbase>] --root [<branch>]
 git-rebase [-i] --continue | --abort | --skip
 --
  Available options are
@@ -364,6 +364,11 @@ and run me again.  I am stopping in case you still have something
 valuable there.'
 fi
 
+if test -n "$rebase_root" && test -z "$onto"
+then
+	test -z "$interactive_rebase" && interactive_rebase=implied
+fi
+
 if test -n "$interactive_rebase"
 then
 	type=interactive
@@ -397,7 +402,12 @@ then
 	die "invalid upstream $upstream_name"
 	upstream_arg="$upstream_name"
 else
-	test -z "$onto" && die "You must specify --onto when using --root"
+	if test -z "$onto"
+	then
+		empty_tree=`git hash-object -t tree /dev/null`
+		onto=`git commit-tree $empty_tree </dev/null`
+		squash_onto="$onto"
+	fi
 	unset upstream_name
 	unset upstream
 	upstream_arg=--root
-- 
1.7.10

[PATCH 2/2] Add tests for rebase -i --root without --onto

[Chris Webb]

Test for likely breakages in t3404, including successful reordering of
non-conflicting changes with a new root, correct preservation of commit
message and author in a root commit when it is squashed with the
sentinel, and presence of the sentinel following a conflicting
cherry-pick of a new root.

Remove test_must_fail for git rebase --root without --onto from t3412 as
this case will now be successfully handled by an implicit git rebase -i.

Signed-off-by: Chris Webb <chris@arachsys.com>
---
 t/t3404-rebase-interactive.sh |   27 +++++++++++++++++++++++++++
 t/t3412-rebase-root.sh        |    4 ----
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 025c1c6..6ffc9c2 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -755,4 +755,31 @@ test_expect_success 'rebase-i history with funny messages' '
 	test_cmp expect actual
 '
 
+test_expect_success 'rebase -i --root re-order and drop commits' '
+	git checkout E &&
+	FAKE_LINES="3 1 2 5" git rebase -i --root &&
+	test E = $(git cat-file commit HEAD | sed -ne \$p) &&
+	test B = $(git cat-file commit HEAD^ | sed -ne \$p) &&
+	test A = $(git cat-file commit HEAD^^ | sed -ne \$p) &&
+	test C = $(git cat-file commit HEAD^^^ | sed -ne \$p) &&
+	test 0 = $(git cat-file commit HEAD^^^ | grep -c ^parent\ )
+'
+
+test_expect_success 'rebase -i --root retain root commit author and message' '
+	git checkout A &&
+	echo B >file7 &&
+	git add file7 &&
+	GIT_AUTHOR_NAME="Twerp Snog" git commit -m "different author" &&
+	FAKE_LINES="2" git rebase -i --root &&
+	git cat-file commit HEAD | grep -q "^author Twerp Snog" &&
+	git cat-file commit HEAD | grep -q "^different author$"
+'
+
+test_expect_success 'rebase -i --root temporary sentinel commit' '
+	git checkout B &&
+	FAKE_LINES="2" test_must_fail git rebase -i --root &&
+	git cat-file commit HEAD | grep "^tree 4b825dc642cb" &&
+	git rebase --abort
+'
+
 test_done
diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
index 086c91c..e4f9da8 100755
--- a/t/t3412-rebase-root.sh
+++ b/t/t3412-rebase-root.sh
@@ -22,10 +22,6 @@ test_expect_success 'prepare repository' '
 	test_commit 4 B
 '
 
-test_expect_success 'rebase --root expects --onto' '
-	test_must_fail git rebase --root
-'
-
 test_expect_success 'setup pre-rebase hook' '
 	mkdir -p .git/hooks &&
 	cat >.git/hooks/pre-rebase <<EOF &&
-- 
1.7.10

git-commit bug (was Re: Editing the root commit)

[Chris Webb]

Chris Webb <chris@arachsys.com> writes:

> PS Whilst experimenting, I also noticed a (presumably unintentional)
> behaviour:
> 
>   $ git init .
>   Initialized empty Git repository in /tmp/foo/.git/
>   $ git checkout --detach
>   $ touch bar
>   $ git add bar
>   $ git commit -m test
>   [(null) (root-commit) 17b5bf9] test
>    0 files changed
>    create mode 100644 bar
>   $ ls .git/refs/heads/
>   (null)
>   $
> 
> Here we've created a branch with the strange name '(null)' instead of
> actually detaching, or refusing to detach because we're on an unborn
> branch.

This was introduced by abe199808c, which is intended to allow

  git init . && git checkout --orphan newbranch

but presumably wasn't also meant to enable

  git checkout --orphan foo
  git checkout --detach

This leads to a printf("%s", NULL) and thus

  $ git symbolic-ref HEAD
  refs/heads/(null)

I've followed up to this message with a patch including a test to catch this
in future.

Best wishes,

Chris.

[PATCH] git-checkout: disallow --detach on unborn branch

[Chris Webb]

abe199808c (git checkout -b: allow switching out of an unborn branch)
introduced a bug demonstrated by

  git checkout --orphan foo
  git checkout --detach
  git symbolic-ref HEAD

which gives 'refs/heads/(null)'.

This happens because we strbuf_addf(&branch_ref, "refs/heads/%s",
opts->new_branch) when opts->new_branch can be NULL for --detach.

Catch and forbid this case, adding a test to t2017 to catch it in
future.

Signed-off-by: Chris Webb <chris@arachsys.com>
---
 builtin/checkout.c         |    2 ++
 t/t2017-checkout-orphan.sh |    6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e8c1b1f..3980d5d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -915,6 +915,8 @@ static int switch_unborn_to_new_branch(struct checkout_opts *opts)
 	int status;
 	struct strbuf branch_ref = STRBUF_INIT;
 
+	if (!opts->new_branch)
+		die(_("You are on a branch yet to be born"));
 	strbuf_addf(&branch_ref, "refs/heads/%s", opts->new_branch);
 	status = create_symref("HEAD", branch_ref.buf, "checkout -b");
 	strbuf_release(&branch_ref);
diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
index 0e3b858..655f278 100755
--- a/t/t2017-checkout-orphan.sh
+++ b/t/t2017-checkout-orphan.sh
@@ -116,4 +116,10 @@ test_expect_success '--orphan refuses to switch if a merge is needed' '
 	git reset --hard
 '
 
+test_expect_success 'cannot --detach on an unborn branch' '
+	git checkout master &&
+	git checkout --orphan new &&
+	test_must_fail git checkout --detach
+'
+
 test_done
-- 
1.7.10

Re: git-commit bug

[Junio C Hamano]

Chris Webb <chris@arachsys.com> writes:

> Chris Webb <chris@arachsys.com> writes:
>
>> PS Whilst experimenting, I also noticed a (presumably unintentional)
>> behaviour:
>> 
>>   $ git init .
>>   Initialized empty Git repository in /tmp/foo/.git/
>>   $ git checkout --detach
>>   $ touch bar
>>   $ git add bar
>>   $ git commit -m test
>>   [(null) (root-commit) 17b5bf9] test
>>    0 files changed
>>    create mode 100644 bar
>>   $ ls .git/refs/heads/
>>   (null)
>>   $
>> 
>> Here we've created a branch with the strange name '(null)' instead of
>> actually detaching, or refusing to detach because we're on an unborn
>> branch.
>
> This was introduced by abe199808c, which is intended to allow
>
>   git init . && git checkout --orphan newbranch
>
> but presumably wasn't also meant to enable
>
>   git checkout --orphan foo
>   git checkout --detach

Correct.

On days like this, I really regret accepting that --orphan crap in
the first place.

Thanks for fixing things up.

> This leads to a printf("%s", NULL) and thus
>
>   $ git symbolic-ref HEAD
>   refs/heads/(null)
>
> I've followed up to this message with a patch including a test to catch this
> in future.
>
> Best wishes,
>
> Chris.

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

[Junio C Hamano]

Chris Webb <chris@arachsys.com> writes:

> +do_pick () {
> +	if test "$(git rev-parse HEAD)" = "$squash_onto"

The idea to create a sentinel root upfront and special case it here
is a good one.

> +	then

I am not quite sure what is going on in this "then" clause.

> +		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?

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

> diff --git a/git-rebase.sh b/git-rebase.sh
> index e616737..bde2be8 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -31,7 +31,7 @@ SUBDIRECTORY_OK=Yes
>  OPTIONS_KEEPDASHDASH=
>  OPTIONS_SPEC="\
>  git rebase [-i] [options] [--onto <newbase>] [<upstream>] [<branch>]
> -git rebase [-i] [options] --onto <newbase> --root [<branch>]
> +git rebase [-i] [options] [--onto <newbase>] --root [<branch>]
>  git-rebase [-i] --continue | --abort | --skip
>  --
>   Available options are
> @@ -364,6 +364,11 @@ and run me again.  I am stopping in case you still have something
>  valuable there.'
>  fi
>  
> +if test -n "$rebase_root" && test -z "$onto"
> +then
> +	test -z "$interactive_rebase" && interactive_rebase=implied
> +fi
> +

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).

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

[Chris Webb]

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

> 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.

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

[Junio C Hamano]

Chris Webb <chris@arachsys.com> writes:

>> 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.

Very understandable.  Perhaps in-code comments would have helped.

>>  (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.

OK.

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

[Chris Webb]

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

> Very understandable.  Perhaps in-code comments would have helped.

Shall I re-spin this with a comment to explain what's going on, and to
mention the implicit -i with rebase --root without -i, or would you prefer
to queue the existing version with any clarifications you think appropriate?

Best wishes,

Chris.

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

[Junio C Hamano]

Chris Webb <chris@arachsys.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Very understandable.  Perhaps in-code comments would have helped.
>
> Shall I re-spin this with a comment to explain what's going on, and to
> mention the implicit -i with rebase --root without -i, or would you prefer
> to queue the existing version with any clarifications you think appropriate?

A reroll would be preferred, as it would be less work for me ;-)

Thanks.

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

[Chris Webb]

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

> Chris Webb <chris@arachsys.com> writes:
> 
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> >> Very understandable.  Perhaps in-code comments would have helped.
> >
> > Shall I re-spin this with a comment to explain what's going on, and to
> > mention the implicit -i with rebase --root without -i, or would you prefer
> > to queue the existing version with any clarifications you think appropriate?
> 
> A reroll would be preferred, as it would be less work for me ;-)

No problem, I'll do that now. Thanks for the feedback and all your help with
this patch series.

Best wishes,

Chris.