-# <a name="Git and gerrit for developers"></a> Git and gerrit for developers
-
-Git opens up a number of new options for contributing to OpenAFS. For the first time, it is easy to review code that is pending addition to the OpenAFS tree. In fact, reviewing code is one of the best ways to ensure that the releases that OpenAFS ships remain stable and functional. If you are interested purely in reviewing, then please skip to that section towards the end of this document.
-
-Git also changes the way that developers interact with the OpenAFS tree. Instead of just having a single version of the tree on your local machine, you have a compressed copy of the entire repository. Additionally, you no longer have to produce patches to send code upstream - any developer can push into the OpenAFS repository directly, through gerrit, our code review tool.
-
-Whilst git is a far more powerful tool than CVS it is also, inevitably, more complex. This document can only scratch the surface of what's possible with git - there are many, many, documents available that describe git in greater detail, and references to some of them are provided at the end.
+Git opens up a number of new options for contributing to OpenAFS. For the first
+time, it is easy to review code that is pending addition to the OpenAFS tree.
+In fact, reviewing code is one of the best ways to ensure that the releases
+that OpenAFS ships remain stable and functional. If you are interested purely
+in reviewing, then please skip to that section towards the end of this
+document.
+
+Git also changes the way that developers interact with the OpenAFS tree.
+Instead of just having a single version of the tree on your local machine, you
+have a compressed copy of the entire repository. Additionally, you no longer
+have to produce patches to send code upstream - any developer can push into the
+OpenAFS repository directly, through gerrit, our code review tool.
+
+Whilst git is a far more powerful tool than CVS it is also, inevitably, more
+complex. This document can only scratch the surface of what's possible with git
+- there are many, many, documents available that describe git in greater
+detail, and references to some of them are provided at the end.
## <a name="Getting git"></a> Getting git
-Firstly, if your machine doesn't already have it installed, get a copy of the 'git' version control system. This is available for many platforms from their upstream package repositories or, failing that, can be downloaded in both source and binary form from <http://git-scm.com/download>
+Firstly, if your machine doesn't already have it installed, get a copy of the 'git' version control system. This is available for many platforms from their upstream package repositories or, failing that, can be downloaded in both source and binary form from
## <a name="Getting the _OpenAFS repository"></a> Getting the OpenAFS repository
git checkout openafs-stable-1_4_10
-Again, whilst a direct checkout of a remote tag is fine for code browsing, it should not be used as a place to start development. If you must do development against a tag, then create a local topic branch with it as a starting point, as is discussed below. However, in general, please don't develop from a particular tag, but instead work from a branch tip. It makes it much easier to integrate your changes!
+Again, whilst a direct checkout of a remote tag is fine for code browsing, it
+should not be used as a place to start development. If you must do development
+against a tag, then create a local topic branch with it as a starting point, as
+is discussed below. However, in general, please don't develop from a particular
+tag, but instead work from a branch tip. It makes it much easier to integrate
+your changes!
## <a name="Viewing deltas"></a> Viewing deltas
-OpenAFS's original CVS repository used the concept of deltas as a means of grouping a large number of related changes into a single item, which could be easily fetched and referred to. In git, a delta should be simply a single commit. Deltas are represented by means of a special form of git tag, allowing you to locally view the change and commit message that corresponds to each one. In order to keep down the transfer size, deltas are not included in the repository you get when you do a git clone - there are over 10,000 delta references, and having them in your local repository can cause performance issues. If you really wish to be able to locally browse deltas, then run the following
+OpenAFS's original CVS repository used the concept of deltas as a means of
+grouping a large number of related changes into a single item, which could be
+easily fetched and referred to. In git, a delta should be simply a single
+commit. Deltas are represented by means of a special form of git tag, allowing
+you to locally view the change and commit message that corresponds to each one.
+In order to keep down the transfer size, deltas are not included in the
+repository you get when you do a git clone - there are over 10,000 delta
+references, and having them in your local repository can cause performance
+issues. If you really wish to be able to locally browse deltas, then run the
+following
git config --add remote.origin.fetch '+refs/deltas/*:refs/remotes/deltas/*'
git fetch origin
git show refs/remotes/deltas/<branch>/<delta>
-Sadly, historical accidents mean that not all of our deltas can be represented by means of single commit. Where this is the case, a delta-name will have a trailing -part-, where each of these numbers must be used to form the complete delta. This only applies to some deltas created before the git conversion - all deltas created from now on will be single commits.
+Sadly, historical accidents mean that not all of our deltas can be represented
+by means of single commit. Where this is the case, a delta-name will have a
+trailing -part-, where each of these numbers must be used to form the complete
+delta. This only applies to some deltas created before the git conversion - all
+deltas created from now on will be single commits.
## <a name="Introducing yourself to git"></a> Introducing yourself to git
-Before you begin development, you should let git know who you are. This provides it with a name, and email address, that is used to attribute all commits in your repository, and in any that you share code with.
+Before you begin development, you should let git know who you are. This
+provides it with a name, and email address, that is used to attribute all
+commits in your repository, and in any that you share code with.
git config user.name "Joe Bloggs"
git config user.email "joe.bloggs@example.org"
git config --global user.name "Joe Bloggs"
git config --global user.email "joe.bloggs@example.org"
-Note that this email address is the address by which you will be identified in [[OpenAFS]]'s revision history - it is also the address to which the gerrit code review tool will send all email related to the review of your code.
+Note that this email address is the address by which you will be identified in
+[[OpenAFS]]'s revision history - it is also the address to which the gerrit
+code review tool will send all email related to the review of your code.
+
+If you plan on making changes to OpenAFS (and why else would you be reading
+this?) you should probably also grab <b>The change id hook</b> described in
+<b>Registering With gerrit</b> below. You can grab and apply the hook before
+registering, and it'll make sure your pre-registration development has the
+appropriate change IDs in the log. The hook only applies to your openafs
+development, so you're not going to mess up any of your non-OpenAFS work.
+
+## <a name="Helpful git tips"></a> Helpful git tips
+
+Here are a few other git settings that may be helpful when working with the source.
+
+Prevent C labels from being treated as function names by git diff:
+
+ git config diff.default.xfuncname '^[[:alpha:]$_].*[^:]$'
-If you plan on making changes to OpenAFS (and why else would you be reading this?) you should probably also grab <b>The change id hook</b> described in <b>Registering With gerrit</b> below. You can grab and apply the hook before registering, and it'll make sure your pre-registration development has the appropriate change IDs in the log. The hook only applies to your openafs development, so you're not going to mess up any of your non-OpenAFS work.
+Changes the style used to indicate merge conflicts in source files:
+
+ git config merge.conflictstyle diff3
+
+Whitespace handling settings:
+
+ git config apply.whitespace fix
+ git config core.whitespace trailing-space,space-before-tab,indent-with-non-tab
+ git config config.cleanup whitespace
## <a name="Starting development"></a> Starting development
-We strongly recommend that you do all of your development upon 'topic branches' This allows you to isolate multiple unrelated changes, and makes it easier to keep your tree in sync with the upstream [[OpenAFS]] one.
+We strongly recommend that you do all of your development upon 'topic branches'
+This allows you to isolate multiple unrelated changes, and makes it easier to
+keep your tree in sync with the upstream [[OpenAFS]] one.
Before creating a new topic branch, running
## <a name="Registering with gerrit"></a> Registering with gerrit
-To register with gerrit, visit <http://gerrit.openafs.org/> and log in using any OpenID. OpenIDs are available from a large number of internet services, including Google Accounts. If you don't have one, <http://myid.net> is an OpenID provider that we know works with Gerrit, and that don't require much in the way of personal details to obtain an account. Note that we're aware of problems using LiveJournal OpenIDs to access the service.
+To register with gerrit, visit <http://gerrit.openafs.org/> and log in using any OpenID. OpenIDs are available from a large number of internet services, including Google Accounts. Note that we're aware of problems using LiveJournal OpenIDs to access the service.
If, when you introduced yourself to git, you gave it a different email address than the one gerrit currently has listed for you, you need to introduce yourself to gerrit under that name, too. Click on 'Contact Information', and select "Register New Email...". If gerrit thinks you are an "Anonymous Coward" then you can fix that on this page as well.
First, each commit should be complete. The maxim "one change per commit, one commit per change" applies here. Each commit should build and test in its own right. Typically, this means that a change will be in a small number of commits. If, during development, you have created many more than this (for example, you've created a large number of bug fix commits), please use 'git rebase', or cherry pick these commits into a separate tree before uploading them. Note, however, that "one change" could equate to a change to source code and a change to the corresponding documentation for that code specific change.
-Secondly, each commit should have a meaningful revision log. The internals of git means that we can't easily edit these before pushing them into the tree, so we'd like you to get them right for us! A commit message should be comprised of a single 'subject' line (which must **not** end with a full stop), followed by a blank line, followed by one or more paragraphs explaining the purpose of the patch. If it is intended to fix a bug in OpenAFS RT, then the word 'FIXES' followed by the bug number or comma-separated list of bug numbers should be included on a line of its own. The 'LICENSE' keyword can be used to indicate code which is covered under a license other than the IPL, although please speak to a gatekeeper if you intend using this. An example commit message would be
+Secondly, each commit should have a meaningful revision log. The internals of git means that we can't easily edit these before pushing them into the tree, so we'd like you to get them right for us! A commit message should be comprised of a single 'subject' line (which must **not** end with a full stop), followed by a blank line, followed by one or more paragraphs explaining the purpose of the patch. Gerrit warns if the 'subject' is longer than 65 characters. If it is intended to fix a bug in OpenAFS RT, then the word 'FIXES' followed by the bug number or comma-separated list of bug numbers should be included on a line of its own. The 'LICENSE' keyword can be used to indicate code which is covered under a license other than the IPL, although please speak to a gatekeeper if you intend using this. An example commit message would be
Add option to disable syscall probing
Thirdly, each commit should have a valid changeID. Manually maintaining these is difficult and error prone, so we would strong advise that you install the changeID hook detailed earlier. This will automatically add a [[ChangeId]] line to your commit message if it doesn't already contain one.
-Once you have commits in this form, use
+Fourthly, each commit must adhere to the OpenAFS whitespace policy whereby new commits will not be accepted if they have trailing spaces, spaces before tabs, or indentation without tabs. Git can be configured to highlight the whitespace policy violation with the following global setting:
+
+ git config --global core.whitespace trailing-space,space-before-tab,indent-with-non-tab
+
+and
+
+ git rebase --whitespace=fix
+
+can be used to automatically fix any policy violations on your local branch before pushing the changes to Gerrit. Finally, Git 1.8.2 and above can be configured to apply this policy to all local commits:
+
+ git config --global config.cleanup whitespace
+
+Then commit your changes, for example with a file that contains your log message
+
+ git commit -a -F <your-log-message-in-this-file>
+
+(where <your-log-message-in-this-file> is a file with the log message)
+
+Once your commits have a proper commit message and have all whitespace errors fixed, use
git log -p origin/<branch>..HEAD
(where <branch> is the upstream branch upon which you are basing this patch).
-to check that what you're giving us makes sense. Then, upload them to gerrit using
+to check that what you're giving us makes sense. Then, upload the commits to gerrit using
+
+ git push ssh://gerrit.openafs.org/openafs.git HEAD:refs/for/<branch>/<topic>
- git push ssh://gerrit.openafs.org/openafs.git HEAD:refs/for/<branch>
+(again <branch> is the name of the upstream branch that you are pushing the changes into, not the name of any local branch you may have been developing on and <topic> is an optional name that can be used to group your commits together for easier reviewing.)
-(again <branch> is the name of the upstream branch that you are pushing the changes into, not the name of any local branch you may have been developing on)
+In this case, refs/for is a literal string. So, if you had been developing against the "master" branch and the change replaced "strcpy" with "strlcpy", you might upload your changes with:
-In this case, refs/for is a literal string. So, if you had been developing against master, you can upload your changes with:
+ git push ssh://gerrit.openafs.org/openafs.git HEAD:refs/for/master/strcpy-to-strlcpy
+
+Although, it would be sufficient to simply issue the command as:
git push ssh://gerrit.openafs.org/openafs.git HEAD:refs/for/master
(assuming your patch is against the 'master' git branch, and lives on the <topic> branch)
-And then simply resubmit your change in the same way as if you had been asked to revise it (see notes above)
+When a rebase is performed there may be conflicts that cannot be automatically resolved by git. The default style of conflict resolution displays the current version of the code on HEAD and the version from the commit that is being rebased. This level of detail is often insufficient to determine how to resolve the conflict. Switching to conflict style "diff3" will also show the original version of the code which your commit modified. Turn on "diff3" by applying the following configuration setting:
+
+ git config --global merge.conflictstyle diff3
+
+After you have resolved all conflicts and are once again happy with the commit, simply resubmit your change in the same way as if you had been asked to revise it (see notes above)
## <a name="Submitting by patch"></a> Submitting by patch
will give all of the changes between the branch point of you topic branch (assuming you branched from 'master') and the last commit.
-You can send those into <openafs-bugs@openafs.org> as before. Note, however, by doing this you're making someone else take the patch, create a topic branch in their local tree, apply the patch, and push it into gerrit. Things would be much more efficient if you pushed into gerrit yourself. Please?
+A better approach is to generate a patch file. To do so commit your changes to a local branch in your repository as you would if you were submitting to gerrit. If your changes are against the "master" branch, instead of pushing the patch execute the command:
+
+ git format-patch origin/master..HEAD
+
+For each commit on your local branch after the most recent patch on "master", a separate patch file will be generated.
+
+Regardless of which approach you use, you can e-mail the changes to <openafs-bugs@openafs.org> as before. Note, however, by doing this you're making someone else take the patch, create a topic branch in their local tree, apply the patch, push it into gerrit, and become responsible for managing the review process. Things would be much more efficient if you pushed into gerrit yourself. Please?
## <a name="Reviewing changes"></a> Reviewing changes
We'll now look at how changes that have made it into gerrit can be reviewed. All code review now happens via the <http://gerrit.openafs.org> interface. You should log in there as detailed above (using any OpenID), and make sure that the email address points to somewhere you'll read regularly.
-You'll be presented with a list of patches requiring review or, if someone has asked, patches you've been explicitly requested to review. There are two types of review - Code Review and Verification. Code Review means that you have read through the code, and are satisified that it works properly, follows the tree's style, and generally doesn't suck. Verification means that you have taken a copy of the patch and tested it. We hope to eventually automate the verification step, but for now both must be perfomed by hand.
+You'll be presented with a list of patches requiring review or, if someone has asked, patches you've been explicitly requested to review. There are two types of review - Code Review and Verification. Code Review means that you have read through the code, and are satisfied that it works properly, follows the tree's style, and generally doesn't suck. Verification means that you have taken a copy of the patch and tested it. We hope to eventually automate the verification step, but for now both must be performed by hand.
To perform a code review, go through each of the diffs in the current changeset for the code you have decided to review. You can double click on a line to leave a comment. Once you have completed commenting, click on the 'Review' button that's about 3/4 of the way down the page containing the list of patch sets. You will then be asked to score the patch, with a range from -1 to +1. -1 means that you don't think the code should be applied, +1 means that it is good to apply. You can also leave further, general, comments for the patch submitter.
Gerrit Documentation <http://gerrit.googlecode.com/svn/documentation/2.0/index.html> (only the first 'User Guide' section of this document is relevant)
+
+
+Five advanced Git merge techniques <http://blog.ezyang.com/2010/01/advanced-git-merge/>
+
## <a name="Acknowledgments"></a> Acknowledgments
Thanks to everyone who has reviewed this document, and offered corrections and contributions.
git://git.openafs.org / openafs-wiki.git / blobdiff