git-rebase skips automatically no more needed commits

git-rebase skips automatically no more needed commits

[Francis Moreau]

Hello,

When rebasing my current branch onto the last master one, there're
sometimes some commits which doesn't add anything anymore.

Currently git-rebase produces the following message:

    nothing added to commit but untracked files present (use "git add" to track)

Would it be possible to add a new option to this command so it simply
skip the unneeded commit ?

Thanks
-- 
Francis

Re: git-rebase skips automatically no more needed commits

[Junio C Hamano]

Francis Moreau <francis.moro@gmail.com> writes:

> When rebasing my current branch onto the last master one, there're
> sometimes some commits which doesn't add anything anymore.
>
> Currently git-rebase produces the following message:
>
>     nothing added to commit but untracked files present (use "git add" to track)
>
> Would it be possible to add a new option to this command so it simply
> skip the unneeded commit ?

Our assumption has always been that it is a notable event that a patch
that does not get filtered with internal "git cherry" (which culls patches
that are textually identical to those that are already merged to the
history you are rebasing onto) becomes totally unneeded and is safe to ask
for human confirmation in the form of "rebase --skip" than to ignore it
and give potentially incorrect result silently.

Obviously you do not find it a notable event for some reason. We would
need to understand why, and if the reason is sensible, it _might_ make
sense to allow a user to say "git rebase --ignore-merged" or something
when starting the rebase.

Re: git-rebase skips automatically no more needed commits

[Francis Moreau]

Hello Junio,

On Tue, Sep 6, 2011 at 7:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Our assumption has always been that it is a notable event that a patch
> that does not get filtered with internal "git cherry" (which culls patches
> that are textually identical to those that are already merged to the
> history you are rebasing onto) becomes totally unneeded and is safe to ask
> for human confirmation in the form of "rebase --skip" than to ignore it
> and give potentially incorrect result silently.

Ok then I think this "git cherry" filtering is not working in my case
since it seems to me that commit that I cherry-picked are not
filtered, please see below.

>
> Obviously you do not find it a notable event for some reason. We would
> need to understand why, and if the reason is sensible, it _might_ make
> sense to allow a user to say "git rebase --ignore-merged" or something
> when starting the rebase.

My use case is the following: I'm maintaining a branch from an
upstream project (the kernel one). While the upstream project follows
its development cycle (including some fixes), my branch is stuck. I
sometime want to includes (or rather backport) some commits that
happened later in the development cycle. To do that I use "git
cherry-pick".

After some period, I'm allowed to rebase to a more recent commit from
the upstream project and  this rebase 'cancel' the previous 'git
cherry-pick' I did. But for some reasons, git telling me "nothing
added to commit ...", which is expected in my case, well I think,
hence my question.

Thanks.
-- 
Francis

Re: git-rebase skips automatically no more needed commits

[Michael J Gruber]

Francis Moreau venit, vidit, dixit 06.09.2011 21:28:
> Hello Junio,
> 
> On Tue, Sep 6, 2011 at 7:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Our assumption has always been that it is a notable event that a patch
>> that does not get filtered with internal "git cherry" (which culls patches
>> that are textually identical to those that are already merged to the
>> history you are rebasing onto) becomes totally unneeded and is safe to ask
>> for human confirmation in the form of "rebase --skip" than to ignore it
>> and give potentially incorrect result silently.
> 
> Ok then I think this "git cherry" filtering is not working in my case
> since it seems to me that commit that I cherry-picked are not
> filtered, please see below.
> 
>>
>> Obviously you do not find it a notable event for some reason. We would
>> need to understand why, and if the reason is sensible, it _might_ make
>> sense to allow a user to say "git rebase --ignore-merged" or something
>> when starting the rebase.
> 
> My use case is the following: I'm maintaining a branch from an
> upstream project (the kernel one). While the upstream project follows
> its development cycle (including some fixes), my branch is stuck. I
> sometime want to includes (or rather backport) some commits that
> happened later in the development cycle. To do that I use "git
> cherry-pick".
> 
> After some period, I'm allowed to rebase to a more recent commit from
> the upstream project and  this rebase 'cancel' the previous 'git
> cherry-pick' I did. But for some reasons, git telling me "nothing
> added to commit ...", which is expected in my case, well I think,
> hence my question.
> 
> Thanks.

