CONTRIBUTING.md

   1 Authors: Jason Lowe-Power
   2          Andreas Sandberg
   3          Steve Reinhardt
   4
   5 If you've made changes to gem5 that might benefit others, we strongly encourage
   6 you to contribute those changes to the public gem5 repository. There are
   7 several reasons to do this:
   8  * Share your work with others, so that they can benefit from new functionality.
   9  * Support the scientific principle by enabling others to evaluate your
  10    suggestions without having to guess what you did.
  11  * Once your changes are part of the main repo, you no longer have to merge
  12    them back in every time you update your local repo. This can be a huge time
  13    saving!
  14  * Once your code is in the main repo, other people have to make their changes
  15    work with your code, and not the other way around.
  16  * Others may build on your contributions to make them even better, or extend
  17    them in ways you did not have time to do.
  18  * You will have the satisfaction of contributing back to the community.
  19
  20 The main method for contributing code to gem5 is via our code review website:
  21 https://gem5-review.googlesource.com/. This documents describes the details of
  22 how to create code changes, upload your changes, have your changes
  23 reviewed, and finally push your changes to gem5. More information can be found
  24 from the following sources:
  25  * http://gem5.org/Submitting_Contributions
  26  * https://gerrit-review.googlesource.com/Documentation/index.html
  27  * https://git-scm.com/book
  28
  29
  30 High-level flow for submitting changes
  31 ======================================
  32
  33     +-------------+
  34     | Make change |
  35     +------+------+
  36            |
  37            |
  38            v
  39     +------+------+
  40     | Post review |
  41     +------+------+
  42            |
  43            v
  44     +--------+---------+
  45     | Wait for reviews | <--------+
  46     +--------+---------+          |
  47            |                      |
  48            |                      |
  49            v                      |
  50       +----+----+   No     +------+------+
  51       |Reviewers+--------->+ Update code |
  52       |happy?   |          +------+------+
  53       +----+----+                 ^
  54            |                      |
  55            | Yes                  |
  56            v                      |
  57       +----+-----+   No           |
  58       |Maintainer+----------------+
  59       |happy?    |
  60       +----+-----+
  61            |
  62            | Yes
  63            v
  64     +------+------+
  65     | Submit code |
  66     +-------------+
  67
  68 After creating your change to gem5, you can post a review on our Gerrit
  69 code-review site: https://gem5-review.googlesource.com. Before being able to
  70 submit your code to the mainline of gem5, the code is reviewed by others in the
  71 community. Additionally, the maintainer for that part of the code must sign off
  72 on it.
  73
  74 Cloning the gem5 repo to contribute
  75 ===================================
  76
  77 If you plan on contributing, it is strongly encouraged for you to clone the
  78 repository directly from our gerrit instance at
  79 https://gem5.googlesource.com/.
  80
  81 To clone the master gem5 repository:
  82  > git clone https://gem5.googlesource.com/public/gem5
  83
  84 Other gem5 repositories
  85 -----------------------
  86
  87 There are a few repositories other than the main gem5 development repository.
  88
  89  * public/m5threads: The code for a pthreads implementation that works with
  90    gem5's syscall emulation mode.
  91
  92 Other gem5 branches
  93 -------------------
  94
  95 None right now.
  96
  97 Making changes to gem5
  98 ======================
  99
 100 It is strongly encouraged to use git branches when making changes to gem5.
 101 Additionally, keeping changes small and concise and only have a single logical
 102 change per commit.
 103
 104 Unlike our previous flow with Mercurial and patch queues, when using git, you
 105 will be committing changes to your local branch. By using separate branches in
 106 git, you will be able to pull in and merge changes from mainline and simply
 107 keep up with upstream changes.
 108
 109 Requirements for change descriptions
 110 ------------------------------------
 111 To help reviewers and future contributors more easily understand and track
 112 changes, we require all change descriptions be strictly formatted.
 113
 114 A canonical commit message consists of three parts:
 115  * A short summary line describing the change. This line starts with one or
 116    more keywords separated by commas followed by a colon and a description of
 117    the change. This line should be no more than 65 characters long since
 118    version control systems usually add a prefix that causes line-wrapping for
 119    longer lines.
 120  * (Optional, but highly recommended) A detailed description. This describes
 121    what you have done and why. If the change isn't obvious, you might want to
 122    motivate why it is needed. Lines need to be wrapped to 75 characters or
 123    less.
 124  * Tags describing patch metadata. You are highly recommended to use
 125    tags to acknowledge reviewers for their work. Gerrit will automatically add
 126    most tags.
 127
 128 The keyword should be one or more of the following separated by commas:
 129  * Architecture name in lower case (e.g., arm or x86): Anything that is
 130    target-architecture specific.
 131  * base
 132  * ext
 133  * stats
 134  * sim
 135  * syscall_emul
 136  * config:
 137  * mem: Classic memory system. Ruby uses its own keyword.
 138  * ruby: Ruby memory models.
 139  * cpu: CPU-model specific (except for kvm)
 140  * kvm: KVM-specific. Changes to host architecture specific components should
 141    include an architecture keyword (e.g., arm or x86) as well.
 142  * gpu-compute
 143  * energy
 144  * dev
 145  * arch: General architecture support (src/arch/)
 146  * scons: Build-system related. Trivial changes as a side effect of doing
 147    something unrelated (e.g., adding a source file to a SConscript) don't
 148    require this.
 149  * tests
 150  * style: Changes to the style checkers of style fixes.
 151  * misc
 152
 153 Tags are an optional mechanism to store additional metadata about a patch and
 154 acknowledge people who reported a bug or reviewed that patch. Tags are
 155 generally appended to the end of the commit message in the order they happen.
 156 We currently use the following tags:
 157  * Signed-off-by: Added by the author and the submitter (if different).
 158    This tag is a statement saying that you believe the patch to be correct and
 159    have the right to submit the patch according to the license in the affected
 160    files. Similarly, if you commit someone else's patch, this tells the rest
 161    of the world that you have have the right to forward it to the main
 162    repository. If you need to make any changes at all to submit the change,
 163    these should be described within hard brackets just before your
 164    Signed-off-by tag. By adding this line, the contributor certifies the
 165    contribution is made under the terms of the Developer Certificate of Origin
 166    (DCO) [https://developercertificate.org/].
 167  * Reviewed-by: Used to acknowledge patch reviewers. It's generally considered
 168    good form to add these. Added automatically.
 169  * Reported-by: Used to acknowledge someone for finding and reporting a bug.
 170  * Reviewed-on: Link to the review request corresponding to this patch. Added
 171    automatically.
 172  * Change-Id: Used by Gerrit to track changes across rebases. Added
 173    automatically with a commit hook by git.
 174  * Tested-by: Used to acknowledge people who tested a patch. Sometimes added
 175    automatically by review systems that integrate with CI systems.
 176
 177 Other than the "Signed-off-by", "Reported-by", and "Tested-by" tags, you
 178 generally don't need to add these manually as they are added automatically by
 179 Gerrit.
 180
 181 It is encouraged for the author of the patch and the submitter to add a
 182 Signed-off-by tag to the commit message. By adding this line, the contributor
 183 certifies the contribution is made under the terms of the Developer Certificate
 184 of Origin (DCO) [https://developercertificate.org/].
 185
 186 It is imperative that you use your real name and your real email address in
 187 both tags and in the author field of the changeset.
 188
 189 Note: If you do not follow these guidelines, the gerrit review site will
 190 automatically reject your patch.
 191 If this happens, update your changeset descriptions to match the required style
 192 and resubmit. The following is a useful git command to update the most recent
 193 commit (HEAD).
 194
 195  > git commit --amend
 196
 197 Posting a review
 198 ================
 199
 200 If you have not signed up for an account on the Gerrit review site
 201 (https://gem5-review.googlesource.com), you first have to create an account.
 202
 203 Setting up an account
 204 ---------------------
 205  1. Go to https://gem5.googlesource.com/
 206  2. Click "Sign In" in the upper right corner. Note: You will need a Google
 207  account to contribute.
 208  3. After signing in, click "Generate Password" and follow the instructions.
 209
 210 Submitting a change
 211 -------------------
 212
 213 In gerrit, to submit a review request, you can simply push your git commits to
 214 a special named branch. For more information on git push see
 215 https://git-scm.com/docs/git-push.
 216
 217 There are three ways to push your changes to gerrit.
 218
 219 Push change to gerrit review
 220 ----------------------------
 221
 222  > git push origin HEAD:refs/for/master
 223
 224 Assuming origin is https://gem5.googlesource.com/public/gem5 and you want to
 225 push the changeset at HEAD, this will create a new review request on top of the
 226 master branch. More generally,
 227
 228  > git push <gem5 gerrit instance> <changeset>:refs/for/<branch>
 229
 230 See https://gerrit-review.googlesource.com/Documentation/user-upload.html for
 231 more information.
 232
 233 Pushing your first change
 234 --------------------------
 235 The first time you push a change you may get the following error:
 236
 237  > remote: ERROR: [fb1366b] missing Change-Id in commit message footer
 238  > ...
 239
 240 Within the error message, there is a command line you should run. For every new
 241 clone of the git repo, you need to run the following command to automatically
 242 insert the change id in the the commit (all on one line).
 243
 244  > curl -Lo `git rev-parse --git-dir`/hooks/commit-msg
 245    https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; chmod +x
 246    `git rev-parse --git-dir`/hooks/commit-msg
 247
 248 If you receive the above error, simply run this command and then amend your
 249 changeset.
 250
 251  > git commit --amend
 252
 253 Push change to gerrit as a draft
 254 --------------------------------
 255
 256  > git push origin HEAD:refs/drafts/master
 257
 258 Push change bypassing gerrit
 259 -----------------------------
 260
 261 Only maintainers can bypass gerrit review. This should very rarely be used.
 262
 263  > git push origin HEAD:refs/heads/master
 264
 265 Other gerrit push options
 266 -------------------------
 267
 268 There are a number of options you can specify when uploading your changes to
 269 gerrit (e.g., reviewers, labels). The gerrit documentation has more
 270 information.
 271 https://gerrit-review.googlesource.com/Documentation/user-upload.html
 272
 273
 274 Reviewing patches
 275 =================
 276
 277 Reviewing patches is done on our gerrit instance at
 278 https://gem5-review.googlesource.com/.
 279
 280 After logging in with your Google account, you will be able to comment, review,
 281 and push your own patches as well as review others' patches. All gem5 users are
 282 encouraged to review patches. The only requirement to review patches is to be
 283 polite and respectful of others.
 284
 285 There are multiple labels in Gerrit that can be applied to each review detailed
 286 below.
 287  * Code-review: This is used by any gem5 user to review patches. When reviewing
 288    a patch you can give it a score of -2 to +2 with the following semantics.
 289    * -2: This blocks the patch. You believe that this patch should never be
 290      committed. This label should be very rarely used.
 291    * -1: You would prefer this is not merged as is
 292    * 0: No score
 293    * +1: This patch seems good, but you aren't 100% confident that it should be
 294      pushed.
 295    * +2: This is a good patch and should be pushed as is.
 296  * Maintainer: Currently only PMC members are maintainers. At least one
 297    maintainer must review your patch and give it a +1 before it can be merged.
 298  * Verified: This is automatically generated from the continuous integrated
 299    (CI) tests. Each patch must receive at least a +1 from the CI tests before
 300    the patch can be merged. The patch will receive a +1 if gem5 builds and
 301    runs, and it will receive a +2 if the stats match.
 302  * Style-Check: This is automatically generated and tests the patch against the
 303    gem5 code style (http://www.gem5.org/Coding_Style). The patch must receive a
 304    +1 from the style checker to be pushed.
 305
 306 Note: Whenever the patch creator updates the patch all reviewers must re-review
 307 the patch. There is no longer a "Fix it, then Ship It" option.
 308
 309 Once you have received reviews for your patch, you will likely need to make
 310 changes. To do this, you should update the original git changeset. Then, you
 311 can simply push the changeset again to the same Gerrit branch to update the
 312 review request.
 313
 314  > git push origin HEAD:refs/for/master
 315
 316 Note: If you have posted a patch and don't receive any reviews, you may need to
 317 prod the reviewers. You can do this by adding a reply to your changeset review
 318 on gerrit. It is expected that at least the maintainer will supply a review for
 319 your patch.
 320
 321 Committing changes
 322 ==================
 323
 324 Each patch must meet the following criteria to be merged:
 325  * At least one review with +2
 326  * At least one maintainer with +1
 327  * At least +1 from the CI tests (gem5 must build and run)
 328  * At least +1 from the style checker
 329
 330 Once a patch meets the above criteria, the submitter of the patch will be able
 331 to merge the patch by pressing the "Submit" button on Gerrit. When the patch is
 332 submitted, it is merged into the public gem5 branch.