docs/submittingpatches.html

   1 <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
   2 <html lang="en">
   3 <head>
   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">
   7 </head>
   8 <body>
   9
  10 <div class="header">
  11   <h1>The Mesa 3D Graphics Library</h1>
  12 </div>
  13
  14 <iframe src="contents.html"></iframe>
  15 <div class="content">
  16
  17 <h1>Submitting patches</h1>
  18
  19
  20 <ul>
  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="#mailing">Mailing 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>
  30 </ul>
  31
  32 <h2 id="guidelines">Basic guidelines</h2>
  33
  34 <ul>
  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 effect individual Mesa/Gallium
  40 components.
  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 submitted to <a href="#mailing">mesa-dev</a>
  46 for <a href="#reviewing">review</a> using <code>git send-email</code>.
  47
  48 </ul>
  49
  50 <h2 id="formatting">Patch formatting</h2>
  51
  52 <ul>
  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 &lt; 80).
  56 <li>The first line should be a short, concise summary of the change prefixed
  57 with a module name.  Examples:
  58 <pre>
  59     mesa: Add support for querying GL_VERTEX_ATTRIB_ARRAY_LONG
  60
  61     gallium: add PIPE_CAP_DEVICE_RESET_STATUS_QUERY
  62
  63     i965: Fix missing type in local variable declaration.
  64 </pre>
  65 <li>Subsequent patch comments should describe the change in more detail,
  66 if needed.  For example:
  67 <pre>
  68     i965: Remove end-of-thread SEND alignment code.
  69
  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
  73     platform.
  74 </pre>
  75 <li>A "Signed-off-by:" line is not required, but not discouraged either.
  76 <li>If a patch addresses a bugzilla issue, that should be noted in the
  77 patch comment.  For example:
  78 <pre>
  79    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89689
  80 </pre>
  81 <li>If there have been several revisions to a patch during the review
  82 process, they should be noted such as in this example:
  83 <pre>
  84     st/mesa: add ARB_texture_stencil8 support (v4)
  85
  86     if we support stencil texturing, enable texture_stencil8
  87     there is no requirement to support native S8 for this,
  88     the texture can be converted to x24s8 fine.
  89
  90     v2: fold fixes from Marek in:
  91        a) put S8 last in the list
  92        b) fix renderable to always test for d/s renderable
  93         fixup the texture case to use a stencil only format
  94         for picking the format for the texture view.
  95     v3: hit fallback for getteximage
  96     v4: put s8 back in front, it shouldn't get picked now (Ilia)
  97 </pre>
  98 <li>If someone tested your patch, document it with a line like this:
  99 <pre>
 100     Tested-by: Joe Hacker &lt;jhacker@foo.com&gt;
 101 </pre>
 102 <li>If the patch was reviewed (usually the case) or acked by someone,
 103 that should be documented with:
 104 <pre>
 105     Reviewed-by: Joe Hacker &lt;jhacker@foo.com&gt;
 106     Acked-by: Joe Hacker &lt;jhacker@foo.com&gt;
 107 </pre>
 108 <li>If sending later revision of a patch, add all the tags - ack, r-b,
 109 Cc: mesa-stable and/or other. This provides reviewers with quick feedback if the
 110 patch has already been reviewed.
 111 <li>In order for your patch to reach the prospective reviewer easier/faster,
 112 use the script scripts/get_reviewer.pl to get a list of individuals and include
 113 them in the CC list.
 114 <br>
 115 Please use common sense and do <strong>not</strong> blindly add everyone.
 116 <br>
 117 <pre>
 118     $ scripts/get_reviewer.pl --help # to get the the help screen
 119     $ scripts/get_reviewer.pl -f src/egl/drivers/dri2/platform_android.c
 120     Rob Herring <robh@kernel.org> (reviewer:ANDROID EGL SUPPORT,added_lines:188/700=27%,removed_lines:58/283=20%)
 121     Tomasz Figa <tfiga@chromium.org> (reviewer:ANDROID EGL SUPPORT,authored:12/41=29%,added_lines:308/700=44%,removed_lines:115/283=41%)
 122     Emil Velikov <emil.l.velikov@gmail.com> (authored:13/41=32%,removed_lines:76/283=27%)
 123 </pre>
 124 </ul>
 125
 126
 127
 128 <h2 id="testing">Testing Patches</h2>
 129
 130 <p>
 131 It should go without saying that patches must be tested.  In general,
 132 do whatever testing is prudent.
 133 </p>
 134
 135 <p>
 136 You should always run the Mesa test suite before submitting patches.
 137 The test suite can be run using the 'make check' command. All tests
 138 must pass before patches will be accepted, this may mean you have
 139 to update the tests themselves.
 140 </p>
 141
 142 <p>
 143 Whenever possible and applicable, test the patch with
 144 <a href="https://piglit.freedesktop.org">Piglit</a> and/or
 145 <a href="https://android.googlesource.com/platform/external/deqp/">dEQP</a>
 146 to check for regressions.
 147 </p>
 148
 149
 150 <h2 id="mailing">Mailing Patches</h2>
 151
 152 <p>
 153 Patches should be sent to the mesa-dev mailing list for review:
 154 <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
 155 mesa-dev@lists.freedesktop.org</a>.
 156 When submitting a patch make sure to use
 157 <a href="https://git-scm.com/docs/git-send-email">git send-email</a>
 158 rather than attaching patches to emails. Sending patches as
 159 attachments prevents people from being able to provide in-line review
 160 comments.
 161 </p>
 162
 163 <p>
 164 When submitting follow-up patches you can use --in-reply-to to make v2, v3,
 165 etc patches show up as replies to the originals. This usually works well
 166 when you're sending out updates to individual patches (as opposed to
 167 re-sending the whole series). Using --in-reply-to makes
 168 it harder for reviewers to accidentally review old patches.
 169 </p>
 170
 171 <p>
 172 When submitting follow-up patches you should also login to
 173 <a href="https://patchwork.freedesktop.org">patchwork</a> and change the
 174 state of your old patches to Superseded.
 175 </p>
 176
 177 <h2 id="reviewing">Reviewing Patches</h2>
 178
 179 <p>
 180 When you've reviewed a patch on the mailing list, please be unambiguous
 181 about your review.  That is, state either
 182 </p>
 183 <pre>
 184     Reviewed-by: Joe Hacker &lt;jhacker@foo.com&gt;
 185 </pre>
 186 or
 187 <pre>
 188     Acked-by: Joe Hacker &lt;jhacker@foo.com&gt;
 189 </pre>
 190 <p>
 191 Rather than saying just "LGTM" or "Seems OK".
 192 </p>
 193
 194 <p>
 195 If small changes are suggested, it's OK to say something like:
 196 </p>
 197 <pre>
 198    With the above fixes, Reviewed-by: Joe Hacker &lt;jhacker@foo.com&gt;
 199 </pre>
 200 <p>
 201 which tells the patch author that the patch can be committed, as long
 202 as the issues are resolved first.
 203 </p>
 204
 205
 206 <h2 id="nominations">Nominating a commit for a stable branch</h2>
 207
 208 <p>
 209 There are three ways to nominate a patch for inclusion in the stable branch and
 210 release.
 211 </p>
 212 <ul>
 213 <li> By adding the Cc: mesa-stable@ tag as described below.
 214 <li> Sending the commit ID (as seen in master branch) to the mesa-stable@ mailing list.
 215 <li> Forwarding the patch from the mesa-dev@ mailing list.
 216 </li>
 217 </ul>
 218 <p>
 219 Note: resending patch identical to one on mesa-dev@ or one that differs only
 220 by the extra mesa-stable@ tag is <strong>not</strong> recommended.
 221 </p>
 222
 223
 224 <h3 id="thetag">The stable tag</h3>
 225
 226 <p>
 227 If you want a commit to be applied to a stable branch,
 228 you should add an appropriate note to the commit message.
 229 </p>
 230
 231 <p>
 232 Here are some examples of such a note:
 233 </p>
 234 <ul>
 235   <li>CC: &lt;mesa-stable@lists.freedesktop.org&gt;</li>
 236   <li>CC: "9.2 10.0" &lt;mesa-stable@lists.freedesktop.org&gt;</li>
 237   <li>CC: "10.0" &lt;mesa-stable@lists.freedesktop.org&gt;</li>
 238 </ul>
 239
 240 Simply adding the CC to the mesa-stable list address is adequate to nominate
 241 the commit for the most-recently-created stable branch. It is only necessary
 242 to specify a specific branch name, (such as "9.2 10.0" or "10.0" in the
 243 examples above), if you want to nominate the commit for an older stable
 244 branch. And, as in these examples, you can nominate the commit for the older
 245 branch in addition to the more recent branch, or nominate the commit
 246 exclusively for the older branch.
 247
 248 This "CC" syntax for patch nomination will cause patches to automatically be
 249 copied to the mesa-stable@ mailing list when you use "git send-email" to send
 250 patches to the mesa-dev@ mailing list. If you prefer using --suppress-cc that
 251 won't have any negative effect on the patch nomination.
 252
 253 <p>
 254 Note: by removing the tag [as the commit is pushed] the patch is
 255 <strong>explicitly</strong> rejected from inclusion in the stable branch(es).
 256 <br>
 257 Thus, drop the line <strong>only</strong> if you want to cancel the nomination.
 258 </p>
 259
 260 <h2 id="criteria">Criteria for accepting patches to the stable branch</h2>
 261
 262 Mesa has a designated release manager for each stable branch, and the release
 263 manager is the only developer that should be pushing changes to these branches.
 264 Everyone else should nominate patches using the mechanism described above.
 265
 266 The following rules define which patches are accepted and which are not. The
 267 stable-release manager is also given broad discretion in rejecting patches
 268 that have been nominated.
 269
 270 <ul>
 271   <li>Patch must conform with the <a href="#guidelines">Basic guidelines</a></li>
 272
 273   <li>Patch must have landed in master first. In case where the original
 274   patch is too large and/or otherwise contradicts with the rules set within, a
 275   backport is appropriate.</li>
 276
 277   <li>It must not introduce a regression - be that build or runtime wise.
 278
 279   Note:  If the regression is due to faulty piglit/dEQP/CTS/other test the
 280   latter must be fixed first. A reference to the offending test(s) and
 281   respective fix(es) should be provided in the nominated patch.</li>
 282
 283   <li>Patch cannot be larger than 100 lines.</li>
 284
 285   <li>Patches that move code around with no functional change should be
 286   rejected.</li>
 287
 288   <li>Patch must be a bug fix and not a new feature.
 289
 290   Note: An exception to this rule, are hardware-enabling "features". For
 291   example, <a href="#backports">backports</a> of new code to support a
 292   newly-developed hardware product can be accepted if they can be reasonably
 293   determined not to have effects on other hardware.</li>
 294
 295   <li>Patch must be reviewed, For example, the commit message has Reviewed-by,
 296   Signed-off-by, or Tested-by tags from someone but the author.</li>
 297
 298   <li>Performance patches are considered only if they provide information
 299   about the hardware, program in question and observed improvement. Use numbers
 300   to represent your measurements.</li>
 301 </ul>
 302
 303 If the patch complies with the rules it will be
 304 <a href="releasing.html#pickntest">cherry-picked</a>. Alternatively the release
 305 manager will reply to the patch in question stating why the patch has been
 306 rejected or would request a backport.
 307
 308 A summary of all the picked/rejected patches will be presented in the
 309 <a href="releasing.html#prerelease">pre-release</a> announcement.
 310
 311 The stable-release manager may at times need to force-push changes to the
 312 stable branches, for example, to drop a previously-picked patch that was later
 313 identified as causing a regression). These force-pushes may cause changes to
 314 be lost from the stable branch if developers push things directly. Consider
 315 yourself warned.
 316
 317 <h2 id="backports">Sending backports for the stable branch</h2>
 318 By default merge conflicts are resolved by the stable-release manager. In which
 319 case he/she should provide a comment about the changes required, alongside the
 320 <code>Conflicts</code> section. Summary of which will be provided in the
 321 <a href="releasing.html#prerelease">pre-release</a> announcement.
 322 <br>
 323 Developers are interested in sending backports are recommended to use either a
 324 <code>[BACKPORT #branch]</code> subject prefix or provides similar information
 325 within the commit summary.
 326
 327 <h2 id="gittips">Git tips</h2>
 328
 329 <ul>
 330 <li><code>git rebase -i ...</code> is your friend. Don't be afraid to use it.
 331 <li>Apply a fixup to commit FOO.
 332 <pre>
 333     git add ...
 334     git commit --fixup=FOO
 335     git rebase -i --autosquash ...
 336 </pre>
 337 <li>Test for build breakage between patches e.g last 8 commits.
 338 <pre>
 339     git rebase -i --exec="make -j4" HEAD~8
 340 </pre>
 341 <li>Sets the default mailing address for your repo.
 342 <pre>
 343     git config --local sendemail.to mesa-dev@lists.freedesktop.org
 344 </pre>
 345 <li> Add version to subject line of patch series in this case for the last 8
 346 commits before sending.
 347 <pre>
 348     git send-email --subject-prefix="PATCH v4" HEAD~8
 349     git send-email -v4 @~8 # shorter version, inherited from git format-patch
 350 </pre>
 351 <li> Configure git to use the get_reviewer.pl script interactively. Thus you
 352 can avoid adding the world to the CC list.
 353 <pre>
 354     git config sendemail.cccmd "./scripts/get_reviewer.pl -i"
 355 </pre>
 356 </ul>
 357
 358
 359 </div>
 360 </body>
 361 </html>