1 <!DOCTYPE HTML PUBLIC
"-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
4 <meta http-equiv=
"content-type" content=
"text/html; charset=utf-8">
5 <title>Submitting Patches
</title>
6 <link rel=
"stylesheet" type=
"text/css" href=
"mesa.css">
11 The Mesa
3D Graphics Library
14 <iframe src=
"contents.html"></iframe>
17 <h1>Submitting Patches
</h1>
21 <li><a href=
"#guidelines">Basic guidelines
</a>
22 <li><a href=
"#formatting">Patch formatting
</a>
23 <li><a href=
"#testing">Testing Patches
</a>
24 <li><a href=
"#submit">Submitting Patches
</a>
25 <li><a href=
"#reviewing">Reviewing Patches
</a>
26 <li><a href=
"#nominations">Nominating a commit for a stable branch
</a>
27 <li><a href=
"#criteria">Criteria for accepting patches to the stable branch
</a>
28 <li><a href=
"#backports">Sending backports for the stable branch
</a>
29 <li><a href=
"#gittips">Git tips
</a>
32 <h2 id=
"guidelines">Basic guidelines
</h2>
35 <li>Patches should not mix code changes with code formatting changes (except,
36 perhaps, in very trivial cases.)
37 <li>Code patches should follow Mesa
38 <a href=
"codingstyle.html" target=
"_parent">coding conventions
</a>.
39 <li>Whenever possible, patches should only affect individual Mesa/Gallium
41 <li>Patches should never introduce build breaks and should be bisectable (see
42 <code>git bisect
</code>.)
43 <li>Patches should be properly
<a href=
"#formatting">formatted
</a>.
44 <li>Patches should be sufficiently
<a href=
"#testing">tested
</a> before submitting.
45 <li>Patches should be
<a href=
"#submit">submitted
</a> via a merge request for
46 <a href=
"#reviewing">review
</a>.
50 <h2 id=
"formatting">Patch formatting
</h2>
53 <li>Lines should be limited to
75 characters or less so that git logs
54 displayed in
80-column terminals avoid line wrapping. Note that git
55 log uses
4 spaces of indentation (
4 +
75 < 80).
56 <li>The first line should be a short, concise summary of the change prefixed
57 with a module name. Examples:
59 mesa: Add support for querying GL_VERTEX_ATTRIB_ARRAY_LONG
61 gallium: add PIPE_CAP_DEVICE_RESET_STATUS_QUERY
63 i965: Fix missing type in local variable declaration.
65 <li>Subsequent patch comments should describe the change in more detail,
66 if needed. For example:
68 i965: Remove end-of-thread SEND alignment code.
70 This was present in Eric's initial implementation of the compaction code
71 for Sandybridge (commit
077d01b6). There is no documentation saying this
72 is necessary, and removing it causes no regressions in piglit on any
75 <li>A
"Signed-off-by:" line is not required, but not discouraged either.
76 <li>If a patch addresses an issue in gitlab, use the Closes: tag
79 Closes: https://gitlab.freedesktop.org/mesa/mesa/issues/
1
81 <p>Prefer the full url to just
<code>Closes: #
1</code>, since the url makes it
82 easier to get to the bug page from
<code>git log
</code></p>
83 <b>Do not use the Fixes: tag for this!
</b> Mesa already uses Fixes for something else.
85 <li>If a patch addresses a issue introduced with earlier commit, that should be
86 noted in the patch comment. For example:
88 Fixes: d7b3707c612
"util/disk_cache: use stat() to check if entry is a directory"
90 <li>You can produce those fixes lines by running
91 <pre>git config --global alias.fixes
"show -s --pretty='format:Fixes: %h (\"%s\
")'"</pre>
92 once and then using
<pre>git fixes
<sha1
></pre>
93 <li>If there have been several revisions to a patch during the review
94 process, they should be noted such as in this example:
96 st/mesa: add ARB_texture_stencil8 support (v4)
98 if we support stencil texturing, enable texture_stencil8
99 there is no requirement to support native S8 for this,
100 the texture can be converted to x24s8 fine.
102 v2: fold fixes from Marek in:
103 a) put S8 last in the list
104 b) fix renderable to always test for d/s renderable
105 fixup the texture case to use a stencil only format
106 for picking the format for the texture view.
107 v3: hit fallback for getteximage
108 v4: put s8 back in front, it shouldn't get picked now (Ilia)
110 <li>If someone tested your patch, document it with a line like this:
112 Tested-by: Joe Hacker
<jhacker@foo.com
>
114 <li>If the patch was reviewed (usually the case) or acked by someone,
115 that should be documented with:
117 Reviewed-by: Joe Hacker
<jhacker@foo.com
>
118 Acked-by: Joe Hacker
<jhacker@foo.com
>
120 <li>If sending later revision of a patch, add all the tags - ack, r-b,
121 Cc: mesa-stable and/or other. This provides reviewers with quick feedback if the
122 patch has already been reviewed.
127 <h2 id=
"testing">Testing Patches
</h2>
130 It should go without saying that patches must be tested. In general,
131 do whatever testing is prudent.
135 You should always run the Mesa test suite before submitting patches.
136 The test suite can be run using the 'meson test' command. All tests
137 must pass before patches will be accepted, this may mean you have
138 to update the tests themselves.
142 Whenever possible and applicable, test the patch with
143 <a href=
"https://piglit.freedesktop.org">Piglit
</a> and/or
144 <a href=
"https://android.googlesource.com/platform/external/deqp/">dEQP
</a>
145 to check for regressions.
149 As mentioned at the beginning, patches should be bisectable.
150 A good way to test this is to make use of the `git rebase` command,
151 to run your tests on each commit. Assuming your branch is based off
152 <code>origin/master
</code>, you can run:
155 $ git rebase --interactive --exec
"meson test -C build/" origin/master
158 replacing
<code>"meson test"</code> with whatever other test you want to
162 <h2 id=
"submit">Submitting Patches
</h2>
165 Patches are submitted to the Mesa project via a
166 <a href=
"https://gitlab.freedesktop.org/mesa/mesa">GitLab
</a> Merge Request.
170 Add labels to your MR to help reviewers find it. For example:
173 <li>Mesa changes affecting all drivers: mesa
174 <li>Hardware vendor specific code: amd, intel, nvidia, ...
175 <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi,
177 <li>Other tag examples: gallium, util
180 Tick the following when creating the MR. It allows developers to
181 rebase your work on top of master.
183 <pre>Allow commits from members who can merge to the target branch
</pre>
185 If you revise your patches based on code review and push an update
186 to your branch, you should maintain a
<strong>clean
</strong> history
187 in your patches. There should not be
"fixup" patches in the history.
188 The series should be buildable and functional after every commit
189 whenever you push the branch.
192 It is your responsibility to keep the MR alive and making progress,
193 as there are no guarantees that a Mesa dev will independently take
200 <li>Make changes and update your branch based on feedback
201 <li>After an update, for the feedback you handled, close the
202 feedback discussion with the
"Resolve Discussion" button. This way
203 the reviewers know which feedback got handled and which didn't.
204 <li>Old, stale MR may be closed, but you can reopen it if you
205 still want to pursue the changes
206 <li>You should periodically check to see if your MR needs to be
208 <li>Make sure your MR is closed if your patches get pushed outside
210 <li>Please send MRs from a personal fork rather than from the main
211 Mesa repository, as it clutters it unnecessarily.
214 <h2 id=
"reviewing">Reviewing Patches
</h2>
217 To participate in code review, you can monitor the GitLab Mesa
218 <a href=
"https://gitlab.freedesktop.org/mesa/mesa/merge_requests">Merge
219 Requests
</a> page, and/or register for notifications in your gitlab
224 When you've reviewed a patch, please be unambiguous about your review.
225 That is, state either
228 Reviewed-by: Joe Hacker
<jhacker@foo.com
>
232 Acked-by: Joe Hacker
<jhacker@foo.com
>
235 Rather than saying just
"LGTM" or
"Seems OK".
239 If small changes are suggested, it's OK to say something like:
242 With the above fixes, Reviewed-by: Joe Hacker
<jhacker@foo.com
>
245 which tells the patch author that the patch can be committed, as long
246 as the issues are resolved first.
250 These Reviewed-by, Acked-by, and Tested-by tags should also be amended
251 into commits in a MR before it is merged.
255 When providing a Reviewed-by, Acked-by, or Tested-by tag in a gitlab MR,
256 enclose the tag in backticks:
259 `Reviewed-by: Joe Hacker
<jhacker@example.com
>`
</pre>
261 This is the markdown format for literal, and will prevent gitlab from hiding
262 the
< and
> symbols.
266 Review by non-experts is encouraged. Understanding how someone else
267 goes about solving a problem is a great way to learn your way around
268 the project. The submitter is expected to evaluate whether they have
269 an appropriate amount of review feedback from people who also
270 understand the code before merging their patches.
273 <h2 id=
"nominations">Nominating a commit for a stable branch
</h2>
276 There are three ways to nominate a patch for inclusion in the stable branch and
280 <li> By adding the Cc: mesa-stable@ tag as described below.
281 <li> By adding the fixes: tag as described below.
282 <li> By submitting a merge request against the
"staging/year.quarter" branch on gitlab.
286 Please
<strong>DO NOT
</strong> send patches to
287 mesa-stable@lists.freedesktop.org, it is not monitored actively and is a
291 If you are not the author of the original patch, please Cc: them in your
296 The current patch status can be observed in the
<a href=
"releasing.html#stagingbranch">staging branch
</a>.
299 <h3 id=
"thetag">The stable tag
</h3>
302 If you want a commit to be applied to a stable branch,
303 you should add an appropriate note to the commit message.
307 Using a
"fixes tag" as described in
<a href=
"#formatting">Patch formatting
</a>
308 is the preferred way to nominate a commit that you know ahead of time should be
309 backported. There are scripts that will figure out which releases to apply the
310 patch to automatically, so you don't need to figure it out.
314 Alternatively, you may use a
"CC:" tag.
316 Here are some examples of such a note:
319 CC:
20.0 19.3 <mesa-stable@lists.freedesktop.org
>
323 Using the CC tag
<strong>should
</strong> include the stable branches you want
324 to nominate the patch to. If you do not provide any version it is nominated to
325 all active stable branches.
328 <h2 id=
"criteria">Criteria for accepting patches to the stable branch
</h2>
330 Mesa has a designated release manager for each stable branch, and the release
331 manager is the only developer that should be pushing changes to these branches.
332 Everyone else should nominate patches using the mechanism described above.
334 The following rules define which patches are accepted and which are not. The
335 stable-release manager is also given broad discretion in rejecting patches
336 that have been nominated.
339 <li>Patch must conform with the
<a href=
"#guidelines">Basic guidelines
</a></li>
341 <li>Patch must have landed in master first. In case where the original
342 patch is too large and/or otherwise contradicts with the rules set within, a
343 backport is appropriate.
</li>
345 <li>It must not introduce a regression - be that build or runtime wise.
347 Note: If the regression is due to faulty piglit/dEQP/CTS/other test the
348 latter must be fixed first. A reference to the offending test(s) and
349 respective fix(es) should be provided in the nominated patch.
</li>
351 <li>Patch cannot be larger than
100 lines.
</li>
353 <li>Patches that move code around with no functional change should be
356 <li>Patch must be a bug fix and not a new feature.
358 Note: An exception to this rule, are hardware-enabling
"features". For
359 example,
<a href=
"#backports">backports
</a> of new code to support a
360 newly-developed hardware product can be accepted if they can be reasonably
361 determined not to have effects on other hardware.
</li>
363 <li>Patch must be reviewed, For example, the commit message has Reviewed-by,
364 Signed-off-by, or Tested-by tags from someone but the author.
</li>
366 <li>Performance patches are considered only if they provide information
367 about the hardware, program in question and observed improvement. Use numbers
368 to represent your measurements.
</li>
371 If the patch complies with the rules it will be
372 <a href=
"releasing.html#pickntest">cherry-picked
</a>. Alternatively the release
373 manager will reply to the patch in question stating why the patch has been
374 rejected or would request a backport.
376 The stable-release manager may at times need to force-push changes to the
377 stable branches, for example, to drop a previously-picked patch that was later
378 identified as causing a regression). These force-pushes may cause changes to
379 be lost from the stable branch if developers push things directly. Consider
382 <h2 id=
"backports">Sending backports for the stable branch
</h2>
384 By default merge conflicts are resolved by the stable-release manager. The
385 release maintainer should resolve trivial conflicts, but for complex conflicts
386 they should ask the original author to provide a backport or de-nominate the
391 For patches that either need to be nominated after they've landed in master, or
392 that are known ahead of time to not not apply cleanly to a stable branch (such
393 as due to a rename), using a gitlab MR is most appropriate.
395 The MR should be based on and target the staging/year.quarter branch, not on
396 the year.quarter branch, per the stable branch policy.
398 Assigning the MR to release maintainer for said branch or mentioning them is
399 helpful, but not required.
402 <h2 id=
"gittips">Git tips
</h2>
405 <li><code>git rebase -i ...
</code> is your friend. Don't be afraid to use it.
406 <li>Apply a fixup to commit FOO.
409 git commit --fixup=FOO
410 git rebase -i --autosquash ...
412 <li>Test for build breakage between patches e.g last
8 commits.
414 git rebase -i
--exec=
"ninja -C build/" HEAD~
8
416 <li>Sets the default mailing address for your repo.
418 git config --local sendemail.to mesa-dev@lists.freedesktop.org
420 <li> Add version to subject line of patch series in this case for the last
8
421 commits before sending.
423 git send-email
--subject-prefix=
"PATCH v4" HEAD~
8
424 git send-email -v4 @~
8 # shorter version, inherited from git format-patch