Page MenuHomePhabricator

Remove .gitreview from MediaWiki and Extensions
Closed, ResolvedPublic

Description

Tweaking the .gitreview file is the most intensive part of make-wmf-branch.

Dropping the .gitreview file would be an ideal solution—allowing us to make the branch in gerrit; however it may result in unexpected behavior.


Since git-review version ̀1.25.0, it set up the tracking branch when a downloading a change and can be instructed to default to the tracked branch when sending a patch. Then:

  • In .git-review replace defaultbranch = xxxx with track =1
  • Get developers to use git-review 1.25.0 or later
  • Eventually get people to make that a global option with: git config --global --add gitreview.track 1

We will want to update doc and announce the requirement change with instruction to upgrade. See for T146293#2658764 details.

Details

Related Gerrit Patches:
mediawiki/extensions/Whoops : masterSwitch .gitreview from defaultbranch to track
mediawiki/tools/release : masterStop mangling .gitreview
mediawiki/extensions/VisualEditor : master.gitreview: swapping defaultbranch for track
mediawiki/extensions/Validator : master.gitreview: swapping defaultbranch for track
mediawiki/extensions/RandomImageByCategory : master.gitreview: swapping defaultbranch for track
mediawiki/extensions/DonationInterface : master.gitreview: swapping defaultbranch for track
mediawiki/extensions/CategoryTagSorter : master.gitreview: swapping defaultbranch for track
mediawiki/skins/Metrolook : masterWhoops, track not trace
mediawiki/core : masterSwapping defaultbranch for track
mediawiki/vendor : masterSwapping defaultbranch for track
mediawiki/skins : masterSwapping defaultbranch for track
mediawiki/extensions : masterSwapping defaultbranch for track
mediawiki/skins/Vector : masterSwapping defaultbranch for trace
mediawiki/skins/Truglass : masterSwapping defaultbranch for trace
mediawiki/skins/Tomas : masterSwapping defaultbranch for trace
mediawiki/skins/Timeless : masterSwapping defaultbranch for trace
mediawiki/skins/Tempo : masterSwapping defaultbranch for trace
mediawiki/skins/Synagonism : masterSwapping defaultbranch for trace
mediawiki/skins/Splash : masterSwapping defaultbranch for trace
mediawiki/skins/Schulenburg : masterSwapping defaultbranch for trace
mediawiki/skins/Refreshed : masterSwapping defaultbranch for trace
mediawiki/skins/Nostalgia : masterSwapping defaultbranch for trace
mediawiki/skins/Nimbus : masterSwapping defaultbranch for trace
mediawiki/skins/MonoBook : masterSwapping defaultbranch for trace
mediawiki/skins/Modern : masterSwapping defaultbranch for trace
mediawiki/skins/MinervaNeue : masterSwapping defaultbranch for trace
mediawiki/skins/Mask : masterSwapping defaultbranch for trace
mediawiki/skins/HasSomeColours : masterSwapping defaultbranch for trace
mediawiki/skins/GreyStuff : masterSwapping defaultbranch for trace
mediawiki/skins/Gamepress : masterSwapping defaultbranch for trace
mediawiki/skins/Example : masterSwapping defaultbranch for trace
mediawiki/skins/Empty : masterSwapping defaultbranch for trace
mediawiki/skins/DuskToDawn : masterSwapping defaultbranch for trace
mediawiki/skins/Dusk : masterSwapping defaultbranch for trace
mediawiki/skins/Donate : masterSwapping defaultbranch for trace
mediawiki/skins/DeskMessMirrored : masterSwapping defaultbranch for trace
mediawiki/skins/DeepSea : masterSwapping defaultbranch for trace
mediawiki/skins/Daddio : masterSwapping defaultbranch for trace
mediawiki/skins/CustomPage : masterSwapping defaultbranch for trace
mediawiki/skins/CologneBlue : masterSwapping defaultbranch for trace
mediawiki/skins/Bouquet : masterSwapping defaultbranch for trace
mediawiki/skins/Blueprint : masterSwapping defaultbranch for trace
mediawiki/skins/BlueSpiceSkin : masterSwapping defaultbranch for trace
mediawiki/skins/BlueSky : masterSwapping defaultbranch for trace
mediawiki/skins/VectorV2 : masterSwapping defaultbranch for trace
mediawiki/skins/WPtouch : masterSwapping defaultbranch for trace
mediawiki/skins/WoOgLeShades : masterSwapping defaultbranch for trace
mediawiki/skins/apex : masterSwapping defaultbranch for trace
mediawiki/skins/erudite : masterSwapping defaultbranch for trace
mediawiki/skins/mediawiki-strapping : masterSwapping defaultbranch for trace
mediawiki/skins/p2wiki : masterSwapping defaultbranch for trace
mediawiki/skins/webplatform : masterSwapping defaultbranch for trace
mediawiki/skins/Metrolook : masterSwapping defaultbranch for trace