Unless you had to resolve a conflict when cherry-picking, the picked
commit should be patch-equivalent to the original one, and thus dropped
by rebase automatically.

How do the two patches compare:

git show $picked >a
git show $original >b
git patch-id <a
git patch-id <b
git diff --no-index a b

Michael

Re: git-rebase skips automatically no more needed commits

[Junio C Hamano]

Francis Moreau <francis.moro@gmail.com> writes:

> If I start the rebase process with : "git rebase -i -p master foo"
> then the filtering is happening. Here 'foo' has been created with
> v2.6.39 tag as start point and contains some patches cherry-picked
> from the upstream.
>
> However if I call git-rebase this way: "git rebase -i -p --onto master
> v2.6.39 foo", then it seems that no filtering is done.
>
> Is that expected ?

Because "rebase --onto A B C" has to be usable when commit A does not have
any ancestry relationship with the history between B and C, I wouldn't be
surprised if the command does not look at commits on the history that lead
to A that _might_ be related to the ones between B and C. It does not know
how far to dig from A and stop, and obviously you do not want to dig down
to the beginning of the history. "rebase A B" on the other hand can safely
stop digging the history back from A down to where the history leading to
B forked (i.e. merge base between A and B).

I do not know if "expected" is the right adjective; understandable---yes,
unsurprised---yes, justifiable--- I dunno.

Re: git-rebase skips automatically no more needed commits

[Francis Moreau]

On Wed, Sep 7, 2011 at 6:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Francis Moreau <francis.moro@gmail.com> writes:
>
>> If I start the rebase process with : "git rebase -i -p master foo"
>> then the filtering is happening. Here 'foo' has been created with
>> v2.6.39 tag as start point and contains some patches cherry-picked
>> from the upstream.
>>
>> However if I call git-rebase this way: "git rebase -i -p --onto master
>> v2.6.39 foo", then it seems that no filtering is done.
>>
>> Is that expected ?
>
> Because "rebase --onto A B C" has to be usable when commit A does not have
> any ancestry relationship with the history between B and C,

But is it the common case ? That would mean that A and B are part of
totaly independant branches which is not that common I think.

I'm using "git rebase --onto A B C" because I want to simplify git's
work. I know that any commits between A..B are already in A history
and I'm not interested in rebasing them anyways.

> I wouldn't be
> surprised if the command does not look at commits on the history that lead
> to A that _might_ be related to the ones between B and C. It does not know
> how far to dig from A and stop, and obviously you do not want to dig down
> to the beginning of the history. "rebase A B" on the other hand can safely
> stop digging the history back from A down to where the history leading to
> B forked (i.e. merge base between A and B).

Hmm I dont understand why "rebase --onto A B C" wouldn't try to find
the merge base between A and B. If it doesn't (which is quite uncommon
I guess) then don't do the filtering as we're currently doing but if
there's a merge base (common case) then do the filtering.

No ?
-- 
Francis

Re: git-rebase skips automatically no more needed commits

[Martin von Zweigbergk]

On Thu, 8 Sep 2011, Francis Moreau wrote:

> On Wed, Sep 7, 2011 at 6:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Francis Moreau <francis.moro@gmail.com> writes:
> >
> >> If I start the rebase process with : "git rebase -i -p master foo"
> >> then the filtering is happening. Here 'foo' has been created with
> >> v2.6.39 tag as start point and contains some patches cherry-picked
> >> from the upstream.
> >>
> >> However if I call git-rebase this way: "git rebase -i -p --onto master
> >> v2.6.39 foo", then it seems that no filtering is done.

