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