Related Objects

Mentioned In
rESRD739a0e0244f6: Swapping defaultbranch for trace
rEWOPbc5717d728b0: Switch .gitreview from defaultbranch to track
rESSa4bab2cdae8b: Swapping defaultbranch for trace
rEPPdf4bc82e03bc: Swapping defaultbranch for trace
rEEDIa0c3fa36ed8d: Swapping defaultbranch for trace
rEMEM7c5bc0f1eabe: Swapping defaultbranch for trace
rEBHK0d6ddbc0b77d: Swapping defaultbranch for trace
R1899:53c6e13f0149: Swapping defaultbranch for trace
rETCCb35302936f62: Swapping defaultbranch for trace
rEPVJce7bf487d265: Swapping defaultbranch for trace
rEENFc6a77d258ada: Swapping defaultbranch for trace
rSEXAMPLE776f582d6ef0: Swapping defaultbranch for trace
rENUAC3d932b68a658: Swapping defaultbranch for trace
rEMET3bf332241226: Swapping defaultbranch for trace
rECOS97a6419a35d8: Swapping defaultbranch for trace
R1898:c1f2658f90d1: Swapping defaultbranch for trace
rEPCE88862b07c745: Swapping defaultbranch for trace
rEWBI113344601f9d: Swapping defaultbranch for trace
rEARAb0c730e9cec4: Swapping defaultbranch for trace
rERECa1bd3327f286: Swapping defaultbranch for trace
rENTF341ad6db20a2: Swapping defaultbranch for trace
R1896:71803320b737: Swapping defaultbranch for trace
rEPTQf90a35bdd117: Swapping defaultbranch for trace
R1902:44ef4026ee78: Swapping defaultbranch for trace
rESHH4a5918131490: Swapping defaultbranch for trace
rEIWSf3f00805ebec: Swapping defaultbranch for trace
rEUPEfeb4e4155e86: Swapping defaultbranch for trace
rEBSMfecb64157079: Swapping defaultbranch for trace
rEMWF4f90c4544d97: Swapping defaultbranch for trace
rEGCN41037c569120: Swapping defaultbranch for trace
rEQS6526cf32323f: Swapping defaultbranch for trace
rEWISdcb802eb1459: Swapping defaultbranch for trace
rEBOP09116665eb15: Swapping defaultbranch for trace
rEMWVcfecccec2ee6: Swapping defaultbranch for trace
R1894:c657d9fe82de: Swapping defaultbranch for trace
rEURDb4c72e6663fe: Swapping defaultbranch for trace
rECW8e979282ea2c: Swapping defaultbranch for trace
rEQGV6a7ca83bdb46: Swapping defaultbranch for trace
rEBSENC15d123566258: Swapping defaultbranch for trace
rEATHfbd12361cbee: Swapping defaultbranch for trace
rEXFA21b20d4ac126: Swapping defaultbranch for trace
rEWLE3570b836ced6: Swapping defaultbranch for trace
rEWPI346c04de6e46: Swapping defaultbranch for trace
rEWPE2fea9ca740ec: Swapping defaultbranch for trace
rEULKd2c81a997974: Swapping defaultbranch for trace
rESTLH09b2892e9cd4: Swapping defaultbranch for trace
rESCC06f1ba802f4e: Swapping defaultbranch for trace
rELGN7e004528f487: Swapping defaultbranch for trace
rEBSSMWCc921c0f4c55d: Swapping defaultbranch for trace
rERSL090e1c406638: Swapping defaultbranch for trace
rEPFM517b3f4e186b: Swapping defaultbranch for trace
rEOLYc094ce9b5e58: Swapping defaultbranch for trace
rEBSCe2770b5971cb: Swapping defaultbranch for trace
rECKT7446dea39785: Swapping defaultbranch for trace
rEEPS0d266a4cd692: Swapping defaultbranch for trace
rECOGa46bb165d32d: Swapping defaultbranch for trace
R1893:89cfad57c520: Swapping defaultbranch for trace
rSVEV32ce5cba2134: Swapping defaultbranch for trace

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 317690 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317690

