docs/features: mark GL_ARB_texture_multisample as done for zink
[mesa.git] / docs / submittingpatches.html
index 0581391b2958253b8f4d74d1112650becd6c185f..fb614a790b6a0debbe6cf766af3c0b9513b987f9 100644 (file)
@@ -2,26 +2,26 @@
 <html lang="en">
 <head>
   <meta http-equiv="content-type" content="text/html; charset=utf-8">
-  <title>Submitting patches</title>
+  <title>Submitting Patches</title>
   <link rel="stylesheet" type="text/css" href="mesa.css">
 </head>
 <body>
 
 <div class="header">
-  <h1>The Mesa 3D Graphics Library</h1>
+  The Mesa 3D Graphics Library
 </div>
 
 <iframe src="contents.html"></iframe>
 <div class="content">
 
-<h1>Submitting patches</h1>
+<h1>Submitting Patches</h1>
 
 
 <ul>
 <li><a href="#guidelines">Basic guidelines</a>
 <li><a href="#formatting">Patch formatting</a>
 <li><a href="#testing">Testing Patches</a>
-<li><a href="#mailing">Mailing Patches</a>
+<li><a href="#submit">Submitting Patches</a>
 <li><a href="#reviewing">Reviewing Patches</a>
 <li><a href="#nominations">Nominating a commit for a stable branch</a>
 <li><a href="#criteria">Criteria for accepting patches to the stable branch</a>
 perhaps, in very trivial cases.)
 <li>Code patches should follow Mesa
 <a href="codingstyle.html" target="_parent">coding conventions</a>.
-<li>Whenever possible, patches should only effect individual Mesa/Gallium
+<li>Whenever possible, patches should only affect individual Mesa/Gallium
 components.
 <li>Patches should never introduce build breaks and should be bisectable (see
 <code>git bisect</code>.)
 <li>Patches should be properly <a href="#formatting">formatted</a>.
 <li>Patches should be sufficiently <a href="#testing">tested</a> before submitting.
-<li>Patches should be submitted to <a href="#mailing">mesa-dev</a>
-for <a href="#reviewing">review</a> using <code>git send-email</code>.
+<li>Patches should be <a href="#submit">submitted</a> via a merge request for
+<a href="#reviewing">review</a>.
 
 </ul>
 
@@ -56,76 +56,70 @@ log uses 4 spaces of indentation (4 + 75 &lt; 80).
 <li>The first line should be a short, concise summary of the change prefixed
 with a module name.  Examples:
 <pre>
-    mesa: Add support for querying GL_VERTEX_ATTRIB_ARRAY_LONG
+mesa: Add support for querying GL_VERTEX_ATTRIB_ARRAY_LONG
 
-    gallium: add PIPE_CAP_DEVICE_RESET_STATUS_QUERY
+gallium: add PIPE_CAP_DEVICE_RESET_STATUS_QUERY
 
-    i965: Fix missing type in local variable declaration.
+i965: Fix missing type in local variable declaration.
 </pre>
 <li>Subsequent patch comments should describe the change in more detail,
 if needed.  For example:
 <pre>
-    i965: Remove end-of-thread SEND alignment code.
-    
-    This was present in Eric's initial implementation of the compaction code
-    for Sandybridge (commit 077d01b6). There is no documentation saying this
-    is necessary, and removing it causes no regressions in piglit on any
-    platform.
+i965: Remove end-of-thread SEND alignment code.
+
+This was present in Eric's initial implementation of the compaction code
+for Sandybridge (commit 077d01b6). There is no documentation saying this
+is necessary, and removing it causes no regressions in piglit on any
+platform.
 </pre>
 <li>A "Signed-off-by:" line is not required, but not discouraged either.
-<li>If a patch addresses a bugzilla issue, that should be noted in the
-patch comment.  For example:
+<li>If a patch addresses an issue in gitlab, use the Closes: tag
+For example:
 <pre>
-   Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89689
+Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/1
 </pre>
