Page History

Gerrit is primarily a code inspection review tool, but can also be used to track the status of patch as it is build tested, and finally merged to a branch. Developers submit patches to Gerrit using git, and there is a rich browser interface for performing inspectionscode reviews. Inspectors Reviewers can also pull the change directly to their own repository and inspect review the change locally.

To upload changes to Gerrit, you must first register and add an ssh key to your account. For details on setting up your Gerrit account and sending patches, please see the Gerrit documentation.

Gerrit can host many git repositories, and of course each of those repositories can contain many branches. For instance, these are some of the repositories on Gerrit today:

...

$ git clone -b master git://git.whamcloud.com/fs/lustre-release.git lustre-master
$ cd lustre-master

This will create the master branch in your repository, which should never be modified. Typically in git, one creates a new local branch for each change being made. The branch is created based on one of the local, pristine branches follow the main development branches, usually the master. A new local branch is usually create using checkout:

...

This creates a new branch based on on master called my-brilliant-idea, and also makes it the current branch.

...

[user]
        name = Random J. Developer
        email = random@developer.example.org

Now all all git commits commit commands in any repository will use this name/email regardless of which user account is used to do the modifications, if using $HOME/.gitconfig file. If you want to specify a different name or email for a specific repository, it is possible to add this information to the .git/config file in that specific repository. See commit comment details below.

For Lustre patches, the both the code needs to follow specific code style guidelines Lustre Code Style Guidelines (which is basically the Linux kernel CodingStyle, with extra details for Lustre), as do the commit comments for each patch. In order to help maintain the uniform code style, Lustre-specific Git commit hooks should be installed in all Lustre development trees. In the top-level directory of the checked-out Lustre repository install the hooks using:

...

This will run the build/prepare-commit-msg script from the currently-checked-out Lustre tree to run the modifications through the build/checkpatch.pl script for style errors, and the build/commit-msg script afterward to verify the format of the commit message itself.  The format of commit comments is described below.

Once you've made the change, and you 're ready to create an inspection, you commitwant to save the code for testing and reviewed by others, you commit the change. Before you commit in Git, you need to identify which new or changed files file(s) you want to commit using git add <filename> for a specific filename, or git add -u for all modified files. If you haven't added any new files, and you want to commit all the files that you have changed (which is the usual case), then you can use the -a option to commit:

$ git commit -avavs

This will put you into an editor to update your commit comment, and when you save it will commit the change locally. The -v option appends the diff to the edit buffer so you can review what is about to be committed. This is very useful to verify that you are committing what you want to commit, and not changes or files that are unrelated to the current change.  The -s option adds a Signed-off-by: line to the commit message, which is required for all patches submitted to Lustre.

It is best to commit relatively frequently to keep your changes limited to a single fix. If you notice other, unrelated, issues that need to be fixed, then it's best to put them in a separate commit on a separate branch, so they can be submitted to Gerrit independently for inspectionreview, testing, and landing. The stash command is handy for this:

...

Having good commit comments helps everyone that is working on the code. See Commit Comments for a detailed description of the commit comment style. If your commit does not include the Change-Id: line as described below (which should be added automatically by the commit-msg hook, see above), the patch will be automatically rejected at submission time.

Sample Commit message:

LU-nnnnnnn component: short description of change under 62 columns

A more detailed explanation of the change being made.  This can be as
as detailed as you'd like, possibly several paragraphs in length.

Please provide a detailed explanation of what problem was solved, a
a good high-level description of how it was solved, and which
 parts of 
the code were affected (including function names as
 needed, for easier 
searching).  Including specific error messages, stack traces, and
similar is also good.

Wrap lines at 70 columns or less.

Signed-off-by: Random J Developer <random@developer.example.org>
Change-Id: Ica9ed1612eab0c4673dee088f8b441d806c64932

...

Gerrit requires all changes to have the Change-Id: field, or it will be rejected. It should be added automatically by the .git/hooks/commit-commentmsg script. See Commit Comments for a detailed description of the Change-Id: field.

Submitting Patches for

...

Review, Testing, and Landing

A change request is created by pushing a patch to a special branch on the Gerrit repository. To create an change request for a patch in your local branch the local commit (or series of commits) needs to be pushed to the Gerrit review repository.

