Jump to content

Gerrit/Advanced usage: Difference between revisions

From mediawiki.org
Content deleted Content added
missing Change-Id: Problems cherry-picking changes
Line 560: Line 560:
Note: if upon doing 'git review' you receive a message "Working tree is dirty" try doing 'git add' for the file(s) changed (or created), then 'git commit', and then 'git review'. This was seen on OSX with an older git client.
Note: if upon doing 'git review' you receive a message "Working tree is dirty" try doing 'git add' for the file(s) changed (or created), then 'git commit', and then 'git review'. This was seen on OSX with an older git client.


==== missing Change-Id====
====missing Change-Id in commit message====
Note: if upon doing 'git review' you receive a message about 'missing Change-Id', then your /.git/hooks/commit-msg is probably incorrect. it should look something like:
Note: if upon doing 'git review' you receive a message about 'missing Change-Id', then your /.git/hooks/commit-msg is probably incorrect. it should look something like:


Line 583: Line 583:
fi
fi
</pre>
</pre>

You will also get a missing Change-ID message when trying to merge (<code>git cherry-pick</code>) some change from git that does not have <code>Change-ID</code>. It seems that the hook isn't called by cherry-pick, but it is fortunately called by <code>git commit -c ''some-commit-id''</code>.

In the example below we will be moving trivial change [https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/core.git;a=commit;h=bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc] from git master in the mediawiki/core project (it came via SVN trunk) to REL1_19.

