Submitting patches
+Submitting Patches
-
@@ -42,10 +42,8 @@ components.
- Patches should be properly formatted.
- Patches should be sufficiently tested before submitting. -
- Patches should be submitted -to mesa-dev or with -a merge request -for review. +
- Patches should be submitted via a merge request for +review.
git bisect
.)
- 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.
- 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.
- Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89689 +Closes: https://gitlab.freedesktop.org/mesa/mesa/issues/1+
Prefer the full url to just Closes: #1
, since the url makes it
+easier to get to the bug page from git log
- 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"+
git config --global alias.fixes "show -s --pretty='format:Fixes: %h (\"%s\")'"+once and then using
git fixes <sha1>
- 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)
- Tested-by: Joe Hacker <jhacker@foo.com> +Tested-by: Joe Hacker <jhacker@foo.com>
- Reviewed-by: Joe Hacker <jhacker@foo.com> - Acked-by: Joe Hacker <jhacker@foo.com> +Reviewed-by: Joe Hacker <jhacker@foo.com> +Acked-by: Joe Hacker <jhacker@foo.com>
-Please use common sense and do not blindly add everyone. -
-
- $ 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%) -@@ -141,7 +133,7 @@ do whatever testing is prudent.
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.
@@ -154,93 +146,41 @@ to check for regressions.
-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
origin/master
, you can run:
-$ git rebase --interactive --exec "make check" origin/master +$ git rebase --interactive --exec "meson test -C build/" origin/master
-replacing "make check"
with whatever other test you want to
+replacing "meson test"
with whatever other test you want to
run.
Submitting Patches
-Patches may be submitted to the Mesa project by -email or with a -GitLab merge request. To prevent -duplicate code review, only use one method to submit your changes. -
- -Mailing Patches
- --Patches may be sent to the mesa-dev mailing list for review: - -mesa-dev@lists.freedesktop.org. -When submitting a patch make sure to use -git send-email -rather than attaching patches to emails. Sending patches as -attachments prevents people from being able to provide in-line review -comments. -
- --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. +Patches are submitted to the Mesa project via a +GitLab Merge Request.
--When submitting follow-up patches you should also login to -patchwork and change the -state of your old patches to Superseded. -
- -
-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".
-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.)
-
GitLab Merge Requests
- -- GitLab Merge - Requests (MR) can also be used to submit patches for Mesa. -
- -- If the MR may have interest for most of the Mesa community, you can - send an email to the mesa-dev email list including a link to the MR. - Don't send the patch to mesa-dev, just the MR link. -
Add labels to your MR to help reviewers find it. For example: -
-
-
- Mesa changes affecting all drivers: mesa -
- Hardware vendor specific code: amd, intel, nvidia, ... -
- Driver specific code: anvil, freedreno, i965, iris, radeonsi, - radv, vc4, ... -
- Other tag examples: gallium, util -
-
+
- Mesa changes affecting all drivers: mesa +
- Hardware vendor specific code: amd, intel, nvidia, ... +
- Driver specific code: anvil, freedreno, i965, iris, radeonsi, + radv, vc4, ... +
- Other tag examples: gallium, util +
Tick the following when creating the MR. It allows developers to rebase your work on top of master. -
Allow commits from members who can merge to the target branch+
Allow commits from members who can merge to the target branch
If you revise your patches based on code review and push an update to your branch, you should maintain a clean history @@ -255,39 +195,41 @@ your email administrator for this.)
Some other notes: -
-
-
- Make changes and update your branch based on feedback -
- Old, stale MR may be closed, but you can reopen it if you - still want to pursue the changes -
- You should periodically check to see if your MR needs to be - rebased -
- Make sure your MR is closed if your patches get pushed outside - of GitLab -
- Please send MRs from a personal fork rather than from the main - Mesa repository, as it clutters it unnecessarily. -
-
+
- Make changes and update your branch based on feedback +
- 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. +
- Old, stale MR may be closed, but you can reopen it if you + still want to pursue the changes +
- You should periodically check to see if your MR needs to be + rebased +
- Make sure your MR is closed if your patches get pushed outside + of GitLab +
- Please send MRs from a personal fork rather than from the main + Mesa repository, as it clutters it unnecessarily. +
Reviewing Patches
- To participate in code review, you should monitor the - - mesa-dev email list and the GitLab - Mesa Merge - Requests page. + To participate in code review, you can monitor the GitLab Mesa + Merge + Requests page, and/or register for notifications in your gitlab + settings.
-When you've reviewed a patch on the mailing list, please be unambiguous -about your review. That is, state either +When you've reviewed a patch, please be unambiguous about your review. + That is, state either
- Reviewed-by: Joe Hacker <jhacker@foo.com> +Reviewed-by: Joe Hacker <jhacker@foo.com>or
- Acked-by: Joe Hacker <jhacker@foo.com> +Acked-by: Joe Hacker <jhacker@foo.com>
Rather than saying just "LGTM" or "Seems OK". @@ -297,7 +239,7 @@ Rather than saying just "LGTM" or "Seems OK". If small changes are suggested, it's OK to say something like:
- With the above fixes, Reviewed-by: Joe Hacker <jhacker@foo.com> +With the above fixes, Reviewed-by: Joe Hacker <jhacker@foo.com>
which tells the patch author that the patch can be committed, as long @@ -314,7 +256,7 @@ When providing a Reviewed-by, Acked-by, or Tested-by tag in a gitlab MR, enclose the tag in backticks:
- `Reviewed-by: Joe Hacker <jhacker@example.com>`+`Reviewed-by: Joe Hacker <jhacker@example.com>`
This is the markdown format for literal, and will prevent gitlab from hiding the < and > symbols. @@ -336,13 +278,14 @@ release.
- By adding the Cc: mesa-stable@ tag as described below. -
- Sending the commit ID (as seen in master branch) to the mesa-stable@ mailing list. -
- Forwarding the patch from the mesa-dev@ mailing list. +
- By adding the fixes: tag as described below. +
- By submitting a merge request against the "staging/year.quarter" branch on gitlab.
-Note: resending patch identical to one on mesa-dev@ or one that differs only -by the extra mesa-stable@ tag is not recommended. +Please DO NOT send patches to +mesa-stable@lists.freedesktop.org, it is not monitored actively and is a +historical artifact.
If you are not the author of the original patch, please Cc: them in your @@ -361,32 +304,27 @@ you should add an appropriate note to the commit message.
-Here are some examples of such a note: +Using a "fixes tag" as described in Patch formatting +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.
--
-
- CC: <mesa-stable@lists.freedesktop.org> -
+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: +
++CC: 20.0 19.3 <mesa-stable@lists.freedesktop.org> +
-Note: by removing the tag [as the commit is pushed] the patch is
-explicitly rejected from inclusion in the stable branch(es).
-
-Thus, drop the line only if you want to cancel the nomination.
+Using the CC tag should 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.
Criteria for accepting patches to the stable branch
Mesa has a designated release manager for each stable branch, and the release @@ -435,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 -pre-release 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 @@ -445,14 +380,24 @@ be lost from the stable branch if developers push things directly. Consider yourself warned.Sending backports for the stable branch
-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 -Conflicts
section. Summary of which will be provided in the
-pre-release announcement.
--Developers are interested in sending backports are recommended to use either a -
[BACKPORT #branch]
subject prefix or provides similar information
-within the commit summary.
++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. +
+ ++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. +
Git tips
@@ -460,28 +405,23 @@ within the commit summary.git rebase -i ...
is your friend. Don't be afraid to use it.
- git add ... - git commit --fixup=FOO - git rebase -i --autosquash ... +git add ... +git commit --fixup=FOO +git rebase -i --autosquash ...
- git rebase -i --exec="make -j4" HEAD~8 +git rebase -i --exec="ninja -C build/" HEAD~8
- git config --local sendemail.to mesa-dev@lists.freedesktop.org +git config --local sendemail.to mesa-dev@lists.freedesktop.org
- git send-email --subject-prefix="PATCH v4" HEAD~8 - git send-email -v4 @~8 # shorter version, inherited from git format-patch --
- 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