It is possible to push a series of dependent commits from a local branch in this same way, and each one will create a separate change in Gerrit. Patch series should be used to split complex code changes that are implementing a single larger feature into chunks that are easier to understand. These changes , review, test, and land. Changes in a patch series will be dependent on each other, so if one patch needs to be refreshed after an inspection a review or test failure it will cause all of the later patches to be refreshed also. That will clear all of the inspection review and test results and restart testing on all of the patches.

...

If you have a series of independent commits to be reviewed, each one should be in a separate local branch and pushed separately to Gerrit. This allows the patches to be inspectedreviewed, tested, and landed independently, and will avoid the overhead and delay of repeatedly inspectingreviewing, building, and testing patches that are only refreshed because of an earlier (unrelated) patch in a series being modified.

git push ssh://USER@review.whamcloud.com:29418/fs/lustre-release HEAD:refs/for/master

for the "the master" branch of the the fs/lustre-release repo, or:

git push ssh://USER@review.whamcloud.com:29418/fs/lustre-release HEAD:refs/for/b1b2_85

for the "b1_8" the b2_5 branch of the the fs/lustre-release repo, or:

git push ssh://USER@review.whamcloud.com:29418/tools/e2fsprogs HEAD:refs/for/master-lustre

for the "the master-lustre" branch of the the tools/e2fsprogs repo.

For convenience, you can add this to your ~/.ssh/config file:

Host review
  Hostname review.whamcloud.com
  Port 29418
  User \{YourUserid\}
  IdentityFile ~/.ssh/my-key-id_rsa

Creating an inspection a review request for a change against master (assuming the remote alias has been added to ssh config):

git push ssh://review/fs/lustre-release HEAD:refs/for/master

Creating an inspection request for a change against b1_8 (assuming the remote alias has been added to ssh config)Add the Gerrit server as a remote to your git repository:

$ git remote add pushreview ssh://review/fs/lustre-release HEAD:refs/for/b1_8

And add a the Gerrit server as a remote to your git repository:

$ git remote add review ssh://review/fs/lustre-release

Automatically Build and Test

Automatically Building and Testing a Patch

When the patch has been pushed to the Gerrit Gerrit fs/lustre-review repository, it will print a URL for that change set that should be added to the Jira ticket for tracking.  This should now happen automatically.

All patches submitted to Gerrit will be built automatically by Jenkins (formerly Hudson) with for a number of configurationsdifferent distro and kernel versions. Currently for the master branch this includes RHEL6 i686/x86_64 client/server, RHEL5 i686/x86_64 client, Ubuntu 10.04 RHEL7 x86_64 client, and SLES11 SP1 x86_64 client/server. The success/failure of these builds will be posted to the change in Gerrit as a link to the Hudson Jenkins build artifacts.

After a successful build on all of the configurations, one or more of the configurations will be regression tested automatically, and a link to the test results in Maloo will be posted to Gerrit. It is also possible to manually post test results to Maloo from your own testing.  Normally, there are four separate test sessions run for patches submitted to the master branch.  The majority of the test sessions will be enforced, which means that they must pass in order to land the patch, but in some cases they may be optional and are not required to pass before landing.

Automated regression testing for each patch should normally pass.  Automated regression testing for each patch should normally pass.  In some cases it might fail for a variety of reasons, such as a bug in the patch, an intermittent test failure that is present in the existing code, problems with the testing system, etc.  In all failure cases, the Maloo test failure(s) should be investigated as to the root cause.  Once the test failure is identified in Maloo, the Maloo failed test result should either be linked (AssociatedAssociate bug) to an existing LU ticket in Jira, or if necessary a new Jira ticket with details of the failure should be filed (RaisedRaise bug), and a comment added to Gerrit with the JIRA LU ticket number.  At this point the test could be resubmitted, if there are no other problems with the patch, or it can be refreshed if there are inspection review comments that need to be addressed (per below).

Requesting

...

Patch Review

In order to actually get a patch landed, you need to request at least two inspections reviews in Gerrit for the patch. This is done by visiting the Gerrit web page for the change (at the URL returned when the commit was pushed, or it can be found from the main your home page after logging in). Enter the name or email address for the inspectors reviewer and click the Add Reviewer button.

In conjunction with the two inspection review requests, the patch has to successfully build on all supported architecture/distro combinations, and pass the automatic testing (as reported by Maloo). Once the patch has passed two inspections reviews (+1 or +2 Review from inspectorsreviewers), built correctly (+1 Verified from Hudson/Jenkins), and has passed autotest (+1 Verified from Maloo) the patch can be submitted for landing by adding adding Gerrit Gatekeeper using the Add Reviewer.