+<p>Prefer the full url to just <code>Closes: #1</code>, since the url makes it
+easier to get to the bug page from <code>git log</code></p>
+<b>Do not use the Fixes: tag for this!</b> Mesa already uses Fixes for something else.
+
 <li>If a patch addresses a issue introduced with earlier commit, that should be
 noted in the patch comment.  For example:
 <pre>
-   Fixes: d7b3707c612 "util/disk_cache: use stat() to check if entry is a directory"
+Fixes: d7b3707c612 "util/disk_cache: use stat() to check if entry is a directory"
 </pre>
+<li>You can produce those fixes lines by running
+<pre>git config --global alias.fixes "show -s --pretty='format:Fixes: %h (\"%s\")'"</pre>
+once and then using <pre>git fixes &lt;sha1&gt;</pre>
 <li>If there have been several revisions to a patch during the review
 process, they should be noted such as in this example:
 <pre>
-    st/mesa: add ARB_texture_stencil8 support (v4)
-    
-    if we support stencil texturing, enable texture_stencil8
-    there is no requirement to support native S8 for this,
-    the texture can be converted to x24s8 fine.
-    
-    v2: fold fixes from Marek in:
-       a) put S8 last in the list
-       b) fix renderable to always test for d/s renderable
-        fixup the texture case to use a stencil only format
-        for picking the format for the texture view.
-    v3: hit fallback for getteximage
-    v4: put s8 back in front, it shouldn't get picked now (Ilia)
+st/mesa: add ARB_texture_stencil8 support (v4)
+
+if we support stencil texturing, enable texture_stencil8
+there is no requirement to support native S8 for this,
+the texture can be converted to x24s8 fine.
+
+v2: fold fixes from Marek in:
+   a) put S8 last in the list
+   b) fix renderable to always test for d/s renderable
+     fixup the texture case to use a stencil only format
+     for picking the format for the texture view.
+v3: hit fallback for getteximage
+v4: put s8 back in front, it shouldn't get picked now (Ilia)
 </pre>
 <li>If someone tested your patch, document it with a line like this:
 <pre>
-    Tested-by: Joe Hacker &lt;jhacker@foo.com&gt;
+Tested-by: Joe Hacker &lt;jhacker@foo.com&gt;
 </pre>
 <li>If the patch was reviewed (usually the case) or acked by someone,
 that should be documented with:
 <pre>
-    Reviewed-by: Joe Hacker &lt;jhacker@foo.com&gt;
-    Acked-by: Joe Hacker &lt;jhacker@foo.com&gt;
+Reviewed-by: Joe Hacker &lt;jhacker@foo.com&gt;
+Acked-by: Joe Hacker &lt;jhacker@foo.com&gt;
 </pre>
 <li>If sending later revision of a patch, add all the tags - ack, r-b,
 Cc: mesa-stable and/or other. This provides reviewers with quick feedback if the
 patch has already been reviewed.
-<li>In order for your patch to reach the prospective reviewer easier/faster,
-use the script scripts/get_reviewer.pl to get a list of individuals and include
-them in the CC list.
-<br>
-Please use common sense and do <strong>not</strong> blindly add everyone.
-<br>
-<pre>
-    $ scripts/get_reviewer.pl --help # to get the help screen
-    $ scripts/get_reviewer.pl -f src/egl/drivers/dri2/platform_android.c
-    Rob Herring <robh@kernel.org> (reviewer:ANDROID EGL SUPPORT,added_lines:188/700=27%,removed_lines:58/283=20%)
-    Tomasz Figa <tfiga@chromium.org> (reviewer:ANDROID EGL SUPPORT,authored:12/41=29%,added_lines:308/700=44%,removed_lines:115/283=41%)
-    Emil Velikov <emil.l.velikov@gmail.com> (authored:13/41=32%,removed_lines:76/283=27%)
-</pre>
 </ul>
 
 
@@ -139,7 +133,7 @@ do whatever testing is prudent.
 
 <p>
 You should always run the Mesa test suite before submitting patches.
-The test suite can be run using the 'make check' command. All tests
+The test suite can be run using the 'meson test' command. All tests
 must pass before patches will be accepted, this may mean you have
 to update the tests themselves.
 </p>
@@ -152,67 +146,90 @@ to check for regressions.
 </p>
 
 <p>