Change 317691 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317691

Change 317692 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317692

Change 317693 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317693

Change 317694 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317694

Change 317695 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317695

Change 317696 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317696

Change 317697 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317697

Change 317698 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317698

Change 317699 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317699

Change 317700 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317700

Change 317701 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317701

Change 317702 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317702

Change 317703 abandoned by Chad:
Swapping defaultbranch for trace

https://gerrit.wikimedia.org/r/317703

Change 317722 merged by Chad:
Swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317722

Change 317714 merged by Chad:
Swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317714

demon closed this task as Resolved.Oct 25 2016, 12:39 AM
demon claimed this task.

Change 317715 merged by jenkins-bot:
Swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317715

Change 317716 merged by jenkins-bot:
Swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317716

hashar updated the task description. (Show Details)Oct 25 2016, 8:03 AM

Note: a bunch of attached commits used trace=1 and were followed up with commits to change it to the proper track=1.

A few follow ups are needed but not much to worry about.

Change 317771 had a related patch set uploaded (by Hashar):
Whoops, track not trace

https://gerrit.wikimedia.org/r/317771

Change 317771 merged by jenkins-bot:
Whoops, track not trace

https://gerrit.wikimedia.org/r/317771

Change 317731 had a related patch set uploaded (by Chad):
Stop mangling .gitreview

https://gerrit.wikimedia.org/r/317731

Change 317774 had a related patch set uploaded (by Hashar):
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317774

Change 317775 had a related patch set uploaded (by Hashar):
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317775

Change 317777 had a related patch set uploaded (by Hashar):
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317777

Change 317778 had a related patch set uploaded (by Hashar):
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317778

Change 317779 had a related patch set uploaded (by Hashar):
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317779

Change 317774 merged by jenkins-bot:
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317774

Change 317775 merged by jenkins-bot:
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317775

Change 317777 merged by jenkins-bot:
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317777

Change 317779 merged by jenkins-bot:
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317779

Change 317778 merged by jenkins-bot:
.gitreview: swapping defaultbranch for track

https://gerrit.wikimedia.org/r/317778

All set; I have caught up the last remaining skins/extensions that got missed :]

Change 317731 merged by jenkins-bot:
Stop mangling .gitreview

https://gerrit.wikimedia.org/r/317731

So....was anyone going to announce this? It did break git-review for me:

km@km-tp ~/p/v/m/e/Linter> git review -s
Cannot find symbolic reference
The following command failed with exit code 1
    "git symbolic-ref -q HEAD"
-----------------------

-----------------------

And I'm using git-review 1.25.0 from the official fedora repos.

So....was anyone going to announce this?

{{done}}

It did break git-review for me:

km@km-tp ~/p/v/m/e/Linter> git review -s
Cannot find symbolic reference
The following command failed with exit code 1
    "git symbolic-ref -q HEAD"
-----------------------
-----------------------

And I'm using git-review 1.25.0 from the official fedora repos.

