1 Authors: Jason Lowe-Power
6 If you've made changes to gem5 that might benefit others, we strongly encourage
7 you to contribute those changes to the public gem5 repository. There are
8 several reasons to do this:
9 * Share your work with others, so that they can benefit from new functionality.
10 * Support the scientific principle by enabling others to evaluate your
11 suggestions without having to guess what you did.
12 * Once your changes are part of the main repo, you no longer have to merge
13 them back in every time you update your local repo. This can be a huge time
15 * Once your code is in the main repo, other people have to make their changes
16 work with your code, and not the other way around.
17 * Others may build on your contributions to make them even better, or extend
18 them in ways you did not have time to do.
19 * You will have the satisfaction of contributing back to the community.
21 The main method for contributing code to gem5 is via our code review website:
22 https://gem5-review.googlesource.com/. This documents describes the details of
23 how to create code changes, upload your changes, have your changes
24 reviewed, and finally push your changes to gem5. More information can be found
25 from the following sources:
26 * http://gem5.org/Submitting_Contributions
27 * https://gerrit-review.googlesource.com/Documentation/index.html
28 * https://git-scm.com/book
31 High-level flow for submitting changes
32 ======================================
41 | Run tests |<--------------+
51 +--------+---------+ |
52 | Wait for reviews | |
53 +--------+---------+ |
57 +----+----+ No +------+------+
58 |Reviewers+--------->+ Update code |
59 |happy? | +------+------+
65 |Maintainer+----------------+
75 After creating your change to gem5, you can post a review on our Gerrit
76 code-review site: https://gem5-review.googlesource.com. Before being able to
77 submit your code to the mainline of gem5, the code is reviewed by others in the
78 community. Additionally, the maintainer for that part of the code must sign off
81 Contributing long-lived feature branches
82 ----------------------------------------
83 Oftentimes users or institutions add features that are necessarily complex,
84 and require many changes on long-lived feature branches. In this case,
85 maintaining a perfect history where all changes work individually is infeasible.
86 When contributing long-lived feature branches back to gem5's public repository
87 users may merge entire long-lived branches into a single changeset and contribute
88 their code back as long as 1) the changes have been reviewed by the maintainer
89 2) the maintainer agrees to allow such a change, and 3) the changes are passing
90 the public tests. Changes that affect common code (outside of a specific
91 maintainer's purview) will still need to follow the standard gem5 protocol.
94 Cloning the gem5 repo to contribute
95 ===================================
97 If you plan on contributing, it is strongly encouraged for you to clone the
98 repository directly from our gerrit instance at
99 https://gem5.googlesource.com/.
101 To clone the master gem5 repository:
103 git clone https://gem5.googlesource.com/public/gem5
106 Other gem5 repositories
107 -----------------------
109 There are a few repositories other than the main gem5 development repository.
111 * public/m5threads: The code for a pthreads implementation that works with
112 gem5's syscall emulation mode.
119 Making changes to gem5
120 ======================
122 It is strongly encouraged to use git branches when making changes to gem5.
123 Additionally, keeping changes small and concise and only have a single logical
126 Unlike our previous flow with Mercurial and patch queues, when using git, you
127 will be committing changes to your local branch. By using separate branches in
128 git, you will be able to pull in and merge changes from mainline and simply
129 keep up with upstream changes.
131 Requirements for change descriptions
132 ------------------------------------
133 To help reviewers and future contributors more easily understand and track
134 changes, we require all change descriptions be strictly formatted.
136 A canonical commit message consists of three parts:
137 * A short summary line describing the change. This line starts with one or
138 more keywords (found in the MAINTAINERS file) separated by commas followed
139 by a colon and a description of the change. This line should be no more than
140 65 characters long since version control systems usually add a prefix that
141 causes line-wrapping for longer lines.
142 * (Optional, but highly recommended) A detailed description. This describes
143 what you have done and why. If the change isn't obvious, you might want to
144 motivate why it is needed. Lines need to be wrapped to 75 characters or
146 * Tags describing patch metadata. You are highly recommended to use
147 tags to acknowledge reviewers for their work. Gerrit will automatically add
150 Tags are an optional mechanism to store additional metadata about a patch and
151 acknowledge people who reported a bug or reviewed that patch. Tags are
152 generally appended to the end of the commit message in the order they happen.
153 We currently use the following tags:
154 * Signed-off-by: Added by the author and the submitter (if different).
155 This tag is a statement saying that you believe the patch to be correct and
156 have the right to submit the patch according to the license in the affected
157 files. Similarly, if you commit someone else's patch, this tells the rest
158 of the world that you have have the right to forward it to the main
159 repository. If you need to make any changes at all to submit the change,
160 these should be described within hard brackets just before your
161 Signed-off-by tag. By adding this line, the contributor certifies the
162 contribution is made under the terms of the Developer Certificate of Origin
163 (DCO) [https://developercertificate.org/].
164 * Reviewed-by: Used to acknowledge patch reviewers. It's generally considered
165 good form to add these. Added automatically.
166 * Reported-by: Used to acknowledge someone for finding and reporting a bug.
167 * Reviewed-on: Link to the review request corresponding to this patch. Added
169 * Change-Id: Used by Gerrit to track changes across rebases. Added
170 automatically with a commit hook by git.
171 * Tested-by: Used to acknowledge people who tested a patch. Sometimes added
172 automatically by review systems that integrate with CI systems.
174 Other than the "Signed-off-by", "Reported-by", and "Tested-by" tags, you
175 generally don't need to add these manually as they are added automatically by
178 It is encouraged for the author of the patch and the submitter to add a
179 Signed-off-by tag to the commit message. By adding this line, the contributor
180 certifies the contribution is made under the terms of the Developer Certificate
181 of Origin (DCO) [https://developercertificate.org/].
183 If your change relates to a [Jira Issue](https://gem5.atlassian.net), it is
184 advised that you provide a link to the issue in the commit message (or messages
185 if the Jira Issue relates to multiple commits). Though optional, doing this
186 can help reviewers understand the context of a change.
188 It is imperative that you use your real name and your real email address in
189 both tags and in the author field of the changeset.
191 For significant changes, authors are encouraged to add copyright information
192 and their names at the beginning of the file. The main purpose of the author
193 names on the file is to track who is most knowledgeable about the file (e.g.,
194 who has contributed a significant amount of code to the file).
196 Note: If you do not follow these guidelines, the gerrit review site will
197 automatically reject your patch.
198 If this happens, update your changeset descriptions to match the required style
199 and resubmit. The following is a useful git command to update the most recent
209 Before posting a change to the code review site, you should always run the
211 See TESTING.md for more information.
216 If you have not signed up for an account on the Gerrit review site
217 (https://gem5-review.googlesource.com), you first have to create an account.
219 Setting up an account
220 ---------------------
221 1. Go to https://gem5.googlesource.com/
222 2. Click "Sign In" in the upper right corner. Note: You will need a Google
223 account to contribute.
224 3. After signing in, click "Generate Password" and follow the instructions.
229 In gerrit, to submit a review request, you can simply push your git commits to
230 a special named branch. For more information on git push see
231 https://git-scm.com/docs/git-push.
233 There are three ways to push your changes to gerrit.
235 Push change to gerrit review
236 ----------------------------
239 git push origin HEAD:refs/for/master
242 Assuming origin is https://gem5.googlesource.com/public/gem5 and you want to
243 push the changeset at HEAD, this will create a new review request on top of the
244 master branch. More generally,
247 git push <gem5 gerrit instance> <changeset>:refs/for/<branch>
250 See https://gerrit-review.googlesource.com/Documentation/user-upload.html for
253 Pushing your first change
254 --------------------------
255 The first time you push a change you may get the following error:
258 remote: ERROR: [fb1366b] missing Change-Id in commit message footer
262 Within the error message, there is a command line you should run. For every new
263 clone of the git repo, you need to run the following command to automatically
264 insert the change id in the the commit (all on one line).
267 curl -Lo `git rev-parse --git-dir`/hooks/commit-msg \
268 https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; \
269 chmod +x `git rev-parse --git-dir`/hooks/commit-msg
272 If you receive the above error, simply run this command and then amend your
279 Push change to gerrit as a draft/private
280 ----------------------------------------
282 See https://gerrit-review.googlesource.com/Documentation/intro-user.html#private-changes
283 for details on private gerrit changes.
286 git push origin HEAD:refs/for/master%private
289 Once you have pushed your change as "private", you can log onto [gerrit]
290 (https://gem5-review.googlesource.com) and once you're happy with the commit
291 click the "unmark private" which may be hidden in the "more options" dropdown
292 in the upper right corner.
294 Push change bypassing gerrit
295 -----------------------------
297 Only maintainers can bypass gerrit review. This should very rarely be used.
300 git push origin HEAD:refs/heads/master
303 Other gerrit push options
304 -------------------------
306 There are a number of options you can specify when uploading your changes to
307 gerrit (e.g., reviewers, labels). The gerrit documentation has more
309 https://gerrit-review.googlesource.com/Documentation/user-upload.html
315 Reviewing patches is done on our gerrit instance at
316 https://gem5-review.googlesource.com/.
318 After logging in with your Google account, you will be able to comment, review,
319 and push your own patches as well as review others' patches. All gem5 users are
320 encouraged to review patches. The only requirement to review patches is to be
321 polite and respectful of others.
323 There are multiple labels in Gerrit that can be applied to each review detailed
325 * Code-review: This is used by any gem5 user to review patches. When reviewing
326 a patch you can give it a score of -2 to +2 with the following semantics.
327 * -2: This blocks the patch. You believe that this patch should never be
328 committed. This label should be very rarely used.
329 * -1: You would prefer this is not merged as is
331 * +1: This patch seems good, but you aren't 100% confident that it should be
333 * +2: This is a good patch and should be pushed as is.
334 * Maintainer: Currently only PMC members are maintainers. At least one
335 maintainer must review your patch and give it a +1 before it can be merged.
336 * Verified: This is automatically generated from the continuous integrated
337 (CI) tests. Each patch must receive at least a +1 from the CI tests before
338 the patch can be merged. The patch will receive a +1 if gem5 builds and
339 runs, and it will receive a +2 if the stats match.
340 * Style-Check: This is automatically generated and tests the patch against the
341 gem5 code style (http://www.gem5.org/Coding_Style). The patch must receive a
342 +1 from the style checker to be pushed.
344 Note: Whenever the patch creator updates the patch all reviewers must re-review
345 the patch. There is no longer a "Fix it, then Ship It" option.
347 Once you have received reviews for your patch, you will likely need to make
348 changes. To do this, you should update the original git changeset. Then, you
349 can simply push the changeset again to the same Gerrit branch to update the
353 git push origin HEAD:refs/for/master
359 Each patch must meet the following criteria to be merged:
360 * At least one review with +2
361 * At least one maintainer with +1
362 * At least +1 from the CI tests (gem5 must build and run)
363 * At least +1 from the style checker
365 Once a patch meets the above criteria, the submitter of the patch will be able
366 to merge the patch by pressing the "Submit" button on Gerrit. When the patch is
367 submitted, it is merged into the public gem5 branch.
369 Review moderation and guidelines
370 --------------------------------
372 Once a change is submitted, reviewers shall review the change. This may require
373 several iterations before a merge. Comments from reviewers may include
374 questions, and requests for alterations to the change prior to merging. The
375 overarching philosophy in managing this process is that there should be
376 politeness and clear communication between all parties at all times, and,
377 whenever possible, permission should be asked before doing anything that may
378 inconvenience another party. Included below are some guidelines we expect
379 contributors and reviewers to follow.
381 * In all forms of communication, contributors and reviewers must be polite.
382 Comments seen as being needlessly hostile or dismissive will not be
384 * Change contributors should respond to, or act upon, each item of feedback
385 given by reviewers. If there is disagreement with a piece of
386 feedback, a sufficiently detailed reason for this disagreement should
387 be given. Polite discussion, and sharing of information and expertise
388 is strongly encouraged.
389 * Contributors are advised to assign reviewers when submitting a change.
390 Anyone who contributes to gem5 can be assigned as a reviewer. However,
391 all changes must be accepted by at least one maintainer prior to a
392 merge, ergo assigning of at least one maintainer as a reviewer is
393 strongly recommended. Please see MAINTAINERS for a breakdown of
394 gem5 maintainers and which components they claim responsibility for.
395 Maintainers should be chosen based on which components the change is
396 targeting. Assigning of reviewers is not strictly enforced, though not
397 assigning reviewers may slow the time in which a change is reviewed.
398 * If a contributor posts a change and does not receive any reviews after two
399 working days (excluding regional holidays), it is acceptable to "prod"
400 reviewers. This can be done by adding a reply to the changeset review
401 (e.g., "Would it be possible for someone to review my change?"). If the
402 contributor has yet to assign reviewers, they are strongly advised to do so.
403 Reviewers will get notified when assigned to referee a change.
404 * By default, the original contributor is assumed to own a change. I.e.,
405 they are assumed to be the sole party to submit patchsets. If someone
406 other than the original contributor wishes to submit patchsets to a
407 change on the original contributor's behalf, they should first ask
408 permission. If two working days pass without a response, a patchset may be
409 submitted without permission. Permission does not need to be asked to submit
410 a patchset consisting of minor, inoffensive, changes such a typo and format
412 * Once a change is ready to merge, it enters a "Ready to Submit" state. The
413 original contributor should merge their change at this point, assuming they
414 are content with the commit in its present form. After two working days, a
415 reviewer may message a contributor to remind them of the change being in a
416 "Ready to Submit" state and ask if they can merge the change on the
417 contributors behalf. If a further two working days elapse without a
418 response, the reviewer may merge without permission. A contributor may keep
419 a change open for whatever reason though this should be communicated to the
421 * After a month of inactivity from a contributor on an active change, a
422 reviewer may post a message on the change reminding the submitter, and
423 anyone else watching the change, of its active status and ask if they are
424 still interested in eventually merging the change. After two weeks of no
425 response the reviewer reserves the right to abandon the change under the
426 assumption there is no longer interest.
427 * The final arbiter in any dispute between reviewers and/or contributors
428 is the PMC (PMC members are highlighted in MAINTAINERS). Disputes requiring
429 intervention by the PMC are undesirable. Attempts should be made to resolve
430 disagreements via respectful and polite discourse before being escalated to