-As mentioned at the begining, patches should be bisectable.
+As mentioned at the beginning, patches should be bisectable.
 A good way to test this is to make use of the `git rebase` command,
 to run your tests on each commit. Assuming your branch is based off
 <code>origin/master</code>, you can run:
+</p>
 <pre>
-$ git rebase --interactive --exec "make check" origin/master
+$ git rebase --interactive --exec "meson test -C build/" origin/master
 </pre>
-replacing <code>"make check"</code> with whatever other test you want to
+<p>
+replacing <code>"meson test"</code> with whatever other test you want to
 run.
 </p>
 
-
-<h2 id="mailing">Mailing Patches</h2>
+<h2 id="submit">Submitting Patches</h2>
 
 <p>
-Patches should be sent to the mesa-dev mailing list for review:
-<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
-mesa-dev@lists.freedesktop.org</a>.
-When submitting a patch make sure to use
-<a href="https://git-scm.com/docs/git-send-email">git send-email</a>
-rather than attaching patches to emails. Sending patches as
-attachments prevents people from being able to provide in-line review
-comments.
+Patches are submitted to the Mesa project via a
+<a href="https://gitlab.freedesktop.org/mesa/mesa">GitLab</a> Merge Request.
 </p>
 
 <p>
-When submitting follow-up patches you can use --in-reply-to to make v2, v3,
-etc patches show up as replies to the originals. This usually works well
-when you're sending out updates to individual patches (as opposed to
-re-sending the whole series). Using --in-reply-to makes
-it harder for reviewers to accidentally review old patches.
+  Add labels to your MR to help reviewers find it. For example:
 </p>
-
+<ul>
+  <li>Mesa changes affecting all drivers: mesa
+  <li>Hardware vendor specific code: amd, intel, nvidia, ...
+  <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi,
+    radv, vc4, ...
+  <li>Other tag examples: gallium, util
+</ul>
 <p>
-When submitting follow-up patches you should also login to
-<a href="https://patchwork.freedesktop.org">patchwork</a> and change the
-state of your old patches to Superseded.
+  Tick the following when creating the MR. It allows developers to
+  rebase your work on top of master.
 </p>
-
+<pre>Allow commits from members who can merge to the target branch</pre>
 <p>
-Some companies' mail server automatically append a legal disclaimer,
-usually containing something along the lines of "The information in this
-email is confidential" and "distribution is strictly prohibited".<br/>
-These legal notices prevent us from being able to accept your patch,
-rendering the whole process pointless. Please make sure these are
-disabled before sending your patches. (Note that you may need to contact
-your email administrator for this.)
+  If you revise your patches based on code review and push an update
+  to your branch, you should maintain a <strong>clean</strong> history
+  in your patches. There should not be "fixup" patches in the history.
+  The series should be buildable and functional after every commit
+  whenever you push the branch.
 </p>
+<p>
+  It is your responsibility to keep the MR alive and making progress,
+  as there are no guarantees that a Mesa dev will independently take
+  interest in it.
+</p>
+<p>
+  Some other notes:
+</p>
+<ul>
+  <li>Make changes and update your branch based on feedback
+  <li>After an update, for the feedback you handled, close the
+    feedback discussion with the "Resolve Discussion" button. This way
+    the reviewers know which feedback got handled and which didn't.
+  <li>Old, stale MR may be closed, but you can reopen it if you
+    still want to pursue the changes
+  <li>You should periodically check to see if your MR needs to be
+    rebased
+  <li>Make sure your MR is closed if your patches get pushed outside
+    of GitLab
+  <li>Please send MRs from a personal fork rather than from the main
+    Mesa repository, as it clutters it unnecessarily.
+</ul>
 
 <h2 id="reviewing">Reviewing Patches</h2>
 
 <p>
-When you've reviewed a patch on the mailing list, please be unambiguous
-about your review.  That is, state either
+  To participate in code review, you can monitor the GitLab Mesa
+ <a href="https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests">Merge
+  Requests</a> page, and/or register for notifications in your gitlab
+  settings.
+</p>
+
+<p>
+When you've reviewed a patch, please be unambiguous about your review.
+  That is, state either
 </p>
 <pre>
