New issue
Advanced search Search tips

Issue 723840 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Gerrit: Unable to break patch dependency chain

Project Member Reported by danakj@chromium.org, May 17 2017

Issue description

I have a branch dependent on another, both uploaded to gerrit.

I closed the second moved it to rietveld with a new git branch, rebased the first on top of the new branch, and changed its parent branch locally to origin/master.

When I upload it with git cl upload HEAD^, it still tries to update the closed parent gerrit CL, with the accompanying errors, and lists it as a dependency.
 

Comment 1 by aga...@chromium.org, May 17 2017

Cc: aga...@chromium.org
Owner: ----
I don't follow the sequence of events you describe. You created a new branch containing a standalone version of what had been your dependent change, then rebased the first change on top of that (changing their dependency order, cool), and then set its upstream to origin/master? That last bit doesn't make sense. Can you provide a facsimile of the commands you ran so I can understand better?

Comment 2 by danakj@chromium.org, May 18 2017

git config alias.co checkout

#make gerrit branch and upload
git co -tb togerrit origin/master
git commit -a -m togerrit
git cl upload --gerrit

#make child gerrit branch and upload
git co -tb togerrit2 togerrit
git commit -a -m togerrit2
git cl upload --gerrit  #now depends on the first

... close the first togerrit issue ...

#disconect first from gerrit
git co togerrit
git cl issue 0

#make non-gerrit version of first branch
git co -b torietveld togerrit
git cl upload --rietveld

#rebase 2nd branch over
git co togerrit2
git rebase --onto torietveld HEAD^

#change upstream to origin/master to try break dependency on the gerrit branch
git branch -u origin/master

#try update this branch to gerrit
git commit -a --amend -C HEAD
git cl upload --gerrit

#see errors cuz it's still trying to treat 'togerrit' as a parent CL somehow.

Comment 3 by aga...@chromium.org, May 18 2017

Thanks! Yeah ok good I wasn't misunderstanding. There are a couple things happening here.

First, we don't currently support a workflow where, in a single chain of dependent branches, some are uploaded to Rietveld and others are uploaded to Gerrit. This was pointed out in early PSAs, although I now realize that the warning was omitted from the most recent round of PSAs.

Second, why do you rebase tog2 onto torietveld? Because of the way you created torietveld, it contains the exact same commit object as togerrit did and the rebase will do nothing except update the commit timestamp of togerrit2.

Third, why are you setting the upstream of togerrit2 as 'origin/master', when you just rebased it on top of torietveld? You should set its upstream to torietveld, so that dependencies can be correctly tracked by either codereview system. 

Finally, when I follow your steps exactly as outlined above, I don't get errors at the end. It results in https://chromium-review.googlesource.com/c/508476/, which has as its first patchset just my second whitespace change (bottom of file), and has as its second patchset the squashed version of both my first and second whitespace changes (top and bottom of file).

Starting from the point where you close the first gerrit issue, I'd suggest a workflow more like this:
git co togerrit
git cl issue 0
# optional: git rename-branch torietveld
git cl upload --rietveld
git co togerrit2
git reparent-branch --root
git cl upload --gerrit

The rename-branch and reparent-branch commands are documented here (https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools.html) or with `man depot_tools`.
Cc: -aga...@chromium.org
Owner: aga...@chromium.org
Status: Assigned (was: Untriaged)

Comment 5 by danakj@chromium.org, May 18 2017

> First, we don't currently support a workflow where, in a single chain of dependent branches, some are uploaded to Rietveld and others are uploaded to Gerrit.

I didn't expect it to, which is why I was trying to tell it that my dependent CL should be treated as independent (git branch -u origin/master).

> Second, why do you rebase tog2 onto torietveld? Because of the way you created torietveld, it contains the exact same commit object as togerrit did and the rebase will do nothing except update the commit timestamp of togerrit2.

I was trying to give it a different upstream branch so it would not try to deal with gerrit, and just be a disconnected patch on its own.

> Third, why are you setting the upstream of togerrit2 as 'origin/master', when you just rebased it on top of torietveld? You should set its upstream to torietveld, so that dependencies can be correctly tracked by either codereview system. 

I didn't think it would understand a branch that has git cl issue report a rietveld issue as the upstream of a gerrit branch, so I tried to make it an independent CL without a parent, and specify HEAD^ to define the diff.

> and has as its second patchset the squashed version of both my first and second whitespace changes (top and bottom of file)

I was doing "git cl upload HEAD^" on the child branch to not include the parent diff. But for you it's not trying to use the old gerrit parent anymore?

Comment 6 by aga...@chromium.org, May 18 2017

> I was trying to give it a different upstream branch so it would not try to deal with gerrit, and just be a disconnected patch on its own.

This is exactly what "git reparent-branch" is for. The method you tried (rebasing on top of another branch, and then setting the upstream to origin/master) doesn't do what you were trying to do, which is what ultimately led to the failure.

Comment 7 by danakj@chromium.org, May 18 2017

Status: Fixed (was: Assigned)
oh i did not know about git reparent-branch. I learnt branch --upstream at some point in the recent past for working with rietveld. OK thanks, I will have to work that into my memory.

Sign in to add a comment