misc: Updated CONTRIBUTING.md to reflect altered release policy
[gem5.git] / CONTRIBUTING.md
1 Authors: Jason Lowe-Power
2 Andreas Sandberg
3 Steve Reinhardt
4 Bobby R. Bruce
5
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
14 saving!
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.
20
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/contributing
27 * https://gerrit-review.googlesource.com/Documentation/index.html
28 * https://git-scm.com/book
29
30
31 High-level flow for submitting changes
32 ======================================
33
34 +-------------+
35 | Make change |
36 +------+------+
37 |
38 |
39 v
40 +-------------+
41 | Run tests |<--------------+
42 +------+------+ |
43 | |
44 | |
45 v |
46 +------+------+ |
47 | Post review | |
48 +------+------+ |
49 | |
50 v |
51 +--------+---------+ |
52 | Wait for reviews | |
53 +--------+---------+ |
54 | |
55 | |
56 v |
57 +----+----+ No +------+------+
58 |Reviewers+--------->+ Update code |
59 |happy? | +------+------+
60 +----+----+ ^
61 | |
62 | Yes |
63 v |
64 +----+-----+ No |
65 |Maintainer+----------------+
66 |happy? |
67 +----+-----+
68 |
69 | Yes
70 v
71 +------+------+
72 | Submit code |
73 +-------------+
74
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
79 on it.
80
81 Cloning the gem5 repo to contribute
82 ===================================
83
84 If you plan on contributing, it is strongly encouraged for you to clone the
85 repository directly, and checkout the `develop` branch from our gerrit instance
86 at https://gem5.googlesource.com/.
87
88 To clone the gem5 repository:
89
90 ```
91 git clone https://gem5.googlesource.com/public/gem5
92 ```
93
94 By default, the master branch is checked out. The master branch is stable,
95 containing the latest released version of gem5. To obtain code still
96 under-development (and which contributions can be made):
97
98 ```
99 cd gem5
100 git checkout --track origin/develop
101 ```
102
103 Changes should be made to this develop branch. Changes to the master branch
104 will be blocked. Once a change on the develop branch is properly incorporated
105 into the gem5 repo it will be merged into the master Branch upon the next
106 release of gem5. New releases of gem5 occur three times a year. Ergo, changes
107 made to the develop branch should appear on the master branch within three to
108 four months as part of a stable release.
109
110 Other gem5 repositories
111 -----------------------
112
113 There are a few repositories other than the main gem5 development repository.
114
115 * public/m5threads: The code for a pthreads implementation that works with
116 gem5's syscall emulation mode.
117
118
119 Making changes to gem5
120 ======================
121
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
124 change per commit.
125
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.
130
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.
135
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
145 less.
146 * Tags describing patch metadata. You are highly recommended to use
147 tags to acknowledge reviewers for their work. Gerrit will automatically add
148 most tags.
149
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
168 automatically.
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.
173
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
176 Gerrit.
177
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/].
182
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.
187
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.
190
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).
195
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
200 commit (HEAD).
201
202 ```
203 git commit --amend
204 ```
205
206 Running tests
207 =============
208
209 Before posting a change to the code review site, you should always run the
210 quick tests!
211 See TESTING.md for more information.
212
213 Posting a review
214 ================
215
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.
218
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.
225
226 Submitting a change
227 -------------------
228
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.
232
233 There are three ways to push your changes to gerrit.
234
235 Push change to gerrit review
236 ----------------------------
237
238 ```
239 git push origin HEAD:refs/for/develop
240 ```
241
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 develop branch. More generally,
245
246 ```
247 git push <gem5 gerrit instance> <changeset>:refs/for/<branch>
248 ```
249
250 See https://gerrit-review.googlesource.com/Documentation/user-upload.html for
251 more information.
252
253 Pushing your first change
254 --------------------------
255 The first time you push a change you may get the following error:
256
257 ```
258 remote: ERROR: [fb1366b] missing Change-Id in commit message footer
259 ...
260 ```
261
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).
265
266 ```
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
270 ```
271
272 If you receive the above error, simply run this command and then amend your
273 changeset.
274
275 ```
276 git commit --amend
277 ```
278
279 Push change to gerrit as a Work In Progress
280 -------------------------------------------
281
282 It is acceptable to push commits as "Work In Progress" (WIP) changes within
283 gerrit. WIP changes are publicly visible though no one will be able to review
284 the changes or be directly notified they have been submitted. WIP changes can
285 be useful for backing up code currently under-development or for sharing
286 incomplete code with the wider community (i.e., the link to the gerrit change
287 may be shared, and others may download the change, comment on it, and track
288 alterations over time).
289
290 See https://gerrit-review.googlesource.com/Documentation/intro-user.html#wip
291 for details on WIP gerrit changes.
292
293 To push a change as a WIP:
294
295 ```
296 git push origin HEAD:refs/for/develop%wip
297 ```
298
299 Once you have pushed your change as a WIP, you can log onto [gerrit](
300 https://gem5-review.googlesource.com) and view it. Once you're happy with the
301 change you can add reviewers which shall move your change from WIP status
302 to be considered for submission by the wider gem5 community. Switching from a
303 WIP to a regular change does not notify the gem5 community, via the gem5-dev
304 mailing-list, that a change has been submitted (as would occur if a change were
305 submitted directly for review). It is therefore important to include reviewers
306 and CC those who you wish to view the change (they will be notified
307 automatically via email).
308
309 Push change bypassing gerrit
310 -----------------------------
311
312 Only maintainers can bypass gerrit review. This should very rarely be used.
313
314 ```
315 git push origin HEAD:refs/heads/develop
316 ```
317
318 Other gerrit push options
319 -------------------------
320
321 There are a number of options you can specify when uploading your changes to
322 gerrit (e.g., reviewers, labels). The gerrit documentation has more
323 information.
324 https://gerrit-review.googlesource.com/Documentation/user-upload.html
325
326 Branches
327 ========
328
329 By default, contributions to gem5 should be made on the develop branch. The
330 master branch is maintained as a stable release branch (i.e., it can be pulled
331 to obtain the latest official release of gem5). Creation of additional branches
332 is generally discouraged due to their tendency to bloat git repositories with
333 abandoned code. However, the creation of new branches is permitted for
334 development of a specific feature or improvement if one or more of the
335 following criteria are met:
336
337 1. The feature/improvement is likely to be of a large size, consisting of many
338 commits, with little logic in these commits being contributed separately.
339 2. The feature/improvement will be developed over a long period of time.
340 3. There is sufficient reason that a feature/improvement should not be part
341 of the next gem5 release (e.g., the change should be held within a feature
342 branch until ready for the next release, at which point it will be merged
343 into the develop branch).
344
345 If a branch is required it can only be created by a project maintainer.
346 Therefore, if a gem5 contributor desires a separate branch for their work, they
347 should request one from the maintainer of the component the work relates to
348 (see MAINTAINERS for the list of maintainers and the components they are
349 responsible for). **The maintainer shall use their discretion to determine
350 whether the creation of a branch is necessary**. If approved, the maintainer
351 shall create the branch which the contributor may then use.
352
353 Development on a branch within Gerrit functions in exactly the same way as
354 contributing to the develop branch. When contributors to a branch are
355 satisfied, they should create a merge commit into the develop branch. The
356 maintainer should then be notified that the branch they created can now be
357 deleted.
358
359 **Abandonment of changes within branches may result in these branches being
360 removed from the repository. All branches within a repo should be under active
361 development.**
362
363 Reviewing patches
364 =================
365
366 Reviewing patches is done on our gerrit instance at
367 https://gem5-review.googlesource.com/.
368
369 After logging in with your Google account, you will be able to comment, review,
370 and push your own patches as well as review others' patches. All gem5 users are
371 encouraged to review patches. The only requirement to review patches is to be
372 polite and respectful of others.
373
374 There are multiple labels in Gerrit that can be applied to each review detailed
375 below.
376 * Code-review: This is used by any gem5 user to review patches. When reviewing
377 a patch you can give it a score of -2 to +2 with the following semantics.
378 * -2: This blocks the patch. You believe that this patch should never be
379 committed. This label should be very rarely used.
380 * -1: You would prefer this is not merged as is
381 * 0: No score
382 * +1: This patch seems good, but you aren't 100% confident that it should be
383 pushed.
384 * +2: This is a good patch and should be pushed as is.
385 * Maintainer: Currently only PMC members are maintainers. At least one
386 maintainer must review your patch and give it a +1 before it can be merged.
387 * Verified: This is automatically generated from the continuous integrated
388 (CI) tests. Each patch must receive at least a +1 from the CI tests before
389 the patch can be merged. The patch will receive a +1 if gem5 builds and
390 runs, and it will receive a +2 if the stats match.
391 * Style-Check: This is automatically generated and tests the patch against the
392 gem5 code style
393 (http://www.gem5.org/documentation/general_docs/development/coding_style/).
394 The patch must receive a +1 from the style checker to be pushed.
395
396 Note: Whenever the patch creator updates the patch all reviewers must re-review
397 the patch. There is no longer a "Fix it, then Ship It" option.
398
399 Once you have received reviews for your patch, you will likely need to make
400 changes. To do this, you should update the original git changeset. Then, you
401 can simply push the changeset again to the same Gerrit branch to update the
402 review request.
403
404 ```
405 git push origin HEAD:refs/for/develop
406 ```
407
408 Committing changes
409 ==================
410
411 Each patch must meet the following criteria to be merged:
412 * At least one review with +2
413 * At least one maintainer with +1
414 * At least +1 from the CI tests (gem5 must build and run)
415 * At least +1 from the style checker
416
417 Once a patch meets the above criteria, the submitter of the patch will be able
418 to merge the patch by pressing the "Submit" button on Gerrit. When the patch is
419 submitted, it is merged into the public gem5 branch.
420
421 Review moderation and guidelines
422 --------------------------------
423
424 Once a change is submitted, reviewers shall review the change. This may require
425 several iterations before a merge. Comments from reviewers may include
426 questions, and requests for alterations to the change prior to merging. The
427 overarching philosophy in managing this process is that there should be
428 politeness and clear communication between all parties at all times, and,
429 whenever possible, permission should be asked before doing anything that may
430 inconvenience another party. Included below are some guidelines we expect
431 contributors and reviewers to follow.
432
433 * In all forms of communication, contributors and reviewers must be polite.
434 Comments seen as being needlessly hostile or dismissive will not be
435 tolerated.
436 * Change contributors should respond to, or act upon, each item of feedback
437 given by reviewers. If there is disagreement with a piece of
438 feedback, a sufficiently detailed reason for this disagreement should
439 be given. Polite discussion, and sharing of information and expertise
440 is strongly encouraged.
441 * Contributors are advised to assign reviewers when submitting a change.
442 Anyone who contributes to gem5 can be assigned as a reviewer. However,
443 all changes must be accepted by at least one maintainer prior to a
444 merge, ergo assigning of at least one maintainer as a reviewer is
445 strongly recommended. Please see MAINTAINERS for a breakdown of
446 gem5 maintainers and which components they claim responsibility for.
447 Maintainers should be chosen based on which components the change is
448 targeting. Assigning of reviewers is not strictly enforced, though not
449 assigning reviewers may slow the time in which a change is reviewed.
450 * If a contributor posts a change and does not receive any reviews after two
451 working days (excluding regional holidays), it is acceptable to "prod"
452 reviewers. This can be done by adding a reply to the changeset review
453 (e.g., "Would it be possible for someone to review my change?"). If the
454 contributor has yet to assign reviewers, they are strongly advised to do so.
455 Reviewers will get notified when assigned to referee a change.
456 * By default, the original contributor is assumed to own a change. I.e.,
457 they are assumed to be the sole party to submit patchsets. If someone
458 other than the original contributor wishes to submit patchsets to a
459 change on the original contributor's behalf, they should first ask
460 permission. If two working days pass without a response, a patchset may be
461 submitted without permission. Permission does not need to be asked to submit
462 a patchset consisting of minor, inoffensive, changes such a typo and format
463 fixes.
464 * Once a change is ready to merge, it enters a "Ready to Submit" state. The
465 original contributor should merge their change at this point, assuming they
466 are content with the commit in its present form. After two working days, a
467 reviewer may message a contributor to remind them of the change being in a
468 "Ready to Submit" state and ask if they can merge the change on the
469 contributors behalf. If a further two working days elapse without a
470 response, the reviewer may merge without permission. A contributor may keep
471 a change open for whatever reason though this should be communicated to the
472 reviewer when asked.
473 * After a month of inactivity from a contributor on an active change, a
474 reviewer may post a message on the change reminding the submitter, and
475 anyone else watching the change, of its active status and ask if they are
476 still interested in eventually merging the change. After two weeks of no
477 response the reviewer reserves the right to abandon the change under the
478 assumption there is no longer interest.
479 * The final arbiter in any dispute between reviewers and/or contributors
480 is the PMC (PMC members are highlighted in MAINTAINERS). Disputes requiring
481 intervention by the PMC are undesirable. Attempts should be made to resolve
482 disagreements via respectful and polite discourse before being escalated to
483 this level.
484
485 Releases
486 ========
487
488 gem5 releases occur 3 times per year. The procedure for releasing gem5 is as
489 follows:
490
491 1. Developers will be notified, via the gem5-dev mailing list, that a new
492 release of gem5 will occur. This should be no sooner than 2 weeks prior to the
493 creation of the staging branch (the first step in releasing a new version of
494 gem5). This gives time for developers to ensure their changes for the next
495 release are submitted to the develop branch.
496 2. When a release is ready, a new staging branch shall be created by a project
497 maintainer, from develop, with the name "release-staging-{VERSION}". The
498 gem5-dev mailing list will be notified that the staging branch will be merged
499 into the master branch after two weeks, thus marking the new release.
500 3. The staging branch will have the full suite of gem5 tests run on it to
501 ensure all tests pass and the to-be-released code is in a decent state.
502 4. If a user submits a changeset to the staging branch, it will be considered
503 and undergo the standard Gerrit review process. However, only alterations that
504 cannot wait until the following release will be accepted for submission into
505 the branch (i.e., submissions to the staging branch for "last minute"
506 inclusions to the release should be of a high priority, such as a critical bug
507 fix). The project maintainers will use their discretion in deciding whether a
508 change may be submitted directly to the staging branch. All other submissions
509 to gem5 will continue to be made to the develop branch. Patches submitted
510 into the staging branch do not need to be re-added to the develop branch.
511 5. Once signed off by members of the PMC the staging branch shall be merged
512 into the master and develop branch. The staging branch will then be deleted.
513 6. The master branch shall be tagged with the correct version number for that
514 release. gem5 conforms to a "v{YY}.{MAJOR}.{MINOR}.{HOTFIX}" versioning system.
515 E.g., the first major release of 2022 will be "v22.0.0.0", followed by
516 "v22.1.0.0". All the releases (with the exception of hotfixes) are considered
517 major releases. For the meantime, there are no minor releases though we keep
518 the minor release numbers in case this policy changes in the future.
519 7. The gem5-dev and gem5-user mailing lists shall be notified of the new gem5
520 release.
521
522 Hotfixes
523 --------
524
525 There may be circumstances in which a change to gem5 is deemed critical and
526 cannot wait for an official release (e.g., a high-priority bug fix). In these
527 circumstances a hotfix shall be made.
528
529 First, if a developer suspects a hotfix may be necessary then the issue
530 should be discussed on the gem5-dev mailing list. The community will decide
531 whether the issue is worthy of a hotfix, and the final decision should be
532 made by members of the PMC if there is no consensus. Assuming the hotfix is
533 permitted, the following steps will be taken:
534
535 1. A new branch with the prefix "hotfix-" will be created from the master
536 branch. Only gem5 maintainers can create branches. If a non-maintainer requires
537 the creation of a hotfix branch then they should contact a gem5 maintainer.
538 2. The change shall be submitted to the hotfix branch via gerrit. Full review,
539 as with any other change, will be required.
540 3. Once fully submitted, the hotfix branch shall be merged into both the
541 develop and the master branch by a gem5 maintainer.
542 4. The master branch will be tagged with the new version number; the same as
543 the last but with an incremented hotfix number (e.g., "v20.2.0.0" would
544 transition to "v20.2.0.1").
545 4. The hotfix branch will then be deleted.
546 5. The gem5-dev and the gem5-user mailing lists shall be notified of this
547 hotfix.