Hrm. The main reason I could think of for git symbolic-ref HEAD to fail is if HEAD is not pointed to a symbolic-ref, for instance in a detached-head state. HEAD not pointing to a ref probably would break track=1 so it seems like a reasonable exception.

Not sure if this is related but with cognate extension i get this error when downloading a patch:

/extensions/Cognate$ git review -d 316780
Cannot query patchset information
The following command failed with exit code 104
    "GET https://gerrit.wikimedia.org/changes/?q=316780&o=CURRENT_REVISION"
-----------------------
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>404 Not Found</title>
</head><body>
<h1>Not Found</h1>
<p>The requested URL /changes/ was not found on this server.</p>
</body></html>

-----------------------

(git-review version 1.25.0)

Paladox added a comment.EditedNov 3 2016, 3:14 PM

I think in the .gitreview file in the host section

Instead of doing

host=gerrit.wikimedia.org

We
Should do

host=gerrit.wikimedia.org/r

Not sure if this is related but with cognate extension i get this error when downloading a patch:

/extensions/Cognate$ git review -d 316780
Cannot query patchset information
The following command failed with exit code 104
    "GET https://gerrit.wikimedia.org/changes/?q=316780&o=CURRENT_REVISION"
-----------------------
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>404 Not Found</title>
</head><body>
<h1>Not Found</h1>
<p>The requested URL /changes/ was not found on this server.</p>
</body></html>
-----------------------

(git-review version 1.25.0)

Hi that's because the link is wrong.

You are doing gerrit.wikimedia.org/change

Whereas it needs to be gerrit.wikimedia.org/r/change

So check in .git/config file and make sure that the gerrit link does gerrit.wikimedia.org/r

The /r prefix should be there ok

Other users have reported this problem.

Maybe try in host gerrit.wikimedia.org/r ?

Not sure if this is related but with cognate extension i get this error when downloading a patch:

/extensions/Cognate$ git review -d 316780
Cannot query patchset information
The following command failed with exit code 104
    "GET https://gerrit.wikimedia.org/changes/?q=316780&o=CURRENT_REVISION"
-----------------------
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>404 Not Found</title>
</head><body>
<h1>Not Found</h1>
<p>The requested URL /changes/ was not found on this server.</p>
</body></html>
-----------------------

(git-review version 1.25.0)

When querying reviews, it seems that git-review uses the gerrit remote url to find that information:
https://github.com/openstack-infra/git-review/blob/master/git_review/cmd.py#L571

For whatever reason, it seems that the url for the gerrit git remote may be incorrect. What is the value of git remote get-url gerrit inside the cognate extension directory?

What is the value of git remote get-url gerrit inside the cognate extension directory?

/extensions/Cognate$ git remote get-url gerrit
fatal: No such remote 'gerrit'

What is the value of git remote get-url gerrit inside the cognate extension directory?

/extensions/Cognate$ git remote get-url gerrit
fatal: No such remote 'gerrit'

heh, maybe this falls back...?

What about git remote -v?

hashar added a comment.Nov 4 2016, 4:52 PM

Also paste the output of git remote -v AND git-review -n --verbose that will show the underlying commands being used by git-review.

IIRC the issue happens when a repository has a push url set to https://. git-review then tries to use the REST API to connect and forge the URL solely based on the hostname (it miss the /r/).

The fix is to use ssh protocol for push. An example for my setup:

$ git remote -v
origin  https://gerrit.wikimedia.org/r/p/mediawiki/core.git (fetch)
origin  ssh://gerrit.wikimedia.org:29418/mediawiki/core.git (push)
$

If you still have a remote named gerrit I am pretty sure you can remove it. git-review 1.25.0 is supposed to use origin and on git-review -s will make the push url ssh://.

Catrope added a subscriber: Catrope.Nov 4 2016, 9:58 PM

git review -d is broken for me too, but only for https remotes. If I manually rewrite the remote to be ssh, it works.

Also paste the output of git remote -v AND git-review -n --verbose that will show the underlying commands being used by git-review.

Will do.

