xref: /aosp_15_r20/external/crosvm/CONTRIBUTING.md (revision bb4ee6a4ae7042d18b07a98463b9c8b875e44b39)
1*bb4ee6a4SAndroid Build Coastguard Worker# How to Contribute to crosvm
2*bb4ee6a4SAndroid Build Coastguard Worker
3*bb4ee6a4SAndroid Build Coastguard Worker## How to report bugs
4*bb4ee6a4SAndroid Build Coastguard Worker
5*bb4ee6a4SAndroid Build Coastguard WorkerWe use Google issue tracker. Please use
6*bb4ee6a4SAndroid Build Coastguard Worker[the public crosvm component](https://issuetracker.google.com/issues?q=status:open%20componentid:1161302).
7*bb4ee6a4SAndroid Build Coastguard Worker
8*bb4ee6a4SAndroid Build Coastguard Worker**For Googlers**: See [go/crosvm#filing-bugs](https://goto.google.com/crosvm#filing-bugs).
9*bb4ee6a4SAndroid Build Coastguard Worker
10*bb4ee6a4SAndroid Build Coastguard Worker## Contributing code
11*bb4ee6a4SAndroid Build Coastguard Worker
12*bb4ee6a4SAndroid Build Coastguard Worker### Gerrit Account
13*bb4ee6a4SAndroid Build Coastguard Worker
14*bb4ee6a4SAndroid Build Coastguard WorkerYou need to set up a user account with [gerrit](https://chromium-review.googlesource.com/). Once
15*bb4ee6a4SAndroid Build Coastguard Workerlogged in, you can obtain
16*bb4ee6a4SAndroid Build Coastguard Worker[HTTP Credentials](https://chromium-review.googlesource.com/settings/#HTTPCredentials) to set up git
17*bb4ee6a4SAndroid Build Coastguard Workerto upload changes.
18*bb4ee6a4SAndroid Build Coastguard Worker
19*bb4ee6a4SAndroid Build Coastguard WorkerOnce set up, run `./tools/cl` to install the gerrit commit message hook. This will insert a unique
20*bb4ee6a4SAndroid Build Coastguard Worker"Change-Id" into all commit messages so gerrit can identify changes. Even warning messages appear,
21*bb4ee6a4SAndroid Build Coastguard Workerthe message hook will be installed.
22*bb4ee6a4SAndroid Build Coastguard Worker
23*bb4ee6a4SAndroid Build Coastguard Worker### Contributor License Agreement
24*bb4ee6a4SAndroid Build Coastguard Worker
25*bb4ee6a4SAndroid Build Coastguard WorkerContributions to this project must be accompanied by a Contributor License Agreement (CLA). You (or
26*bb4ee6a4SAndroid Build Coastguard Workeryour employer) retain the copyright to your contribution; this simply gives us permission to use and
27*bb4ee6a4SAndroid Build Coastguard Workerredistribute your contributions as part of the project. Head over to
28*bb4ee6a4SAndroid Build Coastguard Worker<https://cla.developers.google.com/> to see your current agreements on file or to sign a new one.
29*bb4ee6a4SAndroid Build Coastguard Worker
30*bb4ee6a4SAndroid Build Coastguard WorkerYou generally only need to submit a CLA once, so if you've already submitted one (even if it was for
31*bb4ee6a4SAndroid Build Coastguard Workera different project), you probably don't need to do it again.
32*bb4ee6a4SAndroid Build Coastguard Worker
33*bb4ee6a4SAndroid Build Coastguard Worker### Commit Messages
34*bb4ee6a4SAndroid Build Coastguard Worker
35*bb4ee6a4SAndroid Build Coastguard WorkerAs for commit messages, we follow
36*bb4ee6a4SAndroid Build Coastguard Worker[ChromeOS's guideline](https://www.chromium.org/chromium-os/developer-library/guides/development/contributing/#commit-messages)
37*bb4ee6a4SAndroid Build Coastguard Workerin general.
38*bb4ee6a4SAndroid Build Coastguard Worker
39*bb4ee6a4SAndroid Build Coastguard WorkerHere is an example of a good commit message:
40*bb4ee6a4SAndroid Build Coastguard Worker
41*bb4ee6a4SAndroid Build Coastguard Worker```
42*bb4ee6a4SAndroid Build Coastguard Workerdevices: vhost: user: vmm: Add Connection type
43*bb4ee6a4SAndroid Build Coastguard Worker
44*bb4ee6a4SAndroid Build Coastguard WorkerThis abstracts away the cross-platform differences:
45*bb4ee6a4SAndroid Build Coastguard Workercfg(any(target_os = "android", target_os = "linux")) uses a Unix
46*bb4ee6a4SAndroid Build Coastguard Workerdomain domain stream socket to connect to the vhost-user backend, and
47*bb4ee6a4SAndroid Build Coastguard Workercfg(windows) uses a Tube.
48*bb4ee6a4SAndroid Build Coastguard Worker
49*bb4ee6a4SAndroid Build Coastguard WorkerBUG=b:249361790
50*bb4ee6a4SAndroid Build Coastguard WorkerTEST=tools/presubmit --all
51*bb4ee6a4SAndroid Build Coastguard Worker
52*bb4ee6a4SAndroid Build Coastguard WorkerChange-Id: I47651060c2ce3a7e9f850b7ed9af8bd035f82de6
53*bb4ee6a4SAndroid Build Coastguard Worker```
54*bb4ee6a4SAndroid Build Coastguard Worker
55*bb4ee6a4SAndroid Build Coastguard Worker- The first line is a subject that starts with a tag that represents which components your commit
56*bb4ee6a4SAndroid Build Coastguard Worker  relates to. Tags are usually the name of the crate you modified such as `devices:` or `base:`. If
57*bb4ee6a4SAndroid Build Coastguard Worker  you only modified a specific component in a crate, you can specify the path to the component as a
58*bb4ee6a4SAndroid Build Coastguard Worker  tag like `devices: vhost: user:`. If your commit modified multiple crates, specify the crate where
59*bb4ee6a4SAndroid Build Coastguard Worker  your main change exists. The subject should be no more than 50 characters, including any tags.
60*bb4ee6a4SAndroid Build Coastguard Worker- The body should consist of a motivation followed by an impact/action. The body text should be
61*bb4ee6a4SAndroid Build Coastguard Worker  wrapped to 72 characters.
62*bb4ee6a4SAndroid Build Coastguard Worker- `BUG` lines are used to specify an associated issue number. If the issue is filed at
63*bb4ee6a4SAndroid Build Coastguard Worker  [Google's issue tracker](https://issuetracker.google.com/), write `BUG=b:<bug number>`. If no
64*bb4ee6a4SAndroid Build Coastguard Worker  issue is associated, write `BUG=None`. You can have multiple `BUG` lines.
65*bb4ee6a4SAndroid Build Coastguard Worker- `TEST` lines are used to describe how you tested your commit in a free form. You can have multiple
66*bb4ee6a4SAndroid Build Coastguard Worker  `TEST` lines.
67*bb4ee6a4SAndroid Build Coastguard Worker- `Change-Id` is used to identify your change on Gerrit. It's inserted by the gerrit commit message
68*bb4ee6a4SAndroid Build Coastguard Worker  hook as explained in
69*bb4ee6a4SAndroid Build Coastguard Worker  [the previous section](https://crosvm.dev/book/contributing/index.html#gerrit-account). If a new
70*bb4ee6a4SAndroid Build Coastguard Worker  commit is uploaded with the same `Change-Id` as an existing CL's `Change-Id`, gerrit will
71*bb4ee6a4SAndroid Build Coastguard Worker  recognize the new commit as a new patchset of the existing CL.
72*bb4ee6a4SAndroid Build Coastguard Worker
73*bb4ee6a4SAndroid Build Coastguard Worker### Uploading changes
74*bb4ee6a4SAndroid Build Coastguard Worker
75*bb4ee6a4SAndroid Build Coastguard WorkerTo make changes to crosvm, start your work on a new branch tracking `origin/main`.
76*bb4ee6a4SAndroid Build Coastguard Worker
77*bb4ee6a4SAndroid Build Coastguard Worker```bash
78*bb4ee6a4SAndroid Build Coastguard Workergit checkout -b myfeature --track origin/main
79*bb4ee6a4SAndroid Build Coastguard Worker```
80*bb4ee6a4SAndroid Build Coastguard Worker
81*bb4ee6a4SAndroid Build Coastguard WorkerAfter making the necessary changes, and testing them via
82*bb4ee6a4SAndroid Build Coastguard Worker[Presubmit Checks](https://crosvm.dev/book/building_crosvm/linux.html#presubmit-checks), you can
83*bb4ee6a4SAndroid Build Coastguard Workercommit and upload them:
84*bb4ee6a4SAndroid Build Coastguard Worker
85*bb4ee6a4SAndroid Build Coastguard Worker```bash
86*bb4ee6a4SAndroid Build Coastguard Workergit commit
87*bb4ee6a4SAndroid Build Coastguard Worker./tools/cl upload
88*bb4ee6a4SAndroid Build Coastguard Worker```
89*bb4ee6a4SAndroid Build Coastguard Worker
90*bb4ee6a4SAndroid Build Coastguard WorkerIf you need to revise your change, you can amend the existing commit and upload again:
91*bb4ee6a4SAndroid Build Coastguard Worker
92*bb4ee6a4SAndroid Build Coastguard Worker```bash
93*bb4ee6a4SAndroid Build Coastguard Workergit commit --amend
94*bb4ee6a4SAndroid Build Coastguard Worker./tools/cl upload
95*bb4ee6a4SAndroid Build Coastguard Worker```
96*bb4ee6a4SAndroid Build Coastguard Worker
97*bb4ee6a4SAndroid Build Coastguard WorkerThis will create a new version of the same change in gerrit.
98*bb4ee6a4SAndroid Build Coastguard Worker
99*bb4ee6a4SAndroid Build Coastguard WorkerIf the branch contains multiple commits, each one will be uploaded as a separate review, and they
100*bb4ee6a4SAndroid Build Coastguard Workerwill be linked in Gerrit as [related changes]. You may revise any commit in a branch using tools
101*bb4ee6a4SAndroid Build Coastguard Workerlike `git rebase` and then re-upload the whole series with `./tools/cl upload` when `HEAD` is
102*bb4ee6a4SAndroid Build Coastguard Workerpointing to the tip of the branch.
103*bb4ee6a4SAndroid Build Coastguard Worker
104*bb4ee6a4SAndroid Build Coastguard Worker> Note: We don't accept any pull requests on the [GitHub mirror].
105*bb4ee6a4SAndroid Build Coastguard Worker
106*bb4ee6a4SAndroid Build Coastguard Worker### Getting Reviews
107*bb4ee6a4SAndroid Build Coastguard Worker
108*bb4ee6a4SAndroid Build Coastguard WorkerAll submissions needs to be reviewed by one of the [crosvm owners]. Use the gerrit UI to request a
109*bb4ee6a4SAndroid Build Coastguard Workerreview and add [email protected] to assign to a random owner.
110*bb4ee6a4SAndroid Build Coastguard Worker
111*bb4ee6a4SAndroid Build Coastguard WorkerIf you run into issues with reviews, reach out to the team via
112*bb4ee6a4SAndroid Build Coastguard Worker[chat](https://matrix.to/#/#crosvm:matrix.org) or
113*bb4ee6a4SAndroid Build Coastguard Worker[email list](https://groups.google.com/a/chromium.org/g/crosvm-dev).
114*bb4ee6a4SAndroid Build Coastguard Worker
115*bb4ee6a4SAndroid Build Coastguard Worker**For Googlers**: see [go/crosvm-chat](https://goto.google.com/crosvm-chat).
116*bb4ee6a4SAndroid Build Coastguard Worker
117*bb4ee6a4SAndroid Build Coastguard Worker#### Any change to Cargo.lock
118*bb4ee6a4SAndroid Build Coastguard Worker
119*bb4ee6a4SAndroid Build Coastguard WorkerWhen adding a new crate from crates.io, additional review is required to ensure that the crate meets
120*bb4ee6a4SAndroid Build Coastguard Workerthe crosvm project standards. This review is provided by the members of `OWNERS_COUNCIL`.
121*bb4ee6a4SAndroid Build Coastguard Worker
122*bb4ee6a4SAndroid Build Coastguard WorkerUnfortunately, our tooling cannot tell the difference between adding an external crate and changing
123*bb4ee6a4SAndroid Build Coastguard Workerdependencies within crosvm (e.g. `devices` depending on a new internal crosvm utility crate). For
124*bb4ee6a4SAndroid Build Coastguard Workerthose cases, a rubberstamp is still needed from `OWNERS_COUNCIL`.
125*bb4ee6a4SAndroid Build Coastguard Worker
126*bb4ee6a4SAndroid Build Coastguard Worker**For Googlers**: see [go/crosvm/3p_crates](https://goto.google.com/crosvm/3p_crates).
127*bb4ee6a4SAndroid Build Coastguard Worker
128*bb4ee6a4SAndroid Build Coastguard Worker### Reviewing code (for OWNERS)
129*bb4ee6a4SAndroid Build Coastguard Worker
130*bb4ee6a4SAndroid Build Coastguard WorkerWe have two major types of reviewers on the project:
131*bb4ee6a4SAndroid Build Coastguard Worker
132*bb4ee6a4SAndroid Build Coastguard Worker1. Global OWNERS: these folks are broadly responsible for the health of the crosvm project, and have
133*bb4ee6a4SAndroid Build Coastguard Worker   expertise in multiple project subdomains. While they can technically approve any change, they
134*bb4ee6a4SAndroid Build Coastguard Worker   will often delegate to area OWNERS when a change is outside their expertise.
135*bb4ee6a4SAndroid Build Coastguard Worker1. Area OWNERS: experts in a particular subdomain of the project (e.g. graphics, USB, etc). Major
136*bb4ee6a4SAndroid Build Coastguard Worker   changes in an area SHOULD be reviewed by an area OWNER, if one exists (not all subdomains have
137*bb4ee6a4SAndroid Build Coastguard Worker   OWNERS).
138*bb4ee6a4SAndroid Build Coastguard Worker
139*bb4ee6a4SAndroid Build Coastguard WorkerAll owners are expected to review code in their areas, and to aim for the following goals in
140*bb4ee6a4SAndroid Build Coastguard Workerreviews:
141*bb4ee6a4SAndroid Build Coastguard Worker
142*bb4ee6a4SAndroid Build Coastguard Worker- Reply to reviews within 1 working day. If this is infeasible (especially if overloaded), reassign
143*bb4ee6a4SAndroid Build Coastguard Worker  to crosvm-reviews@ to pick another OWNER at random.
144*bb4ee6a4SAndroid Build Coastguard Worker- Defer to the [styleguide](./coding_style.md) where it makes sense to do so. Update the styleguide
145*bb4ee6a4SAndroid Build Coastguard Worker  when it does not.
146*bb4ee6a4SAndroid Build Coastguard Worker- Strive to avoid reviews getting stuck in endless back & forth. If you see this happening, you can:
147*bb4ee6a4SAndroid Build Coastguard Worker  - Schedule a meeting to discuss it online. Consider inviting another OWNER to help brainstorm
148*bb4ee6a4SAndroid Build Coastguard Worker    solutions.
149*bb4ee6a4SAndroid Build Coastguard Worker  - Bring the review discussion to the hallway chat to let the group weigh in.
150*bb4ee6a4SAndroid Build Coastguard Worker- Follow generally accepted practices for good code review
151*bb4ee6a4SAndroid Build Coastguard Worker  - Technically: We insist on good documentation, clean APIs especially when broadly consumed, and
152*bb4ee6a4SAndroid Build Coastguard Worker    generally keep code health in mind.
153*bb4ee6a4SAndroid Build Coastguard Worker  - Socially: Our goal, above all else, is to be good peers to each other. So we review *code*, not
154*bb4ee6a4SAndroid Build Coastguard Worker    *authors*. We remember to disagree respectfully, and that a code review is a team effort (author
155*bb4ee6a4SAndroid Build Coastguard Worker    and reviewer) against a hard technical problem.
156*bb4ee6a4SAndroid Build Coastguard Worker
157*bb4ee6a4SAndroid Build Coastguard Worker### Submitting code
158*bb4ee6a4SAndroid Build Coastguard Worker
159*bb4ee6a4SAndroid Build Coastguard WorkerCrosvm uses a Commit Queue, which will run pre-submit testing on all changes before merging them
160*bb4ee6a4SAndroid Build Coastguard Workerinto crosvm.
161*bb4ee6a4SAndroid Build Coastguard Worker
162*bb4ee6a4SAndroid Build Coastguard WorkerOnce one of the [crosvm owners] has voted "Code-Review+2" on your change, you can use the "Submit to
163*bb4ee6a4SAndroid Build Coastguard WorkerCQ" button, which will trigger the test process.
164*bb4ee6a4SAndroid Build Coastguard Worker
165*bb4ee6a4SAndroid Build Coastguard WorkerGerrit will show any test failures. Refer to
166*bb4ee6a4SAndroid Build Coastguard Worker[Building Crosvm](https://crosvm.dev/book/building_crosvm/) for information on how to run the same
167*bb4ee6a4SAndroid Build Coastguard Workertests locally.
168*bb4ee6a4SAndroid Build Coastguard Worker
169*bb4ee6a4SAndroid Build Coastguard WorkerEach individual change in a patch series must build and pass the tests. If you are working on a
170*bb4ee6a4SAndroid Build Coastguard Workerseries of related changes, ensure that each incremental commit does not cause test regressions or
171*bb4ee6a4SAndroid Build Coastguard Workerbreak the build if it is merged without the later changes in the series. For example, an
172*bb4ee6a4SAndroid Build Coastguard Workerintermediate change must not trigger any unused code warnings or cause test failures that are fixed
173*bb4ee6a4SAndroid Build Coastguard Workerby later changes in the series.
174*bb4ee6a4SAndroid Build Coastguard Worker
175*bb4ee6a4SAndroid Build Coastguard WorkerWhen all tests pass, your change is merged into `origin/main`.
176*bb4ee6a4SAndroid Build Coastguard Worker
177*bb4ee6a4SAndroid Build Coastguard Worker## Contributing to the documentation
178*bb4ee6a4SAndroid Build Coastguard Worker
179*bb4ee6a4SAndroid Build Coastguard Worker[The book of crosvm] is built with [mdBook]. Each markdown file must follow
180*bb4ee6a4SAndroid Build Coastguard Worker[Google Markdown style guide].
181*bb4ee6a4SAndroid Build Coastguard Worker
182*bb4ee6a4SAndroid Build Coastguard WorkerTo render the book locally, you need to install mdbook and [mdbook-mermaid], which should be
183*bb4ee6a4SAndroid Build Coastguard Workerinstalled when you run `./tools/install-deps` script. Or you can use the `tools/dev_container`
184*bb4ee6a4SAndroid Build Coastguard Workerenvironment.
185*bb4ee6a4SAndroid Build Coastguard Worker
186*bb4ee6a4SAndroid Build Coastguard Worker```sh
187*bb4ee6a4SAndroid Build Coastguard Workercd docs/book/
188*bb4ee6a4SAndroid Build Coastguard Workermdbook build
189*bb4ee6a4SAndroid Build Coastguard Worker```
190*bb4ee6a4SAndroid Build Coastguard Worker
191*bb4ee6a4SAndroid Build Coastguard WorkerOutput is found at `docs/book/book/html/`.
192*bb4ee6a4SAndroid Build Coastguard Worker
193*bb4ee6a4SAndroid Build Coastguard WorkerTo format markdown files, run `./tools/fmt` in the `dev_container`.
194*bb4ee6a4SAndroid Build Coastguard Worker
195*bb4ee6a4SAndroid Build Coastguard Worker[crosvm owners]: https://chromium.googlesource.com/crosvm/crosvm/+/HEAD/OWNERS
196*bb4ee6a4SAndroid Build Coastguard Worker[github mirror]: https://github.com/google/crosvm
197*bb4ee6a4SAndroid Build Coastguard Worker[google markdown style guide]: https://github.com/google/styleguide/blob/gh-pages/docguide/style.md
198*bb4ee6a4SAndroid Build Coastguard Worker[mdbook]: https://rust-lang.github.io/mdBook/
199*bb4ee6a4SAndroid Build Coastguard Worker[mdbook-mermaid]: https://github.com/badboy/mdbook-mermaid
200*bb4ee6a4SAndroid Build Coastguard Worker[related changes]: https://gerrit-review.googlesource.com/Documentation/user-review-ui.html#related-changes
201*bb4ee6a4SAndroid Build Coastguard Worker[the book of crosvm]: https://crosvm.dev/book/
202