CONTRIBUTING.md revision 11921:1b19c0bd7aa8
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 | <--------+
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
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 > git clone https://gem5.googlesource.com/public/gem5
83
84Other gem5 repositories
85-----------------------
86
87There 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
92Other gem5 branches
93-------------------
94
95None right now.
96
97Making changes to gem5
98======================
99
100It is strongly encouraged to use git branches when making changes to gem5.
101Additionally, keeping changes small and concise and only have a single logical
102change per commit.
103
104Unlike our previous flow with Mercurial and patch queues, when using git, you
105will be committing changes to your local branch. By using separate branches in
106git, you will be able to pull in and merge changes from mainline and simply
107keep up with upstream changes.
108
109Requirements for change descriptions
110------------------------------------
111To help reviewers and future contributors more easily understand and track
112changes, we require all change descriptions be strictly formatted.
113
114A 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
128The 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
153Tags are an optional mechanism to store additional metadata about a patch and
154acknowledge people who reported a bug or reviewed that patch. Tags are
155generally appended to the end of the commit message in the order they happen.
156We 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
177Other than the "Signed-off-by", "Reported-by", and "Tested-by" tags, you
178generally don't need to add these manually as they are added automatically by
179Gerrit.
180
181It is encouraged for the author of the patch and the submitter to add a
182Signed-off-by tag to the commit message. By adding this line, the contributor
183certifies the contribution is made under the terms of the Developer Certificate
184of Origin (DCO) [https://developercertificate.org/].
185
186It is imperative that you use your real name and your real email address in
187both tags and in the author field of the changeset.
188
189For significant changes, authors are encouraged to add copyright information
190and their names at the beginning of the file. The main purpose of the author
191names on the file is to track who is most knowledgeable about the file (e.g.,
192who has contributed a significant amount of code to the file).
193
194Note: If you do not follow these guidelines, the gerrit review site will
195automatically reject your patch.
196If this happens, update your changeset descriptions to match the required style
197and resubmit. The following is a useful git command to update the most recent
198commit (HEAD).
199
200 > git commit --amend
201
202Posting a review
203================
204
205If you have not signed up for an account on the Gerrit review site
206(https://gem5-review.googlesource.com), you first have to create an account.
207
208Setting up an account
209---------------------
210 1. Go to https://gem5.googlesource.com/
211 2. Click "Sign In" in the upper right corner. Note: You will need a Google
212 account to contribute.
213 3. After signing in, click "Generate Password" and follow the instructions.
214
215Submitting a change
216-------------------
217
218In gerrit, to submit a review request, you can simply push your git commits to
219a special named branch. For more information on git push see
220https://git-scm.com/docs/git-push.
221
222There are three ways to push your changes to gerrit.
223
224Push change to gerrit review
225----------------------------
226
227 > git push origin HEAD:refs/for/master
228
229Assuming origin is https://gem5.googlesource.com/public/gem5 and you want to
230push the changeset at HEAD, this will create a new review request on top of the
231master branch. More generally,
232
233 > git push <gem5 gerrit instance> <changeset>:refs/for/<branch>
234
235See https://gerrit-review.googlesource.com/Documentation/user-upload.html for
236more information.
237
238Pushing your first change
239--------------------------
240The first time you push a change you may get the following error:
241
242 > remote: ERROR: [fb1366b] missing Change-Id in commit message footer
243 > ...
244
245Within the error message, there is a command line you should run. For every new
246clone of the git repo, you need to run the following command to automatically
247insert the change id in the the commit (all on one line).
248
249 > curl -Lo `git rev-parse --git-dir`/hooks/commit-msg
250   https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; chmod +x
251   `git rev-parse --git-dir`/hooks/commit-msg
252
253If you receive the above error, simply run this command and then amend your
254changeset.
255
256 > git commit --amend
257
258Push change to gerrit as a draft
259--------------------------------
260
261 > git push origin HEAD:refs/drafts/master
262
263Push change bypassing gerrit
264-----------------------------
265
266Only maintainers can bypass gerrit review. This should very rarely be used.
267
268 > git push origin HEAD:refs/heads/master
269
270Other gerrit push options
271-------------------------
272
273There are a number of options you can specify when uploading your changes to
274gerrit (e.g., reviewers, labels). The gerrit documentation has more
275information.
276https://gerrit-review.googlesource.com/Documentation/user-upload.html
277
278
279Reviewing patches
280=================
281
282Reviewing patches is done on our gerrit instance at
283https://gem5-review.googlesource.com/.
284
285After logging in with your Google account, you will be able to comment, review,
286and push your own patches as well as review others' patches. All gem5 users are
287encouraged to review patches. The only requirement to review patches is to be
288polite and respectful of others.
289
290There are multiple labels in Gerrit that can be applied to each review detailed
291below.
292 * Code-review: This is used by any gem5 user to review patches. When reviewing
293   a patch you can give it a score of -2 to +2 with the following semantics.
294   * -2: This blocks the patch. You believe that this patch should never be
295     committed. This label should be very rarely used.
296   * -1: You would prefer this is not merged as is
297   * 0: No score
298   * +1: This patch seems good, but you aren't 100% confident that it should be
299     pushed.
300   * +2: This is a good patch and should be pushed as is.
301 * Maintainer: Currently only PMC members are maintainers. At least one
302   maintainer must review your patch and give it a +1 before it can be merged.
303 * Verified: This is automatically generated from the continuous integrated
304   (CI) tests. Each patch must receive at least a +1 from the CI tests before
305   the patch can be merged. The patch will receive a +1 if gem5 builds and
306   runs, and it will receive a +2 if the stats match.
307 * Style-Check: This is automatically generated and tests the patch against the
308   gem5 code style (http://www.gem5.org/Coding_Style). The patch must receive a
309   +1 from the style checker to be pushed.
310
311Note: Whenever the patch creator updates the patch all reviewers must re-review
312the patch. There is no longer a "Fix it, then Ship It" option.
313
314Once you have received reviews for your patch, you will likely need to make
315changes. To do this, you should update the original git changeset. Then, you
316can simply push the changeset again to the same Gerrit branch to update the
317review request.
318
319 > git push origin HEAD:refs/for/master
320
321Note: If you have posted a patch and don't receive any reviews, you may need to
322prod the reviewers. You can do this by adding a reply to your changeset review
323on gerrit. It is expected that at least the maintainer will supply a review for
324your patch.
325
326Committing changes
327==================
328
329Each patch must meet the following criteria to be merged:
330 * At least one review with +2
331 * At least one maintainer with +1
332 * At least +1 from the CI tests (gem5 must build and run)
333 * At least +1 from the style checker
334
335Once a patch meets the above criteria, the submitter of the patch will be able
336to merge the patch by pressing the "Submit" button on Gerrit. When the patch is
337submitted, it is merged into the public gem5 branch.
338