IIRC the issue happens when a repository has a push url set to https://. git-review then tries to use the REST API to connect and forge the URL solely based on the hostname (it miss the /r/).

That sounds right.

The fix is to use ssh protocol for push. An example for my setup:

$ git remote -v
origin  https://gerrit.wikimedia.org/r/p/mediawiki/core.git (fetch)
origin  ssh://gerrit.wikimedia.org:29418/mediawiki/core.git (push)
$

If you still have a remote named gerrit I am pretty sure you can remove it. git-review 1.25.0 is supposed to use origin and on git-review -s will make the push url ssh://.

That's not what I see though. I have https for both fetch and push, and git review -s doesn't change that. This is with extensions installed via vagrant, for reference.

[2016-11-04 14:57:11 PDT] catrope:~/vagrant/mediawiki/extensions/PageImages (master)$ git remote -v
origin	https://gerrit.wikimedia.org/r/p/mediawiki/extensions/PageImages.git (fetch)
origin	https://gerrit.wikimedia.org/r/p/mediawiki/extensions/PageImages.git (push)
[2016-11-04 14:57:13 PDT] catrope:~/vagrant/mediawiki/extensions/PageImages (master)$ git review -n --verbose -d 319651
2016-11-04 14:57:31.253760 Running: git symbolic-ref -q HEAD
2016-11-04 14:57:31.255941 Running: git for-each-ref --format=%(upstream) refs/heads/master
Following tracked origin/master rather than default origin/master
2016-11-04 14:57:31.258474 Running: git config --get gitreview.scheme
2016-11-04 14:57:31.260785 Running: git config --get gitreview.hostname
2016-11-04 14:57:31.263068 Running: git config --get gitreview.port
2016-11-04 14:57:31.265534 Running: git config --get gitreview.project
2016-11-04 14:57:31.268604 Running: git log --color=never --oneline HEAD^1..HEAD
2016-11-04 14:57:31.272505 Running: git remote
2016-11-04 14:57:31.275482 Running: git branch -a --color=never
2016-11-04 14:57:31.278540 Running: git config --get remote.origin.pushurl
2016-11-04 14:57:31.281717 Running: git config --get remote.origin.url
2016-11-04 14:57:31.285223 Running: git config --list
Found origin Push URL: https://gerrit.wikimedia.org/r/p/mediawiki/extensions/PageImages.git
Query gerrit https://gerrit.wikimedia.org/changes/?q=319651&o=CURRENT_REVISION
2016-11-04 14:57:31.288082 Running: git config --bool --get http.sslVerify
Cannot query patchset information
The following command failed with exit code 104
    "GET https://gerrit.wikimedia.org/changes/?q=319651&o=CURRENT_REVISION"
-----------------------
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>404 Not Found</title>
</head><body>
<h1>Not Found</h1>
<p>The requested URL /changes/ was not found on this server.</p>
</body></html>

-----------------------

git review -d is broken for me too, but only for https remotes. If I manually rewrite the remote to be ssh, it works.

Thanks that is working for me!

git review -d is broken for me too, but only for https remotes. If I manually rewrite the remote to be ssh, it works.

I am also experiencing the issue @Jonas described, but changing the remote manually to ssh did not fix the problem for me. I also examined .git/config to see if I just needed to add /r to a URL, but the only URL was the remote for origin and it was fully formed (right down to the <extension>.git).

Change 355041 had a related patch set uploaded (by QChris; owner: Christian Aistleitner):
[mediawiki/extensions/Whoops@master] Switch .gitreview from defaultbranch to track

https://gerrit.wikimedia.org/r/355041

Change 355041 merged by Hashar:
[mediawiki/extensions/Whoops@master] Switch .gitreview from defaultbranch to track

https://gerrit.wikimedia.org/r/355041

demon added a comment.May 22 2017, 4:03 PM

If we update docs to tell people to use ~/.config/git-review/git-review.conf, we can specify most of the values (host, port, track, defaultrebase) there, then .gitreview files would only need the project listed. Default behaviors would be nicer :)