Updating Inspection Requests

It is the responsibility of the patch submitter to request the reviewers to look at the patch or to request patch landing, if this is not happening in a timely manner, by adding a comment into the patch.

Updating Patches After Review

Gerrit makes it easy to update a patch after review, and doing this allows reviewers to see differences between the patches Gerrit makes it easy to update an inspection request with a new patch, and doing this allows inspectors to see differences between the patches so they only need to inspect review the changes between the patches instead of having to review the entire patch again. Also, the original inline comments are maintained and moved to the new patches. The key to keeping updated inspection review requests linked to the original patch is the Change-Id: field in the commit comment - this is what Gerrit uses to find the original patch to update.

A critical thing to point out is that you need submit a new version of the entire change patch - not just an update to the changepatch.

The easiest way to update the most recent commit (which is often the one you want to update), is to use "git commit --amend -a".  This will "add" any modifications in the current repository and merge them into the last commit.  If there are no changes, or "-a" is not used, it will only editing the most recent commit comment.  This is useful if you don't have a Change-Id: line in your commit message (because you didn't install the commit-msg hook, see Commit Comments above), but one was added automatically by Gerrit.  You can copy the Change-Id: line from the Gerrit web page for that change and paste it into the amended commit message.  Typically, the updated patch should also be rebased to the tip of the current branch before pushing it again, to ensure that it does not conflict with other changes that may have been landed since the patch was first developed.

Another way of modifying a series of existing changes is to use "git rebase -i <parent-branch>". Interactive rebase will display a list of patches in an editor, and you can reorder the patches, "edit" the patch, "reword" the commit comments, and "squash" two patches into a single patch. See the help text in the commit message editor window for more information. 

...

If updates or fixes need to be made to one of the patches, these updates should be merged into the original commit where the code was added, rather than being an additional patch at the end of the series. This can be done by running "git rebase -i <parent-branch>", then marking a particular patch for edit. That will cause the earlier patches in the series up to the to be applied, then stop for interactive editing/testing of that patch. Once the patch has been updated, run git add {modified files} -u to include the new updates into this patch, and git rebase --continue to merge the updates into the existing patch and continue the rebase process.

In the case where changes need to be based on a patch from another developer, it is possible to check out the desired patch from Gerrit using one of the supplied Git URLs in the Download section under the patchset. Select the checkout and Anonymous HTTP tags, then copy the URL and paste it into a working Lustre checkout, for example to fetch patchset 16 from http://review.whamcloud.com/1264, create a branch for it, and then create a branch for it, and then create a new branch for your local changes based on that change, usenew branch for your local changes based on that change, use:

[lustre]$ git fetch http://review.whamcloud.com/p/fs/lustre-release refs/changes/64/1264/16 && git checkout FETCH_HEAD
[lustre]$ git checkout -b b_1264
[lustre]$ git checkout -b b_my_changes
{edit local tree to make changes as usual}
[lustre]$ git commit -a

Any changes in the b_my_changes will be based on those of b_1264. When b_my_changes is pushed to Gerrit, it will have a dependency on change 1264, so it will not be able to land until change 1264 itself is landed.  It is typically not desirable rebase the b_1264 branch or your local branch to a new version of master or it will cause the other developer's patch to be updated in Gerrit, and lose its existing review and test results.

If you already have the changes in a local branch (e.g. b_my_changes) and want to rebase that branch on top of another uncommitted patch from Gerrit, the process is similar:

[lustre]$ git fetch http://review.whamcloud.com/p/fs/lustre-release refs/changes/64/1264/16 && git checkout FETCH_HEAD
[lustre]$ git checkout -b b_1264
[lustre]$ git checkout -b b_my_changes
[lustre]$ git rebase --onto b_1264

That will rebase the patch(es) in the current branch onto the other patch (or branch) checked out from GerritAny changes in the b_my_changes will be based on those of b_1264. When b_my_changes is pushed to Gerrit, it will have a dependency on change 1264, so it will not be able to land until change 1264 itself is landed.

Pushing a Git Tag

This is normally done by the branch maintainer, but is recorded here for future reference.

Switch to the branch you want to create a branch in (normally normally master or b1b2_85).

Create the tag with:

git tag -a TAG_NAME

...

Note, this requires certain privileges on the server. If If review is a secondary server for your repository, use:

...