[http://tools.wikimedia.pl/~saper/fail/cherry-pick-to-branch-message-fail Sample session] to exhibit this problem:

<source lang="bash">
Script started on Sun Mar 25 01:51:46 2012
$ git checkout REL1_19
Switched to branch 'REL1_19'
$ git log --pretty=oneline | head -1
9074142447ab0bd06f4e6a7ca7c9d8c70d33d109 * (bug 35449) Removed double call to OutputPage::setRobotPolicy() in SpecialWatchlist::execute() (the other one is hidden in SpecialPage::setHeaders()) * Special:Watchlist no longer sets links to feed when the user is anonymous
$ git show bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc
</source>
<source lang="text">
commit bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc
Author: Marcin Cieślak <saper@users.mediawiki.org>
Date: Wed Mar 14 00:36:11 2012 +0000

Cosmetic improvements to PostreSQL updater output
* Don't WARN on sequences already existing
* Align dots nicely to the rest

</source>
<source lang="diff">
diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php
index d1fc6f7..d4412cb 100644
--- a/includes/installer/PostgresUpdater.php
+++ b/includes/installer/PostgresUpdater.php
@@ -394,7 +394,7 @@ END;
protected function renameSequence( $old, $new ) {
if ( $this->db->sequenceExists( $new ) ) {
- $this->output( "WARNING sequence $new already exists\n" );
+ $this->output( "...sequence $new already exists.\n" );
return;
</source>
''(...full trivial diff to one file omitted...)''
<source lang="bash">
$ git cherry-pick -x bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc
[REL1_19 a354acd] Cosmetic improvements to PostreSQL updater output
Author: Marcin Cieślak <saper@users.mediawiki.org>
1 files changed, 18 insertions(+), 18 deletions(-)
$ git log HEAD ^FETCH_HEAD
</source>
<source lang="text">
commit a354acd879c3dd840e7be1e3c6d6fc78d696631d
Author: Marcin Cieślak <saper@users.mediawiki.org>
Date: Wed Mar 14 00:36:11 2012 +0000

Cosmetic improvements to PostreSQL updater output
* Don't WARN on sequences already existing
* Align dots nicely to the rest
(cherry picked from commit bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc)
</source>
<source lang="bash">
$ git push gerrit HEAD:refs/for/REL1_19/PostgreSQL
</source>
<source lang="text">
Counting objects: 9, done.
Delta compression using up to 8 threads.
Compressing objects: 20% (1/5)
Compressing objects: 40% (2/5)
Compressing objects: 60% (3/5)
Compressing objects: 80% (4/5)
Compressing objects: 100% (5/5)
Compressing objects: 100% (5/5), done.
Writing objects: 20% (1/5)
Writing objects: 40% (2/5)
Writing objects: 60% (3/5)
Writing objects: 80% (4/5)
Writing objects: 100% (5/5)
Writing objects: 100% (5/5), 750 bytes, done.
Total 5 (delta 4), reused 0 (delta 0)
remote: Resolving deltas: 0% (0/4)
remote: Resolving deltas: 0% (0/4)
remote: ERROR: missing Change-Id in commit message
remote: Suggestion for commit message:
remote: Cosmetic improvements to PostreSQL updater output
remote:
remote: * Don't WARN on sequences already existing
remote: * Align dots nicely to the rest
remote: (cherry picked from commit bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc)
remote:
remote: Change-Id: Ia354acd879c3dd840e7be1e3c6d6fc78d696631d
To ssh://saper@review:29418/mediawiki/core.git
! [remote rejected] HEAD -> refs/for/REL1_19/PostgreSQL (missing Change-Id in commit message)
error: failed to push some refs to 'ssh://saper@review:29418/mediawiki/core.git'
$

Script done on Sun Mar 25 01:54:08 2012
</source>

To fix this, use <code>-n</code> ''(don't commit)'' option to <code>git cherry-pick</code>:
git cherry-pick -n bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc
git commit -c bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc

Unfortunately, if you want to add the original commit ID to the message (as done by <code>git cherry-pick -x</code>) you have to add it yourself.

The change above has been submitted as [https://gerrit.wikimedia.org/r/gitweb?p=mediawiki%2Fcore.git;a=commit;h=3c88c61f1b7e36d5d374a42bb0f50783ab5391a4 3c88c61f1b7e36d5d374a42bb0f50783ab5391a4] for <code>REL1_19</code> review.


==== Don't push ====
==== Don't push ====

Revision as of 01:16, 25 March 2012

This page describes the workflow for how MediaWiki core and extensions developers will use Git, git-review, and Gerrit.

Ideal Git workflow for MediaWiki based on Media:Ideal_git_workflow.jpg

The diagram on the right is an accurate description of what our Git workflow looks like. (Transcription is welcome!)

To understand our workflow, see below.

Justification

Easier submission of code

Get an account!

In the old workflow, developers had to send their patch through Bugzilla or email it, then have someone with proper access rights commit it. Extensions developers were prevented from committing to MediaWiki core.

With Git, everyone commits in their local repository. Then everyone is allowed to submit their code for review to Gerrit. There is no more distinction between core and extensions access rights. Since code does not automatically make it into the WMF repository, there's no danger of bad commits unless someone merges irresponsibly. Thus, for many of the Git repositories hosted by the WMF, only a small group of people needs the access rights to merge (apply) the code. See "Who can review?".

This workflow has proven successful for the WMF operation team. That model is also known as "gated trunk" - code needs review before merge - and will be the default model for WMF hosted git repository. Extensions can optionally choose to adopt a straight push model in which code is reviewed post-commit. This is much more like the Subversion model in use before 2012.

If you are wondering who is going to review your code, jump straight to "Who can review?"

Better branching

Git supports branching and merging far better than Subversion did. We are encouraging people to use branches a lot.

With git-review, you can group branches as a "topic" to help with keeping feature development straight. We aim to encourage people to name branches appropriately. If you are fixing bug n, make a "bug n" branch! Look in Gerrit at the top of changesets to see examples. git-review should detect the "bug XXX" in your commit summary and automatically create a topic branch for you.

Tags vs branches: branches are for forking, to maintain a separate feature branch. This is like traditional branching. Tags are more lightweight, designed to point to an individual commit. You can delete a tag, but they are not mutable. Designed for tagging releases. "As of this commit, this was exactly release 1.2.3" -- useful if you aim to tell people to check out specific versions.

Gerrit and Git deal with merge conflicts pretty well. In cases of merge conflicts, the code reviewer gets a notice from gerrit that the merge couldn't go through, and the final merge step fails, but all the code review work will be untouched. To fix the issue, you will have to manually rebase your branch on top of the master branch and resubmit your change.

Getting set up

If you are on Windows, follow https://labsconsole.wikimedia.org/wiki/Help:Git#Windows .

Install & configure Git

Install Git on your computer.

  • Linux: see http://evgeny-goldin.com/wiki/Git#Linux
  • Windows: users can download msysgit (http://code.google.com/p/msysgit/) which includes a version of git and some nifty "Git Bash" tools that act like a basic UNIX shell.
    • Select the "OpenSSH" option in the installer to use ssh-agent.exe. It can instead be configured to work with plink/pageant as may be convenient (provided it doesn't cause any problems).
    • It may also be useful to install TortoiseGit as a complimentary, shell-integrated, GUI but code can only be pushed with the "git review" command via the command line or Git Bash.

Then do

git config -l

to check that you have a name and email address configured; if you don't, tell Git your username and e-mail address so gerrit can attach it to your accounts:

git config --global user.email "foo@example.com"
git config --global user.name "mrfoo"

Get the right permissions

To clone our code repository properly, so as to easily submit a patch back to us, you need a login. We manage this via a unified LDAP login shared between Gerrit and Labsconsole. Get a login!

Log in to https://labsconsole.wikimedia.org/, verify that you can log in, then log in at https://gerrit.wikimedia.org/ with the same username and password. Click Settings, go to SSH Public Keys, and paste in your public key (usually ~/.ssh/id_rsa.pub on Linux) so our git repository will recognize you. (We aim to fix this soon by simply pulling the key from LDAP.)

If you have a few different email addresses or SSH keys, go into Gerrit's settings and add them to your account.

Teacher: give the user a login using these instructions.

Setup SSH shortcut (optional)

It's easier to access the repository if one does not need to specify full path and the port number. You can just edit your ~/.ssh/config file and add

Host review
Hostname gerrit.wikimedia.org
Port 29418
User yourusername

From now on you can just use shortened commands like:

git clone ssh://review/mediawiki/core.git   

instead of longish

git clone ssh://username@gerrit.wikimedia.org:29418/mediawiki/core.git

Clone the repository

Now, to clone the code repository in question that you want to make a change on. For a list of all projects in Gerrit, please see the project list in Gerrit, or get a plain-text list with:

ssh -p 29418 gerrit.wikimedia.org gerrit ls-projects

Create or move to the directory on your machine that you do your development in. If you're making one, wikimedia-git-repos is a good name.

mkdir wikimedia-git-repos
cd wikimedia-git-repos

Now, clone the respository.

If you're doing the examples tutorial with Sumana, use this:
git clone ssh://USERNAME@gerrit.wikimedia.org:29418/test/mediawiki/extensions/examples.git OPTIONAL-DESCRIPTIVE-NAME

Otherwise you type something like this (replacing mediawiki/core.git with mediawiki/extensions/FooBar.git for an extension or any other part of the repository):

git clone ssh://<USERNAME>@gerrit.wikimedia.org:29418/mediawiki/core.git

Go have a sip of coffee while you wait for a minute :)


Notes are not yet been restored, as of March 23, 2012

Once cloned, get commit notes which will nicely indicate the old SVN version numbers.

cd core && git fetch origin refs/notes/commits:refs/notes/commits

Prepare to work with gerrit

In order to properly work with gerrit, you need to have a pre-commit hook that adds a "change id" to your commit summary. That unique ID will let Gerrit track your commit. Since Git commit identification is based upon a sha1 of the commit id, whenever you amend a commit, the sha1 will change and gerrit would lost track of it!

git-review

Linux

Install git-review, a tool to simplify working with Gerrit repositories so you don't have to remember some pretty confusing commands.

sudo apt-get install python-pip || sudo easy_install pip
sudo pip install git-review

If you do not have easy_install set up, try

sudo apt-get install python-setuptools

Then run:

git review -s

in your cloned copy to setup to work with Gerrit. It will probably ask you for your commit username. Then it will automatically install the pre-commit hook.


Gentoo users can ignore all that apt-get stuff and type:

emerge dev-python/pip
pip install git-review
Windows
  • Install pip using these instructions [1]:
    • Download and uncompress the latest pip version from here: http://pypi.python.org/pypi/pip#downloads (make sure it is the right version for your Python; possibly upgrade Python first)
    • Download and install the latest easy installer for Windows: the .exe at the bottom of http://pypi.python.org/pypi/setuptools (make sure it is the right version for your Python; possibly upgrade Python first)
    • Go to the uncompressed pip directory and run python setup.py install.
    • Add your c:\Python2x\Scripts to the system path (replacing "X" with the actual python version). Different directories are delimited only by a ";", so do not add whitespace.
  • Run pip install git-review
  • Create git-review.bat in some PATH-accessible directory containing the following line (replacing "X" with the actual python version):
@python c:\Python2x\Scripts\git-review %*
  • Make sure you can connect to gerrit properly via SSH.
    • Start Git Bash.
    • Get ssh-agent running if not done already via eval `ssh-agent`.
    • Add your key to the agent if not done so already via ssh-add path/to/key. Make sure the key is in OpenSSH format (.ppk keys can be exported to this format in PuTTyGen).
    • Run ssh <username>@gerrit.wikimedia.org -p 29418. It should give a Gerrit welcome message and then abort.
  • In Git Bash, go to the directory of the fresh MediaWiki clone and run git-review -s. This should install a git hook called "commit-msg" under .git/hooks. Check that the file is there. From now on, commits will have a change-id appended to the summary, which Gerrit will need in order to accept any commits for review.
    • If the commit-msg file fails to download to .git/hooks, then run scp -P 29418 -v <username>@gerrit.wikimedia.org:hooks/commit-msg . in that directory.
Mac OS X via Terminal

Mac OS X comes with Python (for now) but not the installation programs supported by git and git-review.

  1. Open Terminal and change to a directory you're comfortable downloading test Git packages to (such as Downloads or Sites)
  2. Download and install the OSX Installer for Git
  3. Install pip (Note: Already included in some older versions of Mac OS X):
    • sudo easy_install pip
  4. Install git-review:
    • sudo pip install git-review

Manual setup

If installing git-review is not feasible for you, you will use an alternate way to communicate with Gerrit.

First, you need to download a pre-commit hook script and place it in the right directory in your cloned copy of the repository. The script is available from https://gerrit.wikimedia.org/r/tools/hooks/commit-msg and must be placed in the repository sub directory .git/hooks/

1) With a browser:

Download the script from the repo using "Save As ..." then browse to wikimedia-git-repos/examples/.git/hooks/. Voilà!

2) With wget:

Change to the repository directory (for example, cd wikimedia-git-repos/examples/

wget  -P .git/hooks https://gerrit.wikimedia.org/r/tools/hooks/commit-msg

3) With curl:

curl https://gerrit.wikimedia.org/r/tools/hooks/commit-msg > .git/hooks/commit-msg

You also need to ensure the hook is executable. In Linux you do this with:

chmod u+x .git/hooks/commit-msg

When ever you commit a change locally, the hook script will generate a unique Change-Id for you.

Next, add an alias to simplify the command to push changes to Gerrit for review. You can do this by executing the following from your repository clone (for example, within wikimedia-git-repos/examples/):

git config alias.push-for-review "push origin HEAD:refs/for/master"

(The refs/for/ is Gerrit magic and must not be omitted. However, you may adapt master to point to the remote branch that you want to commit to. E.g.: When trying to push to the remote branch Foo use refs/for/Foo.)

How to submit a patch

Update master

Make sure that your master branch (the branch created when you initially cloned the repository is up to date:

git pull master

Create a branch

First, create a branch for your new change. Give the branch a short but reasonably descriptive name.

git checkout -b BRANCHNAME master

This will create a new branch (BRANCHNAME) from 'master' and check it out for you. This is equivalent to doing

git branch BRANCHNAME master
git checkout BRANCHNAME

Make and commit your change

Change the code in the examples directory in some way; have fun! Commit them to your branch by doing:

git commit -a -m "COMMIT MESSAGE HERE"

(don't forget to git add any new files).

Tip: Using git commit -a -m "Commit message here" is nice because you can write the commit message right there in the command. But you can also do git commit -a for longer commit messages, and then it'll open a text editor for you to write the commit summary in.

Please mention the bug numbers of Bugzilla bugs that the commit addresses. There's no autolinking from Gerrit to Bugzilla yet, but we'll work on it! For now, when referring to a Git diff, please paste changeset "Change ID"s, or the changeset number in the Gerrit changeset URL, into the BZ comment. Both are globally unique.

Then check the changes you've made, within the file(s) and within the directory:

git diff
git status

You can repeat this step over and over until you have a set of changes that you want to have pushed to the master branch. One of the cool things about git is that when you git commit, you are committing LOCALLY. This means you can commit as often as you like without potentially screwing things up for another developer on the project, unlike in SVN where you would want to be very careful that the changes you commit would not cause things to break.

Prepare to push your change set to Gerrit

Before your changes can be merged into master, they must undergo review in Gerrit.

But first, it's a good idea to synchronize your change set with any changes that may have occurred in master while you've been hacking. From within the branch you've been working:

git pull master
git rebase

git pull will update the code in your local copy of the master branch. Then, git rebase will temporarily set aside the changes you've made in your branch, apply all of the changes that have happend in master to your working branch, then merge all of the changes you've made back into the branch. Doing this will help avoid future merge conflicts. Plus, it gives you an opportunity to test your changes against the latest code in master.

Once you are satisfied with your change set and you've rebased against master, you are ready to push your code to Gerrit for review.

Push your change set to Gerrit

If you installed git review, the command to push changes to Gerrit is very simple:

git review

If you had to install the hook manually, you can push your changes by doing:

git push-for-review

Upon success, you'll get a confirmation and a link to the changeset in Gerrit.

If your commit addresses a bug in Bugzilla, please comment on that bug to note that the commit is in the merge queue, and link to its changeset in Gerrit.

Note: Some extensions do not require review, in which case you can just git push origin master to push your changes past Gerrit straight into the repository.

Amend your change

Sometimes, you might need to amend the change that you've submitted.

Checkout the change (you can find this line in Gerrit, on the change, in Download -> checkout, ssh):

git fetch ssh://<username>@gerrit.wikimedia.org:29418/mediawiki/ <ref> && git checkout FETCH_HEAD

Make changes then commit the change, ensuring you are amending the commit:

git commit --amend -a

Push the change

git review

Submitting a change to a branch for review

I would like to propose moving change 556c5cf464b9103b04b247ed7dd7ee3051e9aef6 that was approved to master for REL1_19 branch.

Here it goes, using git cherry-pick:

Script started on Sat Mar 24 18:31:53 2012
$ git checkout master
Already on 'master'
$ git show 556c5cf464b9103b04b247ed7dd7ee3051e9aef6
commit 556c5cf464b9103b04b247ed7dd7ee3051e9aef6
Author: Marcin Cieślak <saper@users.mediawiki.org>
Date:   Wed Mar 14 20:20:53 2012 +0000

    Rename "user" and "text" when upgrading on PostgreSQL
    
    Follow-up to r15791.
    
    You can lie to me, but not to your installer.
    
    Make DatabasePostgres::tableExists to check for real table names, not
    faked ones.
    
    DatabasePostgres is currently lying to the rest of the MediaWiki that
    "mwuser" table is actually called "user" and that "pagecontents" is
    called "text". While MediaWiki does not care, the installer (and updater
    do).
    
    This allows us to overcome first hurdle in getting MediaWiki 1.7.3 to
    update to trunk on PostgreSQL and uncover further bugs.
    
    For this commit to actually do something, we rename those tables when
    upgrading to match what we have in maintenance/postgres/tables.sql
    
    And by the way, tell installer not to check for "user" table, since most
    PostgreSQL users will have "mwuser" instead. Picking "archive" instead.
    
    Patchset2: wrapped commit message at 72 chars
    
    Change-Id: Ic874e92bb1adda3430d3292423d4f3617c25456c
diff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php
index 6452f54..e2b38f5 100644
--- a/includes/db/DatabasePostgres.php
+++ b/includes/db/DatabasePostgres.php
@@ -708,14 +708,19 @@ class DatabasePostgres extends DatabaseBase {
                # Replace reserved words with better ones
                switch( $name ) {
                        case 'user':
-                               return 'mwuser';
+                               return $this->realTableName( 'mwuser', $format );
                        case 'text':
-                               return 'pagecontent';
+                               return $this->realTableName( 'pagecontent', $format );
                        default:
-                               return parent::tableName( $name, $format );
+                               return $this->realTableName( $name, $format );
                }
        }
 
+       /* Don't cheat on installer */
+       function realTableName( $name, $format = 'quoted' ) {
+               return parent::tableName( $name, $format );
+       }
+
        /**
         * Return the next in a sequence, save the value for retrieval via insertId()
         * @return null
@@ -990,7 +995,7 @@ class DatabasePostgres extends DatabaseBase {
                if ( !$schema ) {
                        $schema = $this->getCoreSchema();
                }
-               $table = $this->tableName( $table, 'raw' );
+               $table = $this->realTableName( $table, 'raw' );
                $etable = $this->addQuotes( $table );
                $eschema = $this->addQuotes( $schema );
                $SQL = "SELECT 1 FROM pg_catalog.pg_class c, pg_catalog.pg_namespace n "
diff --git a/includes/installer/DatabaseInstaller.php b/includes/installer/DatabaseInstaller.php
index 14604c1..046fa16 100644
--- a/includes/installer/DatabaseInstaller.php
+++ b/includes/installer/DatabaseInstaller.php
@@ -158,7 +158,7 @@ abstract class DatabaseInstaller {
                }
                $this->db->selectDB( $this->getVar( 'wgDBname' ) );
 
-               if( $this->db->tableExists( 'user', __METHOD__ ) ) {
+               if( $this->db->tableExists( 'archive', __METHOD__ ) ) {
                        $status->warning( 'config-install-tables-exist' );
                        $this->enableLB();
                        return $status;
diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php
index d4412cb..6b3cb51 100644
--- a/includes/installer/PostgresUpdater.php
+++ b/includes/installer/PostgresUpdater.php
@@ -27,6 +27,11 @@ class PostgresUpdater extends DatabaseUpdater {
         */
        protected function getCoreUpdateList() {
                return array(
+                       # rename tables 1.7.3
+                       # r15791 Change reserved word table names "user" and "text"
+                       array( 'renameTable', 'user', 'mwuser'),
+                       array( 'renameTable', 'text', 'pagecontent'),
+
                        # new sequences
                        array( 'addSequence', 'logging_log_id_seq'          ),
                        array( 'addSequence', 'page_restrictions_pr_id_seq' ),
@@ -406,7 +411,8 @@ END;
        protected function renameTable( $old, $new ) {
                if ( $this->db->tableExists( $old ) ) {
                        $this->output( "Renaming table $old to $new\n" );
-                       $old = $this->db->addQuotes( $old );
+                       $old = $this->db->realTableName( $old, "quoted" );
                        $new = $this->db->realTableName( $new, "quoted" );
                        $this->db->query( "ALTER TABLE $old RENAME TO $new" );
                }
        }
$ git checkout REL1_19
Switched to branch 'REL1_19'
$ git cherry-pick -n 556c5cf464b9103b04b247ed7dd7ee3051e9aef6
$ git diff
$ git diff --cached
diff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php
index 2b0c0a3..98cf3c7 100644
--- a/includes/db/DatabasePostgres.php
+++ b/includes/db/DatabasePostgres.php
@@ -627,14 +627,19 @@ class DatabasePostgres extends DatabaseBase {
                # Replace reserved words with better ones
                switch( $name ) {
                        case 'user':
-                               return 'mwuser';
+                               return $this->realTableName( 'mwuser', $format );
                        case 'text':
-                               return 'pagecontent';
+                               return $this->realTableName( 'pagecontent', $format );
                        default:
-                               return parent::tableName( $name, $format );
+                               return $this->realTableName( $name, $format );
                }
        }
 
+       /* Don't cheat on installer */
+       function realTableName( $name, $format = 'quoted' ) {
+               return parent::tableName( $name, $format );
+       }
+
        /**
         * Return the next in a sequence, save the value for retrieval via insertId()
         */
@@ -756,7 +761,7 @@ class DatabasePostgres extends DatabaseBase {
                if ( !$schema ) {
                        $schema = $wgDBmwschema;
                }
-               $table = $this->tableName( $table, 'raw' );
+               $table = $this->realTableName( $table, 'raw' );
                $etable = $this->addQuotes( $table );
                $eschema = $this->addQuotes( $schema );
                $SQL = "SELECT 1 FROM pg_catalog.pg_class c, pg_catalog.pg_namespace n "
diff --git a/includes/installer/DatabaseInstaller.php b/includes/installer/DatabaseInstaller.php
index b086478..ab77e2d 100644
--- a/includes/installer/DatabaseInstaller.php
+++ b/includes/installer/DatabaseInstaller.php
@@ -157,7 +157,7 @@ abstract class DatabaseInstaller {
                }
                $this->db->selectDB( $this->getVar( 'wgDBname' ) );
 
-               if( $this->db->tableExists( 'user', __METHOD__ ) ) {
+               if( $this->db->tableExists( 'archive', __METHOD__ ) ) {
                        $status->warning( 'config-install-tables-exist' );
                        $this->enableLB();
                        return $status;
diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php
index 41e5b31..d177d0e 100644
--- a/includes/installer/PostgresUpdater.php
+++ b/includes/installer/PostgresUpdater.php
@@ -26,6 +26,11 @@ class PostgresUpdater extends DatabaseUpdater {
         */
        protected function getCoreUpdateList() {
                return array(
+                       # rename tables 1.7.3
+                       # r15791 Change reserved word table names "user" and "text"
+                       array( 'renameTable', 'user', 'mwuser'),
+                       array( 'renameTable', 'text', 'pagecontent'),
+
                        # new sequences
                        array( 'addSequence', 'logging_log_id_seq'          ),
                        array( 'addSequence', 'page_restrictions_pr_id_seq' ),
@@ -410,7 +415,8 @@ END;
        protected function renameTable( $old, $new ) {
                if ( $this->db->tableExists( $old ) ) {
                        $this->output( "Renaming table $old to $new\n" );
-                       $old = $this->db->addQuotes( $old );
+                       $old = $this->db->realTableName( $old, "quoted" );
+                       $new = $this->db->realTableName( $new, "quoted" );
                        $this->db->query( "ALTER TABLE $old RENAME TO $new" );
                }
        }
$ git commit -m "Merge 556c5cf464b9103b04b247ed7dd7ee3051e9aef6 for REL1_19 branch"
[REL1_19 dd97c81] Merge 556c5cf464b9103b04b247ed7dd7ee3051e9aef6 for REL1_19 branch
 3 files changed, 17 insertions(+), 6 deletions(-)
$ git push gerrit HEAD:refs/for/REL1_19
Counting objects: 15, done.
Delta compression using up to 8 threads.
Compressing objects:  12% (1/8)   
Compressing objects:  25% (2/8)   
Compressing objects:  37% (3/8)   
Compressing objects:  50% (4/8)   
Compressing objects:  62% (5/8)   
Compressing objects:  75% (6/8)   
Compressing objects:  87% (7/8)   
Compressing objects: 100% (8/8)   
Compressing objects: 100% (8/8), done.
Writing objects:  12% (1/8)   
Writing objects:  25% (2/8)   
Writing objects:  37% (3/8)   
Writing objects:  50% (4/8)   
Writing objects:  62% (5/8)   
Writing objects:  75% (6/8)   
Writing objects:  87% (7/8)   
Writing objects: 100% (8/8)   
Writing objects: 100% (8/8), 1.03 KiB, done.
Total 8 (delta 7), reused 0 (delta 0)
remote: Resolving deltas:   0% (0/7)   
remote: Resolving deltas:   0% (0/7)
remote: 
remote: New Changes:
remote:   https://gerrit.wikimedia.org/r/3689
remote: 
To ssh://saper@review:29418/mediawiki/core.git
 * [new branch]      HEAD -> refs/for/REL1_19
$ git branch r3689p1
$
Script done on Sat Mar 24 18:34:00 2012

Thus, gerrit change is created for review.

Committing to non master

To make a change to 1.17, create a tag, and push that tag:

git checkout -b REL1_17 origin/REL1_17

<make code changes>

git add <files-changed>
git commit
git push origin REL1_17
git tag 1.17.3
git push --tags

Troubleshooting

SSH and "permission denied (publickey)"

If you get the error

Permission denied (publickey).
fatal: The remote end hung up unexpectedly

Then you're not logged in to your ssh key right now. Solution: do ssh-add ~/.ssh/id_rsa to make it prompt you for the passphrase for your key and add it to the active keychain. Then you can check what's in your keychain with ssh-add -l. Then try pushing for review again.

The fingerprint of the Gerrit server is

dc:e9:68:7b:99:1b:27:d0:f9:fd:ce:6a:2e:bf:92:e1

so you can say yes when it asks you to add that fingerprint to the known hosts file.

Keep in mind that gerrit listens on port 29418 and if for some reason you forgot to specify port number, you might be hitting "normal" SSH daemon listening on port 22 (this one has RSA key fingerprint b5:e9:dc:b2:dd:6e:70:f7:18:8a:dc:a3:5d:ab:99:4d).

To check whether SSH connectivity and public key authentication work you can use:

 $ ssh -p 29418 username@gerrit.wikimedia.org

  ****    Welcome to Gerrit Code Review    ****

  Hi username, you have successfully connected over SSH.

  Unfortunately, interactive shells are disabled.
  To clone a hosted Git repository, use:

  git clone ssh://username@gerrit.wikimedia.org:29418/REPOSITORY_NAME.git

  Connection to gerrit.wikimedia.org closed.

"[remote rejected] master -> master" and "failed to push some refs"

If you forget to add HEAD:refs/for/master as an alias, you will receive something along the lines of:

$ git push
Counting objects: 5, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 709 bytes, done.
Total 3 (delta 1), reused 0 (delta 0)
remote: Resolving deltas:   0% (0/1)
To ssh://username@gerrit.wikimedia.org:29418/test/mediawiki/extensions/examples
 ! [remote rejected] master -> master (prohibited by Gerrit)
error: failed to push some refs to 'ssh://username@gerrit.wikimedia.org:29418/test/mediawiki/extensions/examples'

This means you tried to commit to branch "master" instead of submitting your changes for review.


Email doesn't match

TODO: help people understand how to granularly config their email and name settings in Git and/or Gerrit per-repo to make gerrit accept their changes

remote: ERROR:  committer email address (email)
remote: ERROR:  does not match your user account.

Fix: add your secondary email address in Gerrit, and make sure you click the confirmation link in the email Gerrit sends you. Then try pushing again.

Working tree is dirty

Note: if upon doing 'git review' you receive a message "Working tree is dirty" try doing 'git add' for the file(s) changed (or created), then 'git commit', and then 'git review'. This was seen on OSX with an older git client.

missing Change-Id in commit message

Note: if upon doing 'git review' you receive a message about 'missing Change-Id', then your /.git/hooks/commit-msg is probably incorrect. it should look something like:

CHANGE_ID_AFTER="Bug|Issue"
MSG="$1"

# Check for, and add if missing, a unique Change-Id
#
add_ChangeId() {
        clean_message=`sed -e '
                /^diff --git a\/.*/{
                        s///
                        q
                }
                /^Signed-off-by:/d
                /^#/d
        ' "$MSG" | git stripspace`
        if test -z "$clean_message"
        then
                return
        fi

You will also get a missing Change-ID message when trying to merge (git cherry-pick) some change from git that does not have Change-ID. It seems that the hook isn't called by cherry-pick, but it is fortunately called by git commit -c some-commit-id.

In the example below we will be moving trivial change bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc from git master in the mediawiki/core project (it came via SVN trunk) to REL1_19.

Sample session to exhibit this problem:

Script started on Sun Mar 25 01:51:46 2012
$ git checkout REL1_19
Switched to branch 'REL1_19'
$ git log --pretty=oneline | head -1
9074142447ab0bd06f4e6a7ca7c9d8c70d33d109 * (bug 35449) Removed double call to OutputPage::setRobotPolicy() in SpecialWatchlist::execute() (the other one is hidden in SpecialPage::setHeaders()) * Special:Watchlist no longer sets links to feed when the user is anonymous
$ git show bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc
commit bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc
Author: Marcin Cieślak <saper@users.mediawiki.org>
Date:   Wed Mar 14 00:36:11 2012 +0000

    Cosmetic improvements to PostreSQL updater output
    
    * Don't WARN on sequences already existing
    * Align dots nicely to the rest
diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php
index d1fc6f7..d4412cb 100644
--- a/includes/installer/PostgresUpdater.php
+++ b/includes/installer/PostgresUpdater.php
@@ -394,7 +394,7 @@ END;
 
 	protected function renameSequence( $old, $new ) {
 		if ( $this->db->sequenceExists( $new ) ) {
-			$this->output( "WARNING sequence $new already exists\n" );
+			$this->output( "...sequence $new already exists.\n" );
 			return;

(...full trivial diff to one file omitted...)

$ git cherry-pick -x bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc
[REL1_19 a354acd] Cosmetic improvements to PostreSQL updater output
 Author: Marcin Cieślak <saper@users.mediawiki.org>
 1 files changed, 18 insertions(+), 18 deletions(-)
$ git log HEAD ^FETCH_HEAD
commit a354acd879c3dd840e7be1e3c6d6fc78d696631d
Author: Marcin Cieślak <saper@users.mediawiki.org>
Date:   Wed Mar 14 00:36:11 2012 +0000

    Cosmetic improvements to PostreSQL updater output
    
    * Don't WARN on sequences already existing
    * Align dots nicely to the rest
    (cherry picked from commit bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc)
$ git push gerrit HEAD:refs/for/REL1_19/PostgreSQL
Counting objects: 9, done.
Delta compression using up to 8 threads.
Compressing objects:  20% (1/5)   
Compressing objects:  40% (2/5)   
Compressing objects:  60% (3/5)   
Compressing objects:  80% (4/5)   
Compressing objects: 100% (5/5)   
Compressing objects: 100% (5/5), done.
Writing objects:  20% (1/5)   
Writing objects:  40% (2/5)   
Writing objects:  60% (3/5)   
Writing objects:  80% (4/5)   
Writing objects: 100% (5/5)   
Writing objects: 100% (5/5), 750 bytes, done.
Total 5 (delta 4), reused 0 (delta 0)
remote: Resolving deltas:   0% (0/4)   
remote: Resolving deltas:   0% (0/4)
remote: ERROR: missing Change-Id in commit message
remote: Suggestion for commit message:
remote: Cosmetic improvements to PostreSQL updater output
remote: 
remote: * Don't WARN on sequences already existing
remote: * Align dots nicely to the rest
remote: (cherry picked from commit bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc)
remote: 
remote: Change-Id: Ia354acd879c3dd840e7be1e3c6d6fc78d696631d
To ssh://saper@review:29418/mediawiki/core.git
 ! [remote rejected] HEAD -> refs/for/REL1_19/PostgreSQL (missing Change-Id in commit message)
error: failed to push some refs to 'ssh://saper@review:29418/mediawiki/core.git'
$ 

Script done on Sun Mar 25 01:54:08 2012

To fix this, use -n (don't commit) option to git cherry-pick:

git cherry-pick -n bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc
git commit -c bd7a268e4be2da23ba0b9943c3b0ba1ac88294dc

Unfortunately, if you want to add the original commit ID to the message (as done by git cherry-pick -x) you have to add it yourself.

The change above has been submitted as 3c88c61f1b7e36d5d374a42bb0f50783ab5391a4 for REL1_19 review.

Don't push

If you attempt to do 'git push' after doing 'git commit' you may receive a response 'Everything up-to-date'. You have not pushed to the branch. You have to do 'git review' to move your changes to gerrit, and only from gerrit will the branch be updated. This seems to be a side effect of checking out master as a branch as of February 2012.

In some projects (e.g. test/) it is possible to do 'git push' instead of 'git review' and have the push succeed. It is probably better not to do that, as it confuses those who find your changes later and don't know where they came from.

Fixing unsatisfied dependencies (rebase changes)

TODO: Someone fill me in please? =)

How we review code

The things we look for in code won't change, but the workflow is different than it was before the Git switch.

Review before merge

It's important to us to have a review-before-merge workflow, for MediaWiki core and also for any extension we deploy. We will also offer that option to any extensions author who wants it for their extension. (The default is to use the current model we've been using with Subversion -- pushed and automatically merged.) For this decision we will defer to whoever is considered the primary author on the extension. See Git/Gerrit project ownership.

The one exception is i18n commits, which will be able to be pushed without review.

Who can review? Gerrit project owners

Who has the ability to do code review?

We use gerrit to manage code review. Anyone can ask for a Gerrit account (Get an account!). Within Gerrit, anyone can comment on commits and signal their criticisms and approvals. Anyone can give a nonbinding "+1" to any commit (this is like "inspection" and "signoff" in the old Subversion code review interface). However, for any given repository ("Gerrit project"), only a small group of people will have the ability to approve code within Gerrit and merge it into the repository. (Within gerrit, this superapproval is a "+2" even though that's a misleading name, because two +1 approvals DO NOT add up to a +2.) These people are "Gerrit project owners". To learn about becoming a Gerrit project owner, see Git/Gerrit project ownership.

Even within a Gerrit project, we can also specify particular branches that only specific people can pull into.

To make a new Project Owner:

MediaWiki core

We are maintaining a "WMF" branch of mediawiki/core.git. We use submodules for deployed extensions, and can pull from master as regularly as we want for deployments. At the start of the migration to git, the project owners of this branch are going to be the people who have the ability to deploy code to Wikimedia Foundation servers. gerrit will offer a list of the "Gerrit project owners" for this branch, except for the Operations (system administration) group, which is an LDAP group. Every member of the Wikimedia Foundation operations team will also be in the Gerrit project owners group insofar as they have code review rights globally, but in practice will rarely review code. We may add some existing code reviewers to this Gerrit project owners group. Details; you can request to be added.

At the start of the migration, this list of Gerrit project owners for the WMF branch is also the list of Gerrit project owners for the master branch. However, eventually, we will add to the list of Gerrit project owners for master, using as criteria the number and quality of developers' previous commits and code reviews.

Details and procedure for adding and removing people from the Gerrit project owners groups.

MediaWiki has release branches (19 so far) for core, and master (the default branch on trunk). Example: [2]. ("Heads" is gitweb's term for branches.) MediaWiki core and WMF-deployed extensions will be tagging releases just as we did in Subversion, except they'll be Git tags instead of SVN tags. Any other extension will make its own decisions regarding tagging.

MediaWiki extensions that the Wikimedia Foundation deploys

Same procedure as for MediaWiki core, and the same Gerrit project owner groups.

Other MediaWiki extensions

Every extension author can choose between two choices here: the gated-trunk/push-for-review model, and a straight push model. For any given extension, we will honor the wishes of the person/s listed as the main author on the extension's mediawiki.org page.

The gated-trunk/push-for-review is the model that we are using for MediaWiki core, as mentioned above. A Gerrit project owners group (plus the above mentioned Gerrit project owners group for MediaWiki core) will be able to "+2" (approve and merge) changes to their extensions. The extension author(s) will be able to define a Gerrit project owners group and add others to it.
The straight push model is similar to how we did things in Subversion; anyone can suggest a change and submit a pull request, and it will automatically be approved and merged.

We could define groups to make this easier for batches of extensions (e.g. SMW developers). Chad will offer your community a choice. Please let Chad what you would like via Git/New repositories.

Other Gerrit projects

Same procedure as for "other MediaWiki extensions" above.

How to comment on, review, and merge code in Gerrit

A sample changeset, with annotations
Side-by-side diff
Review screen

Anyone can comment on code in Gerrit.

Viewing and commenting on code

  • Make sure you have a https://gerrit.wikimedia.org login (Get an account!). If you don't know, try logging in at https://labsconsole.wikimedia.org; the username and password should be the same. If you can't, ask in #mediawiki connect for someone to help.
  • Log in to Gerrit. If you know the changeset you want to look at (URL will look like https://gerrit.wikimedia.org/r/#change,2713 ), go to that. Otherwise, use the search box and try searching. There is no fulltext search in Gerrit, but you can search by author ("Owner"), Gerrit project, branch, changesets you've starred, etc.
  • The changeset has a few important fields, links and buttons:
  • Reviewers. 'gerrit2' is the autoreviewer that auto-verifies anything that passes the Jenkins tests. If it passes, you see a green checkmark. If it fails, a red X. A changeset needs a green checkmark before anyone can merge it. (In the future, the autoreviewer proxy usernames will change to be more indicative, "jenkins" or "lint".)
  • Add reviewer (manually ping someone to request their review. It'll show up in their Gerrit stream)
  • Side-by-side diff button:
  • Opens the diff in a new tab. You can double-click on a line and comment on that line, then save a draft comment! Then, click "Up to change" to go back to the changeset.
  • Abandon Change button (you'll see this if you wrote this diff. This action removes the diff from the merge queue, but leaves it in Gerrit for archival purposes)
  • Review button:
  • On this page you can leave an overall comment, view inline comments from the diff that are still in draft form and awaiting publication, and signal your thoughts on the commit. You can signal approval with a "+1" and disapproval with a "-1". These numbers are nonbinding, won't cause merges or rejections, and have no formal effect on the code review. To publish your draft inline comments plus your comments on the diff as a whole, click Publish.

Formally reviewing and merging or rejecting code

If you are one of the Gerrit project owners, you'll also see:

A Gerrit review screen with approval and veto options
  • Abandon Change button
  • on the Review page, additional Code Review options to +2 (approve) or -2 (veto) a diff, and a Publish And Submit button (publish your comment and merge diff into the branch, in 1 step)
  • Submit Patch Set 1 button (merge -- only useful if you or someone else has already given a +2 approval to the diff, but not merged it)

And once you've merged something into the example Gerrit project you'll see it in https://gerrit.wikimedia.org/r/gitweb?p=test/mediawiki/extensions/examples.git;a=summary .

If you merge a commit that references a Bugzilla bug, please go to that bug and mark it RESOLVED: FIXED and reference the merge ID.

Teacher: for this section, add the participant to the project owner group for the examples Gerrit project at https://gerrit.wikimedia.org/r/#admin,group,17 . Add the "Member" by email or wiki username.

Troubleshooting

Your change could not be merged due to a path conflict

If you get the error "Your change could not be merged due to a path conflict" when submitting a change set in Gerrit, you need to resolve the conflict in a manner very similar to how you would in SVN.

  1. Make sure your master branch is up to date: git pull master
  2. Create and switch to a new branch in which to checkout the change set with the conflict: git checkout -b BRANCHNAME
  3. Checkout the conflicting change set into this branch. You can copy/paste the correct command from the 'Download' section in the Gerrit review. It will look something like this: git fetch ssh://awjrichards@gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend refs/changes/38/3538/2 && git checkout FETCH_HEAD
  4. Rebase against master (this should give you a conflict warning. this is OK!): git rebase master
  5. Open the file(s) with the conflict, and resolve them. They will be denoted the same way they are in SVN (with <<<<<< and >>>>>>)
  6. Once you have finished resolving the conflicts, you need to git add the file again. For instance: git add HtmlFormatter.php
  7. Tell the rebase to continue now that the issue is resolved: git rebase --continue
  8. Push the resolved conflict to Gerrit for review: git review
  9. Re-review the change set in Gerrit, and then submit the changes to be merged to master.

Your change requires a recursive merge to resolve

If you get the error "Your change requires a recursive merge to resolve", you need to rebase the change set against master.

  1. Make sure your master branch is up to date: git pull master
  2. Create and switch to a new branch in which to checkout the change set with the conflict: git checkout -b BRANCHNAME
  3. Checkout the conflicting change set into this branch. You can copy/paste the correct command from the 'Download' section in the Gerrit review. It will look something like this: git fetch ssh://awjrichards@gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend refs/changes/14/3414/3 && git checkout FETCH_HEAD
  4. Rebase against master: git rebase master
  5. Push the change to Gerrit for review: git review
  6. Re-review the change set in Gerrit, and then submit the changes to be merged to master.

How to create a repository ("Gerrit project")

See "Request a new Git repository". There's a form to fill out. It should get processed very quickly (within a couple of days).

See also

Also useful are these pages:

Third party guides to Git

This might change. If you'd like to help, we're seeking test subjects! Also, John Du Hart would like for us to consider using Phabricator instead; we will consider this in June 2012.

Template:Git footer