Patches that are in both sides of v2.6.39...foo will be filtered, but
as Junio said, what is in master is not considered. In your case,
since foo was created from the tag, there should of course be no
commits in foo..v2.6.39.

> I'm using "git rebase --onto A B C" because I want to simplify git's
> work.

What work? Calculating mege-base and patch-ids? But that's exactly
what you said you don't want to avoid, no?

> I know that any commits between A..B are already in A history

Did you mean "B..A" here?

> and I'm not interested in rebasing them anyways.

They wouldn't be rebased by running "git rebase A C" either... Maybe
I'm misunderstanding something.

> Hmm I dont understand why "rebase --onto A B C" wouldn't try to find
> the merge base between A and B. If it doesn't (which is quite uncommon
> I guess) then don't do the filtering as we're currently doing but if
> there's a merge base (common case) then do the filtering.

It could do that. I think that's what Junio meant by "justifiable--- I
dunno". In your case, though, it seems like "git rebase master foo"
would do exactly what you want.

Somewhat related, I think "git rebase --onto A B C" should NOT filter
out patches that also appear both sides of "B...C", because when
"--onto" is used, "B" is only used as a way to calculate the
merge-base. I think it would make more sense to filter out from
appears in both "B..C" what also appears in "C..A". This has been on
my todo list for way too long. Some day I'll send out a patch for it
and we'll see what others think.


Martin

Re: git-rebase skips automatically no more needed commits

[Martin von Zweigbergk]

On Thu, 8 Sep 2011, Francis Moreau wrote:

> On Thu, Sep 8, 2011 at 3:14 PM, Martin von Zweigbergk
> <martin.von.zweigbergk@gmail.com> wrote:
> >
> > Patches that are in both sides of v2.6.39...foo will be filtered, but
> 
> what do you mean by "both sides" ?

See the section called "SPECIFYING RANGES" in "git --help
rev-parse". It doesn't define "both sides", but what I mean by that is
"both in v2.6.39..foo and in foo..v2.6.39". I believe that section
will answer most of the other questions too.

> Yes my use of "git rebase --onto master foo~10 foo"  is equivalent to
> "git rebase master foo, the only difference is that the --onto variant
> allow me to limit the range of commits that I want to rebase. So I
> still want git rebase to do its the filtering process.

Ok, that is different. I think you used v2.6.39 instead of foo~10
previously. Assuming that v2.6.39 is the merge base of foo and master
and that foo~10 is a later commit than v2.6.39, you are right that
"git rebase --onto master foo~10 foo" could potentially filter out
patches already in foo..master, without calculating patch-ids for all
commits in master..foo for that matter. I think that would make sense
and as I said, it has been on my todo list for a long time. If
necessary, we could have a flag to disable the filtering e.g. when the
user knows that master is part of a completely separate history from
foo.


Martin

Re: git-rebase skips automatically no more needed commits

[Francis Moreau]

On Fri, Sep 9, 2011 at 4:23 AM, Martin von Zweigbergk
<martin.von.zweigbergk@gmail.com> wrote:
> On Thu, 8 Sep 2011, Francis Moreau wrote:
>
>> On Thu, Sep 8, 2011 at 3:14 PM, Martin von Zweigbergk
>> <martin.von.zweigbergk@gmail.com> wrote:
>> >
>> > Patches that are in both sides of v2.6.39...foo will be filtered, but
>>
>> what do you mean by "both sides" ?
>
> See the section called "SPECIFYING RANGES" in "git --help
> rev-parse". It doesn't define "both sides", but what I mean by that is
> "both in v2.6.39..foo and in foo..v2.6.39". I believe that section
> will answer most of the other questions too.
>
>> Yes my use of "git rebase --onto master foo~10 foo"  is equivalent to
>> "git rebase master foo, the only difference is that the --onto variant
>> allow me to limit the range of commits that I want to rebase. So I
>> still want git rebase to do its the filtering process.
>
> Ok, that is different. I think you used v2.6.39 instead of foo~10
> previously. Assuming that v2.6.39 is the merge base of foo and master
> and that foo~10 is a later commit than v2.6.39,

