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