Merging a devel branch with aosp

[Brad Larson]

I want to make sure we are handling this situation correctly...

Let's start with a diagram (and hope it doesn't garble too badly):

C* - - - - - - - - -H*
/ /
A - B - C - D - E - F - G - H

The master branch from a.g.k.o starts at A. We branched to an
internal development branch for a change C*. Now we want to merge
other changes made in master so we are back up to tips (H*).

We have pulled up by calling 'git-merge H' while on our development
branch. However, if we repo upload, it wants to push changes D-H
(actually something like 27 changes in our case). Can a merge like
this go through repo, or does it need to be git-pushed underneath
gerrit? (following the user-upload docs). We are comfortable doing
the git push, but would prefer if all changes/merges went through
gerrit and the code-review process. What is the best way to handle
this situation?

Thanks!
-Brad

[Shawn Pearce]

Let me be sure I understand you correctly.

Your Gerrit says "master" is currently at A.  E.g. a

  git ls-remote your-gerrit:project.git refs/heads/master

outputs "A".

Your client has gone and merged H from the AOSP, in addition to creating C* and H*.

You only want to code review C* and H* on your internal Gerrit.


To get this to work, you need to enable push access on the project, and push H to master directly, then "repo sync" or at least "git fetch" to update the tracking branch.  repo upload will then show only C*, H* as being uploaded.

But, this only works the first time.  The next time you try to update from AOSP "master" is pointing at H* (or something further down the line) and you can't directly push P from the AOSP because P isn't a strict superset of H*.


The best way around this is to create a pristine master branch that only contains the AOSP commits.  Go into Gerrit web UI and create branch named "aosp/master" with revision A.  Then push H into aosp/master directly.

Now when you repo upload H* into master, Gerrit will only make changes for C*, H*, but master won't get the D-H until H* is submitted.  The reason this works is, Gerrit makes a change record only for the first time a commit enters the repository.  A-H are already visible in the repository through asop/master, and A is visible through master, so only C*,H* need changes to be reviewed.

On future updates from AOSP, push AOSP into asop/master, then merge it on a client, and upload the merge to master.  Rinse, repeat.


Did that make any sense?

[Brad Larson]

Not quite... sorry I wasn't as clear as I'd hoped.

On Mar 11, 4:35 pm, Shawn Pearce <s...@google.com> wrote:

> On Wed, Mar 11, 2009 at 14:23, Brad Larson <bklar...@gmail.com> wrote:
>
> > I want to make sure we are handling this situation correctly...
>
> > Let's start with a diagram (and hope it doesn't garble too badly):
>
> >          C* - - - - - - - - -H*
> >         /                   /
> > A - B - C - D - E - F - G - H
>
> > The master branch from a.g.k.o starts at A.  We branched to an
> > internal development branch for a change C*.  Now we want to merge
> > other changes made in master so we are back up to tips (H*).
>
> > We have pulled up by calling 'git-merge H' while on our development
> > branch.  However, if we repo upload, it wants to push changes D-H
> > (actually something like 27 changes in our case).  Can a merge like
> > this go through repo, or does it need to be git-pushed underneath
> > gerrit? (following the user-upload docs).  We are comfortable doing
> > the git push, but would prefer if all changes/merges went through
> > gerrit and the code-review process.  What is the best way to handle
> > this situation?
>
> Let me be sure I understand you correctly.
>
> Your Gerrit says "master" is currently at A.  E.g. a
>
>   git ls-remote your-gerrit:project.git refs/heads/master
>
> outputs "A".

No, it outputs "H". Our "master" branch is a pristine copy of AOSP/
master. Our changes are only done in a devel branch.

>
> Your client has gone and merged H from the AOSP, in addition to creating C*
> and H*.
>
> You only want to code review C* and H* on your internal Gerrit.

Actually, our only change is C*. I meant H* to mean the merge of C*
and H. We haven't introduced any changes since C* and just want to
merge in changes from upstream (D - H).