-    Reviewed-by: Joe Hacker &lt;jhacker@foo.com&gt;
+Reviewed-by: Joe Hacker &lt;jhacker@foo.com&gt;
 </pre>
 or
 <pre>
-    Acked-by: Joe Hacker &lt;jhacker@foo.com&gt;
+Acked-by: Joe Hacker &lt;jhacker@foo.com&gt;
 </pre>
 <p>
 Rather than saying just "LGTM" or "Seems OK".
@@ -222,13 +239,36 @@ Rather than saying just "LGTM" or "Seems OK".
 If small changes are suggested, it's OK to say something like:
 </p>
 <pre>
-   With the above fixes, Reviewed-by: Joe Hacker &lt;jhacker@foo.com&gt;
+With the above fixes, Reviewed-by: Joe Hacker &lt;jhacker@foo.com&gt;
 </pre>
 <p>
 which tells the patch author that the patch can be committed, as long
 as the issues are resolved first.
 </p>
 
+<p>
+These Reviewed-by, Acked-by, and Tested-by tags should also be amended
+into commits in a MR before it is merged.
+</p>
+
+<p>
+When providing a Reviewed-by, Acked-by, or Tested-by tag in a gitlab MR,
+enclose the tag in backticks:
+</p>
+<pre>
+`Reviewed-by: Joe Hacker &lt;jhacker@example.com&gt;`</pre>
+<p>
+This is the markdown format for literal, and will prevent gitlab from hiding
+the &lt; and &gt; symbols.
+</p>
+
+<p>
+Review by non-experts is encouraged.  Understanding how someone else
+goes about solving a problem is a great way to learn your way around
+the project.  The submitter is expected to evaluate whether they have
+an appropriate amount of review feedback from people who also
+understand the code before merging their patches.
+</p>
 
 <h2 id="nominations">Nominating a commit for a stable branch</h2>
 
@@ -238,15 +278,23 @@ release.
 </p>
 <ul>
 <li> By adding the Cc: mesa-stable@ tag as described below.
-<li> Sending the commit ID (as seen in master branch) to the mesa-stable@ mailing list.
-<li> Forwarding the patch from the mesa-dev@ mailing list.
+<li> By adding the fixes: tag as described below.
+<li> By submitting a merge request against the "staging/year.quarter" branch on gitlab.
 </li>
 </ul>
 <p>
-Note: resending patch identical to one on mesa-dev@ or one that differs only
-by the extra mesa-stable@ tag is <strong>not</strong> recommended.
+Please <strong>DO NOT</strong> send patches to
+mesa-stable@lists.freedesktop.org, it is not monitored actively and is a
+historical artifact.
+</p>
+<p>
+If you are not the author of the original patch, please Cc: them in your
+nomination request.
 </p>
 
+<p>
+The current patch status can be observed in the <a href="releasing.html#stagingbranch">staging branch</a>.
+</p>
 
 <h3 id="thetag">The stable tag</h3>
 
@@ -256,32 +304,27 @@ you should add an appropriate note to the commit message.
 </p>
 
 <p>
-Here are some examples of such a note:
+Using a "fixes tag" as described in <a href="#formatting">Patch formatting</a>
+is the preferred way to nominate a commit that you know ahead of time should be
+backported. There are scripts that will figure out which releases to apply the
+patch to automatically, so you don't need to figure it out.
 </p>
-<ul>
-  <li>CC: &lt;mesa-stable@lists.freedesktop.org&gt;</li>
-</ul>
 
-Simply adding the CC to the mesa-stable list address is adequate to nominate
-the commit for all the active stable branches. If the commit is not applicable
-for said branch the stable-release manager will reply stating so.
+<p>
+Alternatively, you may use a "CC:" tag.
 
-This "CC" syntax for patch nomination will cause patches to automatically be
-copied to the mesa-stable@ mailing list when you use "git send-email" to send
-patches to the mesa-dev@ mailing list. If you prefer using --suppress-cc that
-won't have any negative effect on the patch nomination.
+Here are some examples of such a note:
+</p>
+<pre>
+CC: 20.0 19.3 &lt;mesa-stable@lists.freedesktop.org&gt;
+</pre>
 
 <p>