That's what I meant.

> you are right that
> "git rebase --onto master foo~10 foo" could potentially filter out
> patches already in foo..master, without calculating patch-ids for all
> commits in master..foo for that matter. I think that would make sense

you meant master..foo~10, didn't you ?

> and as I said, it has been on my todo list for a long time. If

please let me know when you submitting your work, I'm interested to see it.

> necessary, we could have a flag to disable the filtering e.g. when the
> user knows that master is part of a completely separate history from
> foo.

Can't git figure this out itself ? (I'm not saying the switch is useless)

Thanks
-- 
Francis

Re: git-rebase skips automatically no more needed commits

[Martin von Zweigbergk]

On Fri, 9 Sep 2011, Francis Moreau wrote:

> On Fri, Sep 9, 2011 at 4:23 AM, Martin von Zweigbergk
> <martin.von.zweigbergk@gmail.com> wrote:
> 
> > you are right that
> > "git rebase --onto master foo~10 foo" could potentially filter out
> > patches already in foo..master, without calculating patch-ids for all
> > commits in master..foo for that matter. I think that would make sense
> 
> you meant master..foo~10, didn't you ?

Yes. (I actually did mean "all commits in master..foo", but "_any_
commits in master..foo~10" is more clear and exact.)

> please let me know when you submitting your work, I'm interested to see it.

Will do. It's not really that much work, but I'm not sure when I will
have time for it. If you or anyone else is interested in doing it, you
are of course welcome.

> > necessary, we could have a flag to disable the filtering e.g. when the
> > user knows that master is part of a completely separate history from
> > foo.
> 
> Can't git figure this out itself ? (I'm not saying the switch is useless)

Yes, but this hypothetical flag would only be to speed things up by
avoid going to the roots to find out that the histories are disjoint.

Martin

Re: git-rebase skips automatically no more needed commits

[Francis Moreau]

On Fri, Sep 9, 2011 at 3:06 PM, Martin von Zweigbergk
<martin.von.zweigbergk@gmail.com> wrote:
> On Fri, 9 Sep 2011, Francis Moreau wrote:
>
>> On Fri, Sep 9, 2011 at 4:23 AM, Martin von Zweigbergk
>> <martin.von.zweigbergk@gmail.com> wrote:
>>
>> > you are right that
>> > "git rebase --onto master foo~10 foo" could potentially filter out
>> > patches already in foo..master, without calculating patch-ids for all
>> > commits in master..foo for that matter. I think that would make sense
>>
>> you meant master..foo~10, didn't you ?
>
> Yes. (I actually did mean "all commits in master..foo", but "_any_
> commits in master..foo~10" is more clear and exact.)
>
>> please let me know when you submitting your work, I'm interested to see it.
>
> Will do. It's not really that much work, but I'm not sure when I will
> have time for it. If you or anyone else is interested in doing it, you
> are of course welcome.

I'm a simple mortal user ;)

>
>> > necessary, we could have a flag to disable the filtering e.g. when the
>> > user knows that master is part of a completely separate history from
>> > foo.
>>
>> Can't git figure this out itself ? (I'm not saying the switch is useless)
>
> Yes, but this hypothetical flag would only be to speed things up by
> avoid going to the roots to find out that the histories are disjoint.

You're right but in my (mortal) experience, I'm always rebasing
branches onto another one which is not disjoint, so I'm assuming it's
the (very) common case, but I may be wrong. And what I'm assuming is
correct, I think it's make more sense to do the filtering by default
unless the flag tells to do otherwise.