>
> To get this to work, you need to enable push access on the project, and push
> H to master directly, then "repo sync" or at least "git fetch" to update the
> tracking branch.  repo upload will then show only C*, H* as being uploaded.
>
> But, this only works the first time.  The next time you try to update from
> AOSP "master" is pointing at H* (or something further down the line) and you
> can't directly push P from the AOSP because P isn't a strict superset of H*.
>
> The best way around this is to create a pristine master branch that only
> contains the AOSP commits.  Go into Gerrit web UI and create branch named
> "aosp/master" with revision A.  Then push H into aosp/master directly.
>
> Now when you repo upload H* into master, Gerrit will only make changes for
> C*, H*, but master won't get the D-H until H* is submitted.  The reason this
> works is, Gerrit makes a change record only for the first time a commit
> enters the repository.  A-H are already visible in the repository through
> asop/master, and A is visible through master, so only C*,H* need changes to
> be reviewed.

We have already reviewed and merged C*. Now the developer has merged
D-H in his sandbox. If he runs 'repo upload', it says that he has 5
commits to merge, one for each of D-H. If he continues with the
merge, will it create 5 changes in Gerrit? That is what we are trying
to avoid. We would like to see 1 change in Gerrit for the merge of C*
with H (or, as a fallback, no changes in Gerrit and the merge to be
done behind the scenes).

>
> On future updates from AOSP, push AOSP into asop/master, then merge it on a
> client, and upload the merge to master.  Rinse, repeat.

This is basically what we are doing... now we need to merge from asop/
master to us/devel.

Thanks!
-Brad

[Shawn Pearce]

On Wed, Mar 11, 2009 at 14:54, Brad Larson <bkla...@gmail.com> wrote:

We have already reviewed and merged C*.  Now the developer has merged
D-H in his sandbox.  If he runs 'repo upload', it says that he has 5
commits to merge, one for each of D-H.  If he continues with the
merge, will it create 5 changes in Gerrit?  That is what we are trying
to avoid.  We would like to see 1 change in Gerrit for the merge of C*
with H (or, as a fallback, no changes in Gerrit and the merge to be
done behind the scenes).

repo upload is just not seeing the full picture like Gerrit does.

Do the upload.  You'll only get H* as a change to review.

[jesse]

Hi Shawn,

I'm working with Brad on merging in these changes. I followed the
instructions you described here and almost everything worked like a
charm. However, I ran into trouble uploading the changes for one
project. On the upload, it's complaining that the remote rejected it
because of "(invalid committer j...@google.com)".

We started with uploading all of the latest changes from ASOP to
gerrit (bypassing review), so I think that whatever commit it's
complaining about should already be in the repository.

Do you have any suggestions as to what might be going wrong?

Thanks,
Jesse

On Mar 11, 4:57 pm, Shawn Pearce <s...@google.com> wrote:

[Shawn Pearce]

On Wed, Mar 11, 2009 at 16:34, jesse <jesse.g...@gmail.com> wrote:

I'm working with Brad on merging in these changes.  I followed the
instructions you described here and almost everything worked like a
charm.  However, I ran into trouble uploading the changes for one
project.  On the upload, it's complaining that the remote rejected it
because of "(invalid committer j...@google.com)".

We started with uploading all of the latest changes from ASOP to
gerrit (bypassing review), so I think that whatever commit it's
complaining about should already be in the repository.

Do you have any suggestions as to what might be going wrong?

That commit by jbq isn't in that repository.  That, or there's a bug in JGit's commit walking loop.

- Make sure you actually did push (bypassing review) the AOSP changes in this project.  You may have just missed it.

- If the push was already done and isn't the problem, humor me by restarting Gerrit to see if that fixes the problem.

- After that, I would think it means there is a bug in JGit.  I'd need the topology at least in order to recreate it here and debug it.  Easier though if you could just share the repository with me.

[jesse]

On Mar 11, 6:42 pm, Shawn Pearce <s...@google.com> wrote:
> That commit by jbq isn't in that repository.  That, or there's a bug in
> JGit's commit walking loop.
>
> - Make sure you actually did push (bypassing review) the AOSP changes in
> this project.  You may have just missed it.

