automagically determine default branch without needing per-branch defaults in .gitreview

[Anne Gentle]

Here's what I did:
git checkout master
git pull origin master
git checkout -b BRANCHNAME

...

git commit -a -m 'message'
git review
git log

...copy/paste the sha1 of the revision...

git checkout gerrit/stable/diablo
git checkout -b ANOTHERBRANCHNAME
git cherry-pick sha1pastevalue
... fix a merge conflict if needed,
grep -r "<<<<<<< HEAD" .
...
git add .
git commit -c sha1nnnnnnnnnnnnn
git review

Here's what I got:

Errors running GIT_EDITOR=true git rebase -i remotes/gerrit/master
Finished one cherry-pick.
# Not currently on any branch.
nothing to commit (working directory clean)
Could not apply 3688cad... Fixes bug 882781 multi_host flag addition

[Jay Pipes (jaypipes)]

When a merge conflict occurs with a rebasing (like what happened above with your cherry-pick), you need to do a git rebase --continue instead of a git commit -c <BLAH>

[Anne Gentle (annegentle)]

Fascinating. Now when I went to work on a branch based off of master, it still complained that there was already an interactive rebase already started. I did the git rebase --continue and then git review and managed to get two new changes ready for review. I'm not sure that was my intent but I'm reporting back what happened. Here's a paste.

M60L7XJ:openstack-manuals anne.gentle$ git commit -a -m 'Adds xen example nova conf file and references hypervisor chapter for configuration'
[xenserver c83c90b] Adds xen example nova conf file and references hypervisor chapter for configuration
 2 files changed, 3 insertions(+), 21 deletions(-)
M60L7XJ:openstack-manuals anne.gentle$ git review
Errors running GIT_EDITOR=true git rebase -i remotes/gerrit/master
Interactive rebase already started
M60L7XJ:openstack-manuals anne.gentle$ git rebase --continue
Successfully rebased and updated refs/heads/newcherrypick.
M60L7XJ:openstack-manuals anne.gentle$ git review
remote: Resolving deltas: 0% (0/10)
remote: (W) a2d5eae: commit subject >50 characters; use shorter first paragraph
remote: (W) a2d5eae: commit message lines >70 characters; manually wrap lines
remote:
remote: New Changes:
remote: https://review.openstack.org/1264
remote: https://review.openstack.org/1265
remote:
To ssh://<email address hidden>:29418/openstack/openstack-manuals.git
 * [new branch] HEAD -> refs/for/master/newcherrypick

It's as if the git review step took my branch based on master and my branch based on stable/diablo and committed them both at once?

Both changes are fine, but one is dependent on the other.

How can I learn more about what git review does precisely so I can untangle this?

[Monty Taylor (mordred)]

If you run git-review -v it will, amongst other things, output every git command that it runs.

[reedy (tehreedy)]

Currently, git review cannot be used on branches.

Which means only people with core review access can commit directly to those branches.

As ialex brought up, there is no way to request the merges to the branches, other than asking a core review to cherry pick and push.

So this brings 2 points; We have a bunch of core committers who probably should be allowed to commit to these branches. Maybe some of these should also have review privileges for core too, but that's a different discussion.

Also, we need a way to people to request pulls into branches. Having git-review work would be nice, as would pressing a button in gerrit. Going back to having to add requests to wikipages etc is going in the wrong direction.

[Marcin-cieslak (marcin-cieslak)]

The magic part is "refs/for/REL1_19" insted of "refs/for/master"

Here's what I did:

$ git checkout gerrit/REL1_19
(...)
$ git branch REL1_19
$ echo .... >> RELEASE-NOTES-1.19
$ git add RELEASE-NOTES-1.19
$ git commit -m "Testing for bug 35456"
[detached HEAD f4b682a] Testing for bug 35456
 1 files changed, 1 insertions(+), 0 deletions(-)
$ git push gerrit HEAD:refs/for/REL1_19
Counting objects: 5, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 371 bytes, done.
Total 3 (delta 2), reused 0 (delta 0)
remote: Resolving deltas: 0% (0/2)
remote:
remote: New Changes:
remote: https://gerrit.wikimedia.org/r/3688
remote:
To ssh://saper@review:29418/mediawiki/core.git
 * [new branch] HEAD -> refs/for/REL1_19

So we have gerrit change 3688

[reedy (tehreedy)]

<gerrit-wm> New patchset: saper; "Testing for bug 35456" [mediawiki/core] (REL1_19) - https://gerrit.wikimedia.org/r/3688
<Reedy> lol wut
<saper> git-review is too magic to me
<Reedy> git-review wouldn't work for me when I tried it earlier this week...
<saper> it wasn't git-review, that's why it worked
<Reedy> Did you use git push origin REL1_1X ?
<saper> Reedy: refs/for/REL1_19
<Reedy> Hmmm
<Reedy> saper: guess it should be at least documented :p
<saper> Reedy: it is documented very well
<saper> in gerrit docs
<Reedy> Lol
<saper> but we give workarounds

[Marcin Cieślak (saperski)]

Seems you have to use -R since otherwise git-review will attempt to rebase your changes to whatever is specified in the default branch in .gitreview.

Added link to https://bugzilla.wikimedia.org/show_bug.cgi?id=35456

[Marcin-cieslak (marcin-cieslak)]

It seems that you can use -R to tell git-review not to rebase against .gitreview's idea of what the current branch is.