-Note: by removing the tag [as the commit is pushed] the patch is
-<strong>explicitly</strong> rejected from inclusion in the stable branch(es).
-<br>
-Thus, drop the line <strong>only</strong> if you want to cancel the nomination.
+Using the CC tag <strong>should</strong> include the stable branches you want
+to nominate the patch to. If you do not provide any version it is nominated to
+all active stable branches.
 </p>
 
-Alternatively, if one uses the "Fixes" tag as described in the "Patch formatting"
-section, it nominates a commit for all active stable branches that include the
-commit that is referred to.
-
 <h2 id="criteria">Criteria for accepting patches to the stable branch</h2>
 
 Mesa has a designated release manager for each stable branch, and the release
@@ -330,9 +373,6 @@ If the patch complies with the rules it will be
 manager will reply to the patch in question stating why the patch has been
 rejected or would request a backport.
 
-A summary of all the picked/rejected patches will be presented in the
-<a href="releasing.html#prerelease">pre-release</a> announcement.
-
 The stable-release manager may at times need to force-push changes to the
 stable branches, for example, to drop a previously-picked patch that was later
 identified as causing a regression). These force-pushes may cause changes to
@@ -340,14 +380,24 @@ be lost from the stable branch if developers push things directly. Consider
 yourself warned.
 
 <h2 id="backports">Sending backports for the stable branch</h2>
-By default merge conflicts are resolved by the stable-release manager. In which
-case he/she should provide a comment about the changes required, alongside the
-<code>Conflicts</code> section. Summary of which will be provided in the
-<a href="releasing.html#prerelease">pre-release</a> announcement.
-<br>
-Developers are interested in sending backports are recommended to use either a
-<code>[BACKPORT #branch]</code> subject prefix or provides similar information
-within the commit summary.
+<p>
+By default merge conflicts are resolved by the stable-release manager. The
+release maintainer should resolve trivial conflicts, but for complex conflicts
+they should ask the original author to provide a backport or de-nominate the
+patch.
+</p>
+
+<p>
+For patches that either need to be nominated after they've landed in master, or
+that are known ahead of time to not not apply cleanly to a stable branch (such
+as due to a rename), using a gitlab MR is most appropriate.
+
+The MR should be based on and target the staging/year.quarter branch, not on
+the year.quarter branch, per the stable branch policy.
+
+Assigning the MR to release maintainer for said branch or mentioning them is
+helpful, but not required.
+</p>
 
 <h2 id="gittips">Git tips</h2>
 
@@ -355,28 +405,23 @@ within the commit summary.
 <li><code>git rebase -i ...</code> is your friend. Don't be afraid to use it.
 <li>Apply a fixup to commit FOO.
 <pre>
-    git add ...
-    git commit --fixup=FOO
-    git rebase -i --autosquash ...
+git add ...
+git commit --fixup=FOO
+git rebase -i --autosquash ...
 </pre>
 <li>Test for build breakage between patches e.g last 8 commits.
 <pre>
-    git rebase -i --exec="make -j4" HEAD~8
+git rebase -i --exec="ninja -C build/" HEAD~8
 </pre>
 <li>Sets the default mailing address for your repo.
 <pre>
-    git config --local sendemail.to mesa-dev@lists.freedesktop.org
+git config --local sendemail.to mesa-dev@lists.freedesktop.org
 </pre>
 <li> Add version to subject line of patch series in this case for the last 8
 commits before sending.
 <pre>
-    git send-email --subject-prefix="PATCH v4" HEAD~8
-    git send-email -v4 @~8 # shorter version, inherited from git format-patch
-</pre>
-<li> Configure git to use the get_reviewer.pl script interactively. Thus you
-can avoid adding the world to the CC list.
-<pre>
-    git config sendemail.cccmd "./scripts/get_reviewer.pl -i"
+git send-email --subject-prefix="PATCH v4" HEAD~8
+git send-email -v4 @~8 # shorter version, inherited from git format-patch
 </pre>
 </ul>