I'm fairly certain that it is in there. We have a script that goes
through all of the AOSP projects and pulls refs/heads/* from them and
pushes those heads into refs/heads/* on our server. This is done
using the bypass mechanism.

To pull the new AOSP changes into our development branch, I make a
fresh checkout from repo and do the merge there. So, the only new
revision should be the merge.

> - If the push was already done and isn't the problem, humor me by restarting
> Gerrit to see if that fixes the problem.

We did try that, but I still ran into trouble.

> - After that, I would think it means there is a bug in JGit.  I'd need the
> topology at least in order to recreate it here and debug it.  Easier though
> if you could just share the repository with me.

Unfortunately, we can not share the repository. Is there some way to
export just the topology (other than taking a screen shot of gitk)?

Thanks,
Jesse

[Shawn Pearce]

On Thu, Mar 12, 2009 at 07:57, jesse <jesse.g...@gmail.com> wrote:

> - After that, I would think it means there is a bug in JGit.  I'd need the
> topology at least in order to recreate it here and debug it.  Easier though
> if you could just share the repository with me.

Unfortunately, we can not share the repository.  Is there some way to
export just the topology (other than taking a screen shot of gitk)?

On server:
  git for-each-ref >server.refs
  git log --all '--pretty=format:%H %ct %P'  >server.graph

On client:
  git for-each-ref >client.refs
  git log --all '--pretty=format:%H %ct %P'  >client.graph
  git rev-parse HEAD >client.HEAD

Zip and send those 4 files.  The graph files only contain the commit SHA-1s and commit time stamps, which is the only data that matters in the graph traversal algorithm.  I can use that to create an identical graph here, but with different SHA-1s (since I can't recreate the objects).  But first I'll have to write a tool to recreate a graph.  :-)

The ref files contain your branch and tag names.  Please review them by hand and adjust ref names if you need to if there might be an information leak.  But if you do edit them, please make sure you are consistent in your edit on both server.refs and client.refs.  :-)

I also need to know:

 - which branch you are uploading into
 - which AOSP project this is, if you can tell me.

Send all of the above to me by private email, I'll dig through and see what I can turn up.

Thanks.

[Shawn Pearce]

On Thu, Mar 12, 2009 at 08:11, Shawn Pearce <s...@google.com> wrote:
>
> On Thu, Mar 12, 2009 at 07:57, jesse <jesse.g...@gmail.com> wrote:
>>
>> > - After that, I would think it means there is a bug in JGit.  I'd need the
>> > topology at least in order to recreate it here and debug it.  Easier though
>> > if you could just share the repository with me.
>

> Send all of the above to me by private email, I'll dig through and see what I can turn up.

I spent the better part of this morning digging through Jesse's DAG.  Someone else also reported a similar problem to me by private mail in a different repository.  I have determined this really is a bug in JGit, or at least Gerrit's use of it.

What's happening is given some commit graph such as:

 Z-A------------B
 |   +---------/
 \   /
  Q-R-S-T-U-V

If "V" is master and on the server, and A is "development" and on the server, and we try to repo upload "B", which is a merge of "A" and "R", Gerrit fails, whining about "R" not having a proper committer string for the caller.

The problem is, JGit isn't coloring the commit "R" as reachable from "V", because it processed and emitted "R" before it evaluated the T-S-R chain.  This is due to a performance optimization, where JGit is only lazily parsing the V-Z chain, trying to avoid scanning things it may not need.

There is a common hack used in the Git world where commit timestamps can be used to create a loose ordering, ensuring that the V-R chain will be evaluated before the B-R chain.  JGit doesn't enable this by default, and Gerrit doesn't enable it either.  Of course the observant reader will recognize that commit timestamps aren't enough to always ensure this works, due to clock skew on distributed systems.  The Git would has mostly said its "good enough", but accurate reporting is really required here.

Oddly enough, Gerrit enables a topological sorting, which is supposed to ensure R cannot be produced until V-S has been considered, but R doesn't learn about its descendant S until too late, again, because of the same optimization to avoid long chains we may not care about.


I'm trying to decide how I want to fix it.  A close first approximation is to enable the commit timestamp sorting.  But there are still scenarios where it can make this mistake.

[Shawn Pearce]

On Thu, Mar 12, 2009 at 11:48, Shawn Pearce <s...@google.com> wrote:

On Thu, Mar 12, 2009 at 08:11, Shawn Pearce <s...@google.com> wrote:
>
> On Thu, Mar 12, 2009 at 07:57, jesse <jesse.g...@gmail.com> wrote:
>>
>> > - After that, I would think it means there is a bug in JGit.  I'd need the
>> > topology at least in order to recreate it here and debug it.  Easier though
>> > if you could just share the repository with me.
>

> Send all of the above to me by private email, I'll dig through and see what I can turn up.

I spent the better part of this morning digging through Jesse's DAG.  Someone else also reported a similar problem to me by private mail in a different repository.  I have determined this really is a bug in JGit, or at least Gerrit's use of it.

This appears to be fixed in Gerrit 2.0.7.   If its still causing you trouble, consider upgrading.