xref: /aosp_15_r20/external/skia/site/docs/dev/contrib/submit.md (revision c8dee2aa9b3f27cf6c858bd81872bdeb2c07ed17)
1*c8dee2aaSAndroid Build Coastguard Worker---
2*c8dee2aaSAndroid Build Coastguard Workertitle: 'How to submit a patch'
3*c8dee2aaSAndroid Build Coastguard WorkerlinkTitle: 'How to submit a patch'
4*c8dee2aaSAndroid Build Coastguard Worker---
5*c8dee2aaSAndroid Build Coastguard Worker
6*c8dee2aaSAndroid Build Coastguard Worker## Configure git
7*c8dee2aaSAndroid Build Coastguard Worker
8*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?-->
9*c8dee2aaSAndroid Build Coastguard Worker
10*c8dee2aaSAndroid Build Coastguard Worker    git config --global user.name "Your Name"
11*c8dee2aaSAndroid Build Coastguard Worker    git config --global user.email [email protected]
12*c8dee2aaSAndroid Build Coastguard Worker
13*c8dee2aaSAndroid Build Coastguard Worker## Making changes
14*c8dee2aaSAndroid Build Coastguard Worker
15*c8dee2aaSAndroid Build Coastguard WorkerFirst create a branch for your changes:
16*c8dee2aaSAndroid Build Coastguard Worker
17*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?-->
18*c8dee2aaSAndroid Build Coastguard Worker
19*c8dee2aaSAndroid Build Coastguard Worker    git config branch.autosetuprebase always
20*c8dee2aaSAndroid Build Coastguard Worker    git checkout -b my_feature origin/main
21*c8dee2aaSAndroid Build Coastguard Worker
22*c8dee2aaSAndroid Build Coastguard WorkerAfter making your changes, create a commit
23*c8dee2aaSAndroid Build Coastguard Worker
24*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?-->
25*c8dee2aaSAndroid Build Coastguard Worker
26*c8dee2aaSAndroid Build Coastguard Worker    git add [file1] [file2] ...
27*c8dee2aaSAndroid Build Coastguard Worker    git commit
28*c8dee2aaSAndroid Build Coastguard Worker
29*c8dee2aaSAndroid Build Coastguard WorkerIf your branch gets out of date, you will need to update it:
30*c8dee2aaSAndroid Build Coastguard Worker
31*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?-->
32*c8dee2aaSAndroid Build Coastguard Worker
33*c8dee2aaSAndroid Build Coastguard Worker    git pull
34*c8dee2aaSAndroid Build Coastguard Worker    python3 tools/git-sync-deps
35*c8dee2aaSAndroid Build Coastguard Worker
36*c8dee2aaSAndroid Build Coastguard Worker## Adding a unit test
37*c8dee2aaSAndroid Build Coastguard Worker
38*c8dee2aaSAndroid Build Coastguard WorkerIf you are willing to change Skia codebase, it's nice to add a test at the same
39*c8dee2aaSAndroid Build Coastguard Workertime. Skia has a simple unittest framework so you can add a case to it.
40*c8dee2aaSAndroid Build Coastguard Worker
41*c8dee2aaSAndroid Build Coastguard WorkerTest code is located under the 'tests' directory.
42*c8dee2aaSAndroid Build Coastguard Worker
43*c8dee2aaSAndroid Build Coastguard WorkerSee [Writing Unit and Rendering Tests](/docs/dev/testing/tests) for details.
44*c8dee2aaSAndroid Build Coastguard Worker
45*c8dee2aaSAndroid Build Coastguard WorkerUnit tests are best, but if your change touches rendering and you can't think of
46*c8dee2aaSAndroid Build Coastguard Workeran automated way to verify the results, consider writing a GM test. Also, if
47*c8dee2aaSAndroid Build Coastguard Workeryour change is in the GPU code, you may not be able to write it as part of the
48*c8dee2aaSAndroid Build Coastguard Workerstandard unit test suite, but there are GPU-specific testing paths you can
49*c8dee2aaSAndroid Build Coastguard Workerextend.
50*c8dee2aaSAndroid Build Coastguard Worker
51*c8dee2aaSAndroid Build Coastguard Worker## Updating BUILD.bazel files
52*c8dee2aaSAndroid Build Coastguard Worker
53*c8dee2aaSAndroid Build Coastguard WorkerIf you added or removed files, you will need to update the `BUILD.bazel` file in the directory
54*c8dee2aaSAndroid Build Coastguard Workerof those files. Many `BUILD.bazel` files have a list of files that is broken up into two
55*c8dee2aaSAndroid Build Coastguard Worker[`filegroup`](https://bazel.build/reference/be/general#filegroup) rules using the
56*c8dee2aaSAndroid Build Coastguard Worker`split_srcs_and_hdrs` macro. You should add the new file names or delete the old ones from these
57*c8dee2aaSAndroid Build Coastguard Workerfile lists.
58*c8dee2aaSAndroid Build Coastguard Worker
59*c8dee2aaSAndroid Build Coastguard WorkerIf your feature will be conditionally enabled (e.g. like the GPU backends or image codecs), you
60*c8dee2aaSAndroid Build Coastguard Workermay need to add or modify
61*c8dee2aaSAndroid Build Coastguard Worker[`select`](https://bazel.build/reference/be/common-definitions#configurable-attributes) statements
62*c8dee2aaSAndroid Build Coastguard Workerto achieve that goal. Look at existing rules for examples of this.
63*c8dee2aaSAndroid Build Coastguard Worker
64*c8dee2aaSAndroid Build Coastguard Worker## Submitting a patch
65*c8dee2aaSAndroid Build Coastguard Worker
66*c8dee2aaSAndroid Build Coastguard WorkerFor your code to be accepted into the codebase, you must complete the
67*c8dee2aaSAndroid Build Coastguard Worker[Individual Contributor License Agreement](http://code.google.com/legal/individual-cla-v1.0.html).
68*c8dee2aaSAndroid Build Coastguard WorkerYou can do this online, and it only takes a minute. If you are contributing on
69*c8dee2aaSAndroid Build Coastguard Workerbehalf of a corporation, you must fill out the
70*c8dee2aaSAndroid Build Coastguard Worker[Corporate Contributor License Agreement](http://code.google.com/legal/corporate-cla-v1.0.html)
71*c8dee2aaSAndroid Build Coastguard Workerand send it to us as described on that page. Add your (or your organization's)
72*c8dee2aaSAndroid Build Coastguard Workername and contact info to the AUTHORS file as a part of your CL.
73*c8dee2aaSAndroid Build Coastguard Worker
74*c8dee2aaSAndroid Build Coastguard WorkerNow that you've made a change and written a test for it, it's ready for the code
75*c8dee2aaSAndroid Build Coastguard Workerreview! Submit a patch and getting it reviewed is fairly easy with depot tools.
76*c8dee2aaSAndroid Build Coastguard Worker
77*c8dee2aaSAndroid Build Coastguard WorkerUse `git-cl`, which comes with
78*c8dee2aaSAndroid Build Coastguard Worker[depot tools](http://sites.google.com/a/chromium.org/dev/developers/how-tos/install-depot-tools).
79*c8dee2aaSAndroid Build Coastguard WorkerFor help, run `git cl help`. Note that in order for `git cl` to work correctly,
80*c8dee2aaSAndroid Build Coastguard Workerit needs to run on a clone of <https://skia.googlesource.com/skia>. Using clones
81*c8dee2aaSAndroid Build Coastguard Workerof mirrors, including Google's mirror on GitHub, might lead to issues with
82*c8dee2aaSAndroid Build Coastguard Worker`git cl` usage.
83*c8dee2aaSAndroid Build Coastguard Worker
84*c8dee2aaSAndroid Build Coastguard Worker### Find a reviewer
85*c8dee2aaSAndroid Build Coastguard Worker
86*c8dee2aaSAndroid Build Coastguard WorkerIdeally, the reviewer is someone who is familiar with the area of code you are
87*c8dee2aaSAndroid Build Coastguard Workertouching. Look at the git blame for the file to see who else has been editing
88*c8dee2aaSAndroid Build Coastguard Workerit. If unsuccessful, another option is to click on the 'Suggested Reviewers'
89*c8dee2aaSAndroid Build Coastguard Workerbutton to add one of the listed Skia contacts. They should be able to add
90*c8dee2aaSAndroid Build Coastguard Workerappropriate reviewers for your change. The button is located here:
91*c8dee2aaSAndroid Build Coastguard Worker<img src="/docs/dev/contrib/SuggestedReviewers.png" style="display: inline-block; max-width: 75%" />
92*c8dee2aaSAndroid Build Coastguard Worker
93*c8dee2aaSAndroid Build Coastguard Worker### Uploading changes for review
94*c8dee2aaSAndroid Build Coastguard Worker
95*c8dee2aaSAndroid Build Coastguard WorkerSkia uses the Gerrit code review tool. Skia's instance is
96*c8dee2aaSAndroid Build Coastguard Worker[skia-review](http://skia-review.googlesource.com). Use `git cl` to upload your
97*c8dee2aaSAndroid Build Coastguard Workerchange:
98*c8dee2aaSAndroid Build Coastguard Worker
99*c8dee2aaSAndroid Build Coastguard Worker<!--?prettify lang=sh?-->
100*c8dee2aaSAndroid Build Coastguard Worker
101*c8dee2aaSAndroid Build Coastguard Worker    git cl upload
102*c8dee2aaSAndroid Build Coastguard Worker
103*c8dee2aaSAndroid Build Coastguard WorkerYou may have to enter a Google Account username and password to authenticate
104*c8dee2aaSAndroid Build Coastguard Workeryourself to Gerrit. A free gmail account will do fine, or any other type of
105*c8dee2aaSAndroid Build Coastguard WorkerGoogle account. It does not have to match the email address you configured using
106*c8dee2aaSAndroid Build Coastguard Worker`git config --global user.email` above, but it can.
107*c8dee2aaSAndroid Build Coastguard Worker
108*c8dee2aaSAndroid Build Coastguard WorkerThe command output should include a URL, similar to
109*c8dee2aaSAndroid Build Coastguard Worker([https://skia-review.googlesource.com/c/4559/](https://skia-review.googlesource.com/c/4559/)),
110*c8dee2aaSAndroid Build Coastguard Workerindicating where your changelist can be reviewed.
111*c8dee2aaSAndroid Build Coastguard Worker
112*c8dee2aaSAndroid Build Coastguard Worker### Submit try jobs
113*c8dee2aaSAndroid Build Coastguard Worker
114*c8dee2aaSAndroid Build Coastguard WorkerSkia's trybots allow testing and verification of changes before they land in the
115*c8dee2aaSAndroid Build Coastguard Workerrepo. You need to have permission to trigger try jobs; if you need permission,
116*c8dee2aaSAndroid Build Coastguard Workerask a committer. After uploading your CL to
117*c8dee2aaSAndroid Build Coastguard Worker[Gerrit](https://skia-review.googlesource.com/), you may trigger a try job for
118*c8dee2aaSAndroid Build Coastguard Workerany job listed in tasks.json, either via the Gerrit UI, using `git cl try`, eg.
119*c8dee2aaSAndroid Build Coastguard Worker
120*c8dee2aaSAndroid Build Coastguard Worker    git cl try -B skia.primary -b Some-Tryjob-Name
121*c8dee2aaSAndroid Build Coastguard Worker
122*c8dee2aaSAndroid Build Coastguard Workeror using bin/try, a small wrapper for `git cl try` which helps to choose try
123*c8dee2aaSAndroid Build Coastguard Workerjobs. From a Skia checkout:
124*c8dee2aaSAndroid Build Coastguard Worker
125*c8dee2aaSAndroid Build Coastguard Worker    bin/try --list
126*c8dee2aaSAndroid Build Coastguard Worker
127*c8dee2aaSAndroid Build Coastguard WorkerYou can also search using regular expressions:
128*c8dee2aaSAndroid Build Coastguard Worker
129*c8dee2aaSAndroid Build Coastguard Worker    bin/try "Test.*GTX660.*Release"
130*c8dee2aaSAndroid Build Coastguard Worker
131*c8dee2aaSAndroid Build Coastguard WorkerFor more information about testing, see
132*c8dee2aaSAndroid Build Coastguard Worker[testing infrastructure](/docs/dev/testing/automated_testing).
133*c8dee2aaSAndroid Build Coastguard Worker
134*c8dee2aaSAndroid Build Coastguard Worker### Request review
135*c8dee2aaSAndroid Build Coastguard Worker
136*c8dee2aaSAndroid Build Coastguard WorkerGo to the supplied URL or go to the code review page and select the **Your**
137*c8dee2aaSAndroid Build Coastguard Workerdropdown and click on **Changes**. Select the change you want to submit for
138*c8dee2aaSAndroid Build Coastguard Workerreview and click **Reply**. Enter at least one reviewer's email address. Now add
139*c8dee2aaSAndroid Build Coastguard Workerany optional notes, and send your change off for review by clicking on **Send**.
140*c8dee2aaSAndroid Build Coastguard WorkerUnless you send your change to reviewers, no one will know to look at it.
141*c8dee2aaSAndroid Build Coastguard Worker
142*c8dee2aaSAndroid Build Coastguard Worker_Note_: If you don't see editing commands on the review page, click **Sign in**
143*c8dee2aaSAndroid Build Coastguard Workerin the upper right. _Hint_: You can add -r [email protected] --send-mail to
144*c8dee2aaSAndroid Build Coastguard Workersend the email directly when uploading a change using `git-cl`.
145*c8dee2aaSAndroid Build Coastguard Worker
146*c8dee2aaSAndroid Build Coastguard Worker## The review process
147*c8dee2aaSAndroid Build Coastguard Worker
148*c8dee2aaSAndroid Build Coastguard WorkerIf you submit a giant patch, or do a bunch of work without discussing it with
149*c8dee2aaSAndroid Build Coastguard Workerthe relevant people, you may have a hard time convincing anyone to review it!
150*c8dee2aaSAndroid Build Coastguard Worker
151*c8dee2aaSAndroid Build Coastguard WorkerCode reviews are an important part of the engineering process. The reviewer will
152*c8dee2aaSAndroid Build Coastguard Workeralmost always have suggestions or style fixes for you, and it's important not to
153*c8dee2aaSAndroid Build Coastguard Workertake such suggestions personally or as a commentary on your abilities or ideas.
154*c8dee2aaSAndroid Build Coastguard WorkerThis is a process where we work together to make sure that the highest quality
155*c8dee2aaSAndroid Build Coastguard Workercode gets submitted!
156*c8dee2aaSAndroid Build Coastguard Worker
157*c8dee2aaSAndroid Build Coastguard WorkerYou will likely get email back from the reviewer with comments. Fix these and
158*c8dee2aaSAndroid Build Coastguard Workerupdate the patch set in the issue by uploading again. The upload will explain
159*c8dee2aaSAndroid Build Coastguard Workerthat it is updating the current CL and ask you for a message explaining the
160*c8dee2aaSAndroid Build Coastguard Workerchange. Be sure to respond to all comments before you request review of an
161*c8dee2aaSAndroid Build Coastguard Workerupdate.
162*c8dee2aaSAndroid Build Coastguard Worker
163*c8dee2aaSAndroid Build Coastguard WorkerIf you need to update code the code on an already uploaded CL, simply edit the
164*c8dee2aaSAndroid Build Coastguard Workercode, commit it again locally, and then run git cl upload again e.g.
165*c8dee2aaSAndroid Build Coastguard Worker
166*c8dee2aaSAndroid Build Coastguard Worker    echo "GOATS" > whitespace.txt
167*c8dee2aaSAndroid Build Coastguard Worker    git add whitespace.txt
168*c8dee2aaSAndroid Build Coastguard Worker    git commit -m 'add GOATS fix to whitespace.txt'
169*c8dee2aaSAndroid Build Coastguard Worker    git cl upload
170*c8dee2aaSAndroid Build Coastguard Worker
171*c8dee2aaSAndroid Build Coastguard WorkerOnce you're ready for another review, use **Reply** again to send another
172*c8dee2aaSAndroid Build Coastguard Workernotification (it is helpful to tell the reviewer what you did with respect to
173*c8dee2aaSAndroid Build Coastguard Workereach of their comments). When the reviewer is happy with your patch, they will
174*c8dee2aaSAndroid Build Coastguard Workerapprove your change by setting the Code-Review label to "+1".
175*c8dee2aaSAndroid Build Coastguard Worker
176*c8dee2aaSAndroid Build Coastguard Worker_Note_: As you work through the review process, both you and your reviewers
177*c8dee2aaSAndroid Build Coastguard Workershould converse using the code review interface, and send notes.
178*c8dee2aaSAndroid Build Coastguard Worker
179*c8dee2aaSAndroid Build Coastguard WorkerOnce your change has received an approval, you can click the "Submit to CQ"
180*c8dee2aaSAndroid Build Coastguard Workerbutton on the codereview page and it will be committed on your behalf.
181*c8dee2aaSAndroid Build Coastguard Worker
182*c8dee2aaSAndroid Build Coastguard WorkerOnce your commit has gone in, you should delete the branch containing your
183*c8dee2aaSAndroid Build Coastguard Workerchange:
184*c8dee2aaSAndroid Build Coastguard Worker
185*c8dee2aaSAndroid Build Coastguard Worker    git checkout -q origin/main
186*c8dee2aaSAndroid Build Coastguard Worker    git branch -D my_feature
187*c8dee2aaSAndroid Build Coastguard Worker
188*c8dee2aaSAndroid Build Coastguard Worker## Final Testing
189*c8dee2aaSAndroid Build Coastguard Worker
190*c8dee2aaSAndroid Build Coastguard WorkerSkia's principal downstream user is Chromium, and any change to Skia rendering
191*c8dee2aaSAndroid Build Coastguard Workeroutput can break Chromium. If your change alters rendering in any way, you are
192*c8dee2aaSAndroid Build Coastguard Workerexpected to test for and alleviate this. You may be able to find a Skia team
193*c8dee2aaSAndroid Build Coastguard Workermember to help you, but the onus remains on each individual contributor to avoid
194*c8dee2aaSAndroid Build Coastguard Workerbreaking Chrome.
195*c8dee2aaSAndroid Build Coastguard Worker
196*c8dee2aaSAndroid Build Coastguard Worker### Evaluating Impact on Chromium
197*c8dee2aaSAndroid Build Coastguard Worker
198*c8dee2aaSAndroid Build Coastguard WorkerKeep in mind that Skia is rolled daily into Blink and Chromium. Run local tests
199*c8dee2aaSAndroid Build Coastguard Workerand watch canary bots for results to ensure no impact. If you are submitting
200*c8dee2aaSAndroid Build Coastguard Workerchanges that will impact layout tests, follow the guides below and/or work with
201*c8dee2aaSAndroid Build Coastguard Workeryour friendly Skia-Blink engineer to evaluate, rebaseline, and land your
202*c8dee2aaSAndroid Build Coastguard Workerchanges.
203*c8dee2aaSAndroid Build Coastguard Worker
204*c8dee2aaSAndroid Build Coastguard WorkerResources:
205*c8dee2aaSAndroid Build Coastguard Worker
206*c8dee2aaSAndroid Build Coastguard Worker[How to land Skia changes that change Blink layout test results](/docs/dev/chrome/blink/)
207*c8dee2aaSAndroid Build Coastguard Worker
208*c8dee2aaSAndroid Build Coastguard WorkerIf you're changing the Skia API, you may need to make an associated change in
209*c8dee2aaSAndroid Build Coastguard WorkerChromium. If you do, please follow these instructions:
210*c8dee2aaSAndroid Build Coastguard Worker[Landing Skia changes which require Chrome changes](/docs/dev/chrome/changes/)
211*c8dee2aaSAndroid Build Coastguard Worker
212*c8dee2aaSAndroid Build Coastguard Worker## Check in your changes
213*c8dee2aaSAndroid Build Coastguard Worker
214*c8dee2aaSAndroid Build Coastguard Worker### Non-Skia-committers
215*c8dee2aaSAndroid Build Coastguard Worker
216*c8dee2aaSAndroid Build Coastguard WorkerIf you already have committer rights, you can follow the directions below to
217*c8dee2aaSAndroid Build Coastguard Workercommit your change directly to Skia's repository.
218*c8dee2aaSAndroid Build Coastguard Worker
219*c8dee2aaSAndroid Build Coastguard WorkerIf you don't have committer rights in https://skia.googlesource.com/skia.git ...
220*c8dee2aaSAndroid Build Coastguard Workerfirst of all, thanks for submitting your patch! We really appreciate these
221*c8dee2aaSAndroid Build Coastguard Workersubmissions. After receiving an approval from a committer, you will be able to
222*c8dee2aaSAndroid Build Coastguard Workerclick the "Submit to CQ" button and submit your patch via the commit queue.
223*c8dee2aaSAndroid Build Coastguard Worker
224*c8dee2aaSAndroid Build Coastguard WorkerIn special instances, a Skia committer may assist you in landing the change by
225*c8dee2aaSAndroid Build Coastguard Workeruploading a new codereview containing your patch (perhaps with some small
226*c8dee2aaSAndroid Build Coastguard Workeradjustments at their discretion). If so, you can mark your change as
227*c8dee2aaSAndroid Build Coastguard Worker"Abandoned", and update it with a link to the new codereview.
228*c8dee2aaSAndroid Build Coastguard Worker
229*c8dee2aaSAndroid Build Coastguard Worker### Skia committers
230*c8dee2aaSAndroid Build Coastguard Worker
231*c8dee2aaSAndroid Build Coastguard Worker- tips on how to apply an externally provided patch are [here](../patch)
232*c8dee2aaSAndroid Build Coastguard Worker- when landing externally contributed patches, please note the original
233*c8dee2aaSAndroid Build Coastguard Worker  contributor's identity (and provide a link to the original codereview) in the
234*c8dee2aaSAndroid Build Coastguard Worker  commit message
235*c8dee2aaSAndroid Build Coastguard Worker
236*c8dee2aaSAndroid Build Coastguard Worker  `git-cl` will squash all your commits into a single one with the description
237*c8dee2aaSAndroid Build Coastguard Worker  you used when you uploaded your change.
238*c8dee2aaSAndroid Build Coastguard Worker
239*c8dee2aaSAndroid Build Coastguard Worker  ```
240*c8dee2aaSAndroid Build Coastguard Worker  git cl land
241*c8dee2aaSAndroid Build Coastguard Worker  ```
242*c8dee2aaSAndroid Build Coastguard Worker
243*c8dee2aaSAndroid Build Coastguard Worker  or
244*c8dee2aaSAndroid Build Coastguard Worker
245*c8dee2aaSAndroid Build Coastguard Worker  ```
246*c8dee2aaSAndroid Build Coastguard Worker  git cl land -c 'Contributor Name <[email protected]>'
247*c8dee2aaSAndroid Build Coastguard Worker  ```
248