Added link to the upstream bug (https://bugs.launchpad.net/mediawiki/+bug/883176)

[Antoine "hashar" Musso (hashar)]

git-review accepts as an optional argument the branch name to interact with. When that argument is not specified, it fallback to look for the defaultbranch parameter in the .gitreview file.

Every branch should have a .gitreview having a correct defaultbranch value. We probably do not want to force people to use something like: `git-review REL1_19`

Output example using test/mediawiki/core and a .gitreview file NOT having default branch set:

$ git-review -v REL1_19
2012-03-26 17:57:30.318755 Running: git log --color=never --oneline HEAD^1..HEAD
2012-03-26 17:57:30.335452 Running: git remote
2012-03-26 17:57:30.340266 Running: git branch -a --color=never
2012-03-26 17:57:30.346184 Running: git branch --color=never
2012-03-26 17:57:30.351537 Running: git log HEAD^1..HEAD
Found topic 'REL1_19' from parsing changes.
2012-03-26 17:57:30.372507 Running: git rev-parse --show-toplevel
2012-03-26 17:57:30.377682 Running: git remote update gerrit
Fetching gerrit
2012-03-26 17:57:32.224581 Running: git rebase -i remotes/gerrit/REL1_19
2012-03-26 17:57:32.505708 Running: git config --get color.ui
2012-03-26 17:57:32.510419 Running: git log --color=always --decorate --oneline REL1_19 --not remotes/gerrit/REL1_19
2012-03-26 17:57:32.531614 Running: git push gerrit HEAD:refs/for/REL1_19/REL1_19
remote: Resolving deltas: 0% (0/2)
remote:
remote: New Changes:
remote: https://gerrit.wikimedia.org/r/3742
remote:
To ssh://<email address hidden>:29418/test/mediawiki/core.git
 * [new branch] HEAD -> refs/for/REL1_19/REL1_19
$

Now with a defaultbranch specified:

$ git-review -v
2012-03-26 17:59:01.654284 Running: git log --color=never --oneline HEAD^1..HEAD
2012-03-26 17:59:01.671722 Running: git remote
2012-03-26 17:59:01.678109 Running: git branch -a --color=never
2012-03-26 17:59:01.684509 Running: git branch --color=never
2012-03-26 17:59:01.690706 Running: git log HEAD^1..HEAD
Found topic 'REL1_19' from parsing changes.
2012-03-26 17:59:01.711494 Running: git rev-parse --show-toplevel
2012-03-26 17:59:01.716346 Running: git remote update gerrit
Fetching gerrit
2012-03-26 17:59:03.550708 Running: git rebase -i remotes/gerrit/REL1_19
2012-03-26 17:59:03.848351 Running: git config --get color.ui
2012-03-26 17:59:03.853355 Running: git log --color=always --decorate --oneline REL1_19 --not remotes/gerrit/REL1_19
You have more than one commit that you are about to submit.
The outstanding commits are:

598114c (HEAD, REL1_19) yeah defaultbranch rocks
22570d4 yah testing file for confirmation

Is this really what you meant to do?
Type 'yes' to confirm: yes
2012-03-26 17:59:07.938479 Running: git push gerrit HEAD:refs/for/REL1_19/REL1_19
remote: Resolving deltas: 0% (0/2)
remote:
remote: New Changes:
remote: https://gerrit.wikimedia.org/r/3743
remote:
To ssh://<email address hidden>:29418/test/mediawiki/core.git
 * [new branch] HEAD -> refs/for/REL1_19/REL1_19
$

Thus, I am marking this issue as work for me

[Antoine "hashar" Musso (hashar)]

I have sent .gitreview files for mediawiki/core.git with changes :

REL1_19 https://gerrit.wikimedia.org/r/3744
REL1_18 https://gerrit.wikimedia.org/r/3745
REL1_17 https://gerrit.wikimedia.org/r/3746

[reedy (tehreedy)]

(In reply to comment #5)
> I have sent .gitreview files for mediawiki/core.git with changes :
>
> REL1_19 https://gerrit.wikimedia.org/r/3744
> REL1_18 https://gerrit.wikimedia.org/r/3745
> REL1_17 https://gerrit.wikimedia.org/r/3746

Great thanks!

It did seem that just getting some clarification and updating the documentation would've been enough to suffice dealing with this bug :)

[Antoine "hashar" Musso (hashar)]

Basic doc updated at : https://www.mediawiki.org/wiki/Git/Workflow#acting_on_remote_branches

[Marcin-cieslak (marcin-cieslak)]

I think we should work towards git-review without the need of .gitreview file at all. There is a work underway (https://review.openstack.org/#change,5720) to not need .gitreview at all to determine repository access and derive it fully from existing git remotes as well as using any remote (https://review.openstack.org/#change,5784) instead of hardcoded "gerrit".

I think it is also possible to get branch name out of gerrit metadata (at least when subsequent changes are submitted, but for the first change it might probably be derived from HEAD).

[Michael Johnson (michael-johnson)]

See two issues in git-review's storyboard. One is forked from this bug:
https://storyboard.openstack.org/#!/story/883176
The other is a newly-created issue:
https://storyboard.openstack.org/#!/story/2000176
Perhaps they should have been only one after all; the second was created due to discussions on IRC before I fully understood that they needed to be handled together.

In any case, my implementation of a feature that I believe resolves the two together is currently in review:
https://review.openstack.org/#/c/158877/