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