Thanks
-- 
Francis

Re: git-rebase skips automatically no more needed commits

[Martin von Zweigbergk]

On Fri, 9 Sep 2011, Martin von Zweigbergk wrote:

> On Fri, 9 Sep 2011, Francis Moreau wrote:
> 
> > please let me know when you submitting your work, I'm interested to see it.
> 
> Will do. It's not really that much work [...]

It seems to be a little less straight-forward than I had first
expected.

We want to get all commits in $oldbase..$branch that do not share
patch-id with commits in $oldbase..$newbase. These are the commits
prefixed by '+' in 'git cherry $newbase $branch $oldbase'. However,
just getting the list of commits is not enough, because we currently
don't explicitly provide a list of commits to create patches from, but
instead run something like 'git format-patch --ignore-if-in-upstream
--stdout | git am --rebasing'.

I can see a few different solutions.

 1. Teach git-format-patch a --stdin option that makes it read the
    list of commits from the command line. git-format-patch currently
    itself walks the history, so I'm not sure how such an option
    should interact with current option. The options should probably
    be mutually exclusive.

 2. Somehow modify git-rebase--am.sh not to depend on format-patch. In
    [1], Junio mentions rewriting git-rebase--am.sh "to have
    format-patch avoid the cost of actually generating the patch text"
    and "when using "am" for rebasing we do not really care anything
    but the commit object names". If all we need is the commit name,
    why would we not use cherry-pick/sequencer instead of git-am?
    Sorry if this makes no sense; I'm not familiar with the git-am
    code.

 3. If I'm reading the code correctly, 'git cherry $upstream $branch
    $limit' does one walk to find patch-ids for "$upstream
    ^$branch". It then walks "$branch ^$upstream ^$limit" and removes
    the commits that have the same patch-id as a commit found in the
    first walk. Since format-patch uses the revision walking
    machinery, we could make it possible to ask git-rev-list directly
    what git-cherry does, i.e. to ask for "commits in a..b that don't
    share patch-id with commits in c..d". We could even make it more
    generic by allowing queries like "commits in
    $rev_list_expression_1 that don't share patch-id with commits in
    $rev_list_expression_2".

I'm not sure which option is better. Option 1 seems easiest to
implement. Option 3 seems a bit harder, but may bring more value. I'm
just not sure if teaching rev-list about patch-ids is generally useful
or if it would feel more like a hack for this specific case. Option 2
is less clear to me and I would probably need more input before
implementing, but I trust Junio that it would be a good long-term
solution.

I will be moving soon and I don't know how much time I will have to
implement any of this. Still, I would be happy to hear your
opinions. Maybe I'm just missing something and there is even a simple
solution to my problem.


Martin


[1] http://thread.gmane.org/gmane.comp.version-control.git/180976/focus=181085

Re: git-rebase skips automatically no more needed commits

[Junio C Hamano]

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

>  2. Somehow modify git-rebase--am.sh not to depend on format-patch. In
>     [1], Junio mentions rewriting git-rebase--am.sh "to have
>     format-patch avoid the cost of actually generating the patch text"
>     and "when using "am" for rebasing we do not really care anything
>     but the commit object names". If all we need is the commit name,
>     why would we not use cherry-pick/sequencer instead of git-am?

What I said is "all 'am' need to use from its input while rebasing is the
commit object name"; that is very different from "we have only commit
object name so we must use cherry-pick" or "because we have commit object
name, we can afford to use cherry-pick".

Look for $rebasing in git-am.sh and notice that:

 - We run get-author-ident-from-commit on $commit to keep the authorship
   information in $dotest/author-script for later use, instead of letting
   them read from $dotest/info prepared by "mailinfo".

 - My "fixing it would be just the matter of doing this" patch in a
   separate thread we had recently ("mailinfo" would obviously have
   trouble telling where the message ends whilerebasing a change with
   patch text in the commit log message) adds a similar logic to extract
   the patch text out of $commit in the same place as above, to override
   what "mailinfo" prepares in $dotest/patch.

 - Even with the above two differences from the normal "separate the
   metainfo and patch from the mailbox, and use them" codepath, $rebasing
   codepath uses the same "git apply && git write-tree && git commit-tree"
   flow.

Use of "apply && write-tree && commit-tree" is to avoid the merge overhead
of using cherry-pick, and historically, it used to matter a lot, because
starting merge-recursive was very slow (it was a Python script). Even
though we have merge-recursive rewritten in C these days, it probably
still matters, especially for large-ish projects, because merge will trash
the cached-tree information in the index, making write-tree inefficient.

Re: git-rebase skips automatically no more needed commits

[Martin von Zweigbergk]

Hi,

On Wed, 14 Sep 2011, Junio C Hamano wrote:

> What I said is "all 'am' need to use from its input while rebasing is the
> commit object name"; that is very different from "we have only commit
> object name so we must use cherry-pick" or "because we have commit object
> name, we can afford to use cherry-pick".

Right. I didn't mean to suggest that we should use cherry-pick.

> Look for $rebasing in git-am.sh and notice that:

Thanks! I had no idea about these "tricks" in git-am.sh. Now I
understand much better what you meant. So we are currently getting the
metainfo from the commit rather than from the mailbox. With your
patch, we would also get the patch body from the commit. The only
thing remaining after that is the commit log message, correct?

Once all that git-am reads from the mailbox while in $rebasing mode is
the commit name, it would be as small change to teach it to read a
list of commit names instead. Is this what you meant or did you really
mean for format-patch to generate output without patch body? At this
point the name 'am' becomes quite misleading, but maybe that's not
worth worrying about.


Thanks,
Martin

Re: git-rebase skips automatically no more needed commits

[Martin von Zweigbergk]

On Thu, 15 Sep 2011, Martin von Zweigbergk wrote:

[CC'ing Ramkunar regarding the footnote]

> [...] With your
> patch, we would also get the patch body from the commit. The only
> thing remaining after that is the commit log message, correct?

It turns out the commit log message have been taken from the commit
for ages. Sorry about not checking the code first.

I have started implementing the changes to git-am.sh now. I have made
it not use mailinfo when in $rebasing mode, as you suggested. So now
git-am works even if only a list of

  From $sha1 Mon Sep 17 00:00:00 2001

lines are fed to it. Since only the $sha1 is needed, we should
probably make --rebasing imply a patch-format is just a simple list of
revisions. Since the --rebasing flag is only for internal use, this
should be safe when it comes to backwards compatibility. [1]

Now the question is how to generate such lines, without the overhead
of a full format-patch call. We still want the --ignore-if-in-upstream
functionality, so we can not use plain rev-list. We also want it to
work with --root, so we can not use git-cherry. Finally, we can not
use 'git rev-list --cherry-pick --right-only' since it seems to be
inherently for symmetric ranges and therefore does not support
git-cherry's <limit> parameter.

If I'm not mistaken, we would have to teach some new option to some
command, but which one?

It doesn't feel right to teach format-patch a --commit-name-only,
simply because it would then no longer be a patch.

It's probably easiest to teach git-cherry a --root flag. Does that
sound like the right solution to people? I think it makes sense for
such an option to exist.

More involved would be to teach rev-list --cherry-pick also for
asymmetric ranges. I would imagine that "--cherry-pick $branch --not
$newbase $oldbase" could be defined as "all commits in $branch that
are not patch-equivalent to any commits in $newbase or $oldbase". Just
s/patch-equivalent/identical/ and you have the regular definition of
the rev-list, right? There is of course no need to actually calculate
the patch-id for any commits are not in "$branch --not $newbase
$oldbase" (identical => patch-equivalent).

I would go for teaching git-cherry --root, unless the "rev-list
--cherry-pick for asymmetric ranges" option would be useful
elsewhere. Either way, teaching git-cherry --root would not be a bad
thing, I think.


Martin


 [1] Long term, would it make sense to teach 'git cherry-pick' a '-2'
     option instead of using the '-3' option to 'git am'? (Where 'git
     cherry-pick -2' would try "apply && write-tree && commit-tree"
     before falling back to merging.)

Re: git-rebase skips automatically no more needed commits

[Ramkumar Ramachandra]

Hi Martin,

Martin von Zweigbergk writes:
> I have started implementing the changes to git-am.sh now. I have made
> it not use mailinfo when in $rebasing mode, as you suggested. So now
> git-am works even if only a list of
>
>  From $sha1 Mon Sep 17 00:00:00 2001

Ah, nice :)

> Now the question is how to generate such lines, without the overhead
> of a full format-patch call. We still want the --ignore-if-in-upstream
> functionality, so we can not use plain rev-list. We also want it to
> work with --root, so we can not use git-cherry. Finally, we can not
> use 'git rev-list --cherry-pick --right-only' since it seems to be
> inherently for symmetric ranges and therefore does not support
> git-cherry's <limit> parameter.

Using git-format-patch for anything except emailing patches to the
maintainer/ list is a mistake imo.

> I would go for teaching git-cherry --root, unless the "rev-list
> --cherry-pick for asymmetric ranges" option would be useful
> elsewhere. Either way, teaching git-cherry --root would not be a bad
> thing, I think.

Either this, or we have to write another wrapper around git-patch-id.

>  [1] Long term, would it make sense to teach 'git cherry-pick' a '-2'
>     option instead of using the '-3' option to 'git am'? (Where 'git
>     cherry-pick -2' would try "apply && write-tree && commit-tree"
>     before falling back to merging.)

Yeah, in the wake of scripts already doing "apply && write-tree &&
commit-tree" by hand, I think this proposal for git-cherry-pick makes
a lot of sense.  I'm not sure if the option should be called '-2'
though.

Thanks.

-- Ram

Re: git-rebase skips automatically no more needed commits

[Martin von Zweigbergk]

Hi Ram and Thomas,

On Sat, 24 Sep 2011, Ramkumar Ramachandra wrote:

> Martin von Zweigbergk writes:
> > [...] We also want it to
> > work with --root, so we can not use git-cherry.

I think I was confused here; git-cherry should work just fine. Simply
leaving out the limit, or setting it to $onto, makes the walk go all
the way to the roots. After thinking a bit more about this, I'm
instead wondering what the purpose of the --root flag to git-rebase
is. Thomas introduced it in 190f532 (rebase: learn to rebase root
commit, 2009-01-05).

If I understand correctly, it was introduced to solve exactly the same
reason problem that made Francis start this thread -- to avoid
re-applying patches that are already in $onto. In Thomas's patch,
git-rebase started passing $onto..$orig_head instead of
$upstream..$orig_head to git-format-patch. It also started passing
--root to git-format-patch, but that was made unneccessary a few days
later in 68c2ec7 (format-patch: show patch text for the root commit,
2009-01-10).

After my patches (that are not yet ready) that calculate revisions as
"git cherry $onto $orig_head $upstream", I don't think there should be
any need for the --root flag. The only place the flag is checked now
(in my current work tree, that is) is when deciding how to interpret
the remaining arguments. Although I have not tried (temporarily)
rewriting all the test cases from

  git rebase --root --onto upstream branch

to

  git rebase upstream branch

, I think it should work. What do you think, Thomas? I saw that
"--root" is also passed to the hook. Should that value be passed to
the hook also when the old base is not explicitly a root (by "rebase
--root"), but only implicitly so (by an $onto that is disjoint from
$branch)?

Martin

Re: git-rebase skips automatically no more needed commits

[Thomas Rast]

Martin von Zweigbergk wrote:
> 
> If I understand correctly, [rebase --root] was introduced to solve
> exactly the same reason problem that made Francis start this thread
> -- to avoid re-applying patches that are already in $onto.

Not quite; I wrote it because at the time, there was no way to
transplant git history onto a git-svn "make empty subdir" commit for
later dcommitting.  So the main point was really

> After my patches (that are not yet ready) that calculate revisions as
> "git cherry $onto $orig_head $upstream", I don't think there should be
> any need for the --root flag. The only place the flag is checked now
> (in my current work tree, that is) is when deciding how to interpret
> the remaining arguments. Although I have not tried (temporarily)
> rewriting all the test cases from
> 
>   git rebase --root --onto upstream branch
> 
> to
> 
>   git rebase upstream branch
> 
> , I think it should work. What do you think, Thomas?

I still think it would be natural for a user to want a way to say "all
the way back to the root commit".  At least for me the "full" rebase
invocation is

  git rebase --onto onto base branch

In that mindset it comes natural to replace 'base' with --root if you
want to go all the way.

Maybe I don't trust rebase enough to do what I want ;-)

> I saw that
> "--root" is also passed to the hook. Should that value be passed to
> the hook also when the old base is not explicitly a root (by "rebase
> --root"), but only implicitly so (by an $onto that is disjoint from
> $branch)?

I think I did it that way because if you use --root, the base/upstream
argument is missing, and the hook needs to know that.

If the user specifies an upstream that is disjoint from the branch
itself, the hook gets the upstream argument and can presumably work it
out from there.  So you could perhaps save the hook some trouble if
you *know* that it's a disjoint rebase, but I wouldn't spend too much
time on that.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: git-rebase skips automatically no more needed commits

[Martin von Zweigbergk]

On Mon, Sep 26, 2011 at 5:10 AM, Thomas Rast <trast@student.ethz.ch> wrote:
> Martin von Zweigbergk wrote:
>>
>> If I understand correctly, [rebase --root] was introduced to solve
>> exactly the same reason problem that made Francis start this thread
>> -- to avoid re-applying patches that are already in $onto.
>
> Not quite; I wrote it because at the time, there was no way to
> transplant git history onto a git-svn "make empty subdir" commit for
> later dcommitting.  So the main point was really

But that was only due to the bug/limitation in format-patch that you
(?) fixed a few days later, no?

> I still think it would be natural for a user to want a way to say "all
> the way back to the root commit".  At least for me the "full" rebase
> invocation is
>
>  git rebase --onto onto base branch

Sure, I can understand that. Still,  it would just be an alternative
syntax and nothing else. So I think I should also update the
documentation after my other patches to make it clear that that is the
case. We also have to think about backwards compatibility, so I'm
obviously not suggesting that we should stop supporting the flag; I
just wanted to make sure that it is not technically needed.

>> I saw that
>> "--root" is also passed to the hook. Should that value be passed to
>> the hook also when the old base is not explicitly a root (by "rebase
>> --root"), but only implicitly so (by an $onto that is disjoint from
>> $branch)?
>
> I think I did it that way because if you use --root, the base/upstream
> argument is missing, and the hook needs to know that.
>
> If the user specifies an upstream that is disjoint from the branch
> itself, the hook gets the upstream argument and can presumably work it
> out from there.  So you could perhaps save the hook some trouble if
> you *know* that it's a disjoint rebase, but I wouldn't spend too much
> time on that.

The hook would still have to be able to handle both cases (i.e.
getting the upstream argument as "--root" or simply a commit that
happens to be disjoint from the the branch-to-be-rebased). I believe
that has been the case since that bug in format-patch was fixed. So if
we were to change git-rebase so it no longer passes the --root flag to
the hook, I think any (correctly written) hooks should still work. I
don't see much reason to change it, though; if the user uses the
--root syntax, we can pass the flag, otherwise we don't.

Martin