Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame^] | 1 | # Chromium OS Development Basics |
| 2 | |
| 3 | This document covers guidelines to keep in mind when working on the Chromium OS |
| 4 | project. |
| 5 | |
| 6 | [TOC] |
| 7 | |
| 8 | ## Documentation |
| 9 | |
| 10 | ### General guidelines |
| 11 | |
| 12 | Chromium OS follows the Chromium project's [documentation guidelines] and |
| 13 | [documentation best practices]. |
| 14 | |
| 15 | ### Design documents |
| 16 | |
| 17 | Design documents typically describe the problem that you're trying to solve, |
| 18 | different approaches that you considered, and the path that you're planning to |
| 19 | take. They're useful for soliciting opinions from others before you start on the |
| 20 | implementation, but they also help your code reviewers get their bearings and |
| 21 | can answer other developers' "why was this done like that?" questions after the |
| 22 | work is complete (even after you've forgotten the reasons why!). On top of all |
| 23 | that, a design doc often helps you solidify a design in your own mind and |
| 24 | convince yourself that you're not missing anything. |
| 25 | |
| 26 | Most non-trivial additions of new functionality, and particularly ones that |
| 27 | require adding new daemons or new interactions between different processes, |
| 28 | should have their own design docs. For smaller changes, documenting what you're |
| 29 | doing and why in the issue tracker is often enough. |
| 30 | |
| 31 | Share your design docs with your team mailing list, if not with the full |
| 32 | [chromium-os-dev] mailing list. See the existing [Chromium OS design docs] and |
| 33 | [Chromium design docs] for inspiration. |
| 34 | |
| 35 | ## Programming languages and style |
| 36 | |
| 37 | ### C++ |
| 38 | |
| 39 | Nearly all userspace code in the Chromium OS project, whether it's part of the |
| 40 | Chrome browser itself or a system daemon exposing new functionality, is written |
| 41 | in C++. Do not use another language unless there's a compelling reason why C++ |
| 42 | can't work for what you're doing. (Being more familiar with another language |
| 43 | than with C++ is not a compelling reason.) |
| 44 | |
| 45 | The Chromium project, and by extension the Chromium OS project, follow the |
| 46 | [Google C++ style guide], with the [Chromium C++ style guide] layered on top to |
| 47 | provide additional guidelines and clarify ambiguities. The [C++11 use in |
| 48 | Chromium] document lists which of the many new features introduced in the C++11 |
| 49 | standard are allowed. |
| 50 | |
| 51 | New C++ programs should go into new directories in the [platform2 repository], |
| 52 | which is checked out to `src/platform2`. |
| 53 | |
| 54 | ### C |
| 55 | |
| 56 | C should only be used for code that is part of the Linux kernel or Chrome OS |
| 57 | firmware. |
| 58 | |
| 59 | Both kernel and firmware code should follow the [Linux kernel coding style]. The |
| 60 | [Kernel FAQ] has more details about Chrome OS kernel development, and the |
| 61 | [EC Development page] discusses firmware. |
| 62 | |
| 63 | ### Shell |
| 64 | |
| 65 | Sometimes shell scripting can be the best way to perform lightweight, |
| 66 | non-persistent tasks that needs to run periodically on Chrome OS systems, but |
| 67 | there are also big downsides: it's much more difficult to write tests for shell |
| 68 | scripts than for C++ code, and scripts have a tendency to grow to the point |
| 69 | where they become difficult to maintain. If you're planning to add a shell |
| 70 | script that is likely to grow in complexity, consider instead using C++ from the |
| 71 | start to save yourself the trouble of rewriting it down the road. |
| 72 | |
| 73 | Shell scripts are mainly used for a few categories of tasks in Chrome OS: |
| 74 | |
| 75 | * Upstart initialization scripts, as in [src/platform2/init]. See the |
| 76 | [init STYLE file] and [init README file] for guidelines. |
| 77 | * Portage .ebuild files, as in the [chromiumos-overlay repository]. We follow |
| 78 | the upstream guidelines; see the [Ebuild Writing] page and specifically the |
| 79 | [Ebuild File Format]. |
| 80 | * Miscellaneous development-related tasks |
| 81 | |
| 82 | Read the [Google shell style guide] and [Chromium OS shell style guidelines] |
| 83 | before writing scripts, with the caveat that the Upstart or Portage guidelines |
| 84 | take precedent when writing those types of scripts. |
| 85 | |
| 86 | ### Python |
| 87 | |
| 88 | The Python interpreter is not included in production Chrome OS system images, |
| 89 | but Python is used heavily for development and testing. |
| 90 | |
| 91 | We largely follow the [Google Python style guide], but see the |
| 92 | [Chromium OS Python style guidelines] for important differences, particularly |
| 93 | around spacing and naming. For tests, see the [autotest coding style]. |
| 94 | |
| 95 | ## Testing |
| 96 | |
| 97 | ### Unit tests |
| 98 | |
| 99 | Unit tests should be added alongside new code. It's important to design your |
| 100 | code with testability in mind, as adding tests after-the-fact often requires |
| 101 | heavy refactoring. |
| 102 | |
| 103 | Good unit tests are fast, lightweight, reliable, and easy to run within the |
| 104 | chroot as part of your development workflow. We use [Google Test] (which is |
| 105 | comprised of the GoogleTest unit testing framework and the GoogleMock mock |
| 106 | framework) to test C++ code. The [Why Google C++ Testing Framework?] and [Google |
| 107 | Test FAQ] are good introductions, and the [unit testing document] has more |
| 108 | details about how unit tests get run. |
| 109 | |
| 110 | Note that although GoogleMock is permitted, there are some pitfalls: |
| 111 | |
| 112 | * When creating partial mocks of classes, it's easy to accidentally call |
| 113 | production code within unit tests. |
| 114 | * GoogleMock often produces spammy, hard-to-read output when a test calls |
| 115 | unimportant methods that haven't been mocked. |
| 116 | * On the other hand, it's difficult to maintain a long list of expectations |
| 117 | for methods that you don't actually care about. |
| 118 | |
| 119 | GoogleMock can help verify complex interactions like multiple objects' methods |
| 120 | being called in a specific order, but for simpler cases it's often cleaner to |
| 121 | define interfaces for the tested class's dependencies and write your own |
| 122 | [stub or fake] implementations. |
| 123 | |
| 124 | ### Autotest |
| 125 | |
| 126 | [Autotest] is used to run tests on live Chrome OS systems. Autotests are useful |
| 127 | for performing integration testing (e.g. verifying that two processes are able |
| 128 | to communicate with each other over IPC), but they have heavy costs: |
| 129 | |
| 130 | * Autotests are harder to run than unit tests: you need either a dedicated |
| 131 | test device or a virtual machine. |
| 132 | * Autotests are much slower than unit tests: even a no-op test can take 30 |
| 133 | seconds or longer per run. |
| 134 | * Since autotests involve at least a controlling system and a test device, |
| 135 | they're susceptible to networking issues and hardware flakiness. |
| 136 | * Since autotests run on full, live systems, failures can be caused by issues |
| 137 | in components unrelated to the one that you're trying to test. |
| 138 | |
| 139 | Whenever you can get equivalent coverage from either unit tests or autotests, |
| 140 | use unit tests. Design your system with unit testing in mind. |
| 141 | |
| 142 | ## Code reviews |
| 143 | |
| 144 | The Chromium OS project follows the [Chromium code review policy]: all code and |
| 145 | data changes must be reviewed by another project member before being committed. |
| 146 | Note that Chromium OS's review process has some differences; see the |
| 147 | [Developer Guide's code review instructions]. |
| 148 | |
| 149 | OWNERS files are not (yet) enforced for non-browser parts of the Chromium OS |
| 150 | codebase, but please act as if they were. If there's an OWNERS file in the |
| 151 | directory that you're modifying or a parent directory, add at least one |
| 152 | developer that's listed in it to your code review and wait for their approval |
| 153 | before committing your change. |
| 154 | |
| 155 | Owners may want to consider setting up notifications for changes to their code. |
| 156 | To receive notifications of changes to `src/platform2/debugd`, open your |
| 157 | [Gerrit notification settings] and add a new entry for project |
| 158 | `chromiumos/platform2` and expression `file:"^debugd/.*"`. |
| 159 | |
| 160 | ## Issue trackers |
| 161 | |
| 162 | Public Chromium OS bugs and feature requests are tracked using the |
| 163 | [chromium issue tracker]. Note that this tracker is shared with the Chromium |
| 164 | project; most OS-specific issues are classified under an `OS>`-prefixed |
| 165 | component and have an `OS=Chrome` label. The `crbug.com` redirector makes it |
| 166 | easy to jump directly to an issue with a given ID; `https://crbug.com/123` will |
| 167 | redirect to issue #123, for instance. |
| 168 | |
| 169 | Keep discussion in the issue tracker instead of in email or over IM. Issues |
| 170 | remain permanently available and are viewable by people not present for the |
| 171 | original discussion; email and IM exchanges are harder to find after the fact |
| 172 | and may even be deleted automatically. `BUG=chromium:123` lines in commit |
| 173 | descriptions and `https://crbug.com/123` mentions in code comments also make it |
| 174 | easy to link back to the issue that describes the original motivation for a |
| 175 | change. |
| 176 | |
| 177 | Avoid discussing multiple problems in a single issue. It's not possible to split |
| 178 | an existing issue into multiple new issues, and it can take a long time to read |
| 179 | through an issue's full history to figure out what is currently being discussed. |
| 180 | |
| 181 | Similarly, do not reopen an old, closed issue in response to the reoccurrence of |
| 182 | a bug: the old issue probably contains now-irrelevant milestone and merge labels |
| 183 | and outdated information. Instead, create a new issue and refer to the prior |
| 184 | issue in its initial description. Text of the form `issue 123` will |
| 185 | automatically be turned into a link. |
| 186 | |
| 187 | ## Mailing lists |
| 188 | |
| 189 | We use the [Chromium technical discussion groups] and |
| 190 | [Chromium OS technical discussion groups] for development-related |
| 191 | discussion. Most Chromium OS discussion takes place on [chromium-os-dev] (for |
| 192 | OS-related topics) and [chromium-dev] (for browser-related topics). You should |
| 193 | subscribe to chromium-os-dev if you're doing Chromium OS development, as |
| 194 | important PSAs are frequently sent there. |
| 195 | |
| 196 | ## Developing in the open |
| 197 | |
| 198 | Chromium OS is an open-source project. Whenever possible (i.e. when not |
| 199 | discussing private, partner-related information), use the public issue tracker |
| 200 | and mailing lists rather than the internal versions. |
| 201 | |
| 202 | [documentation guidelines]: https://chromium.googlesource.com/chromium/src/+/master/docs/documentation_guidelines.md |
| 203 | [documentation best practices]: https://chromium.googlesource.com/chromium/src/+/master/docs/documentation_best_practices.md |
| 204 | [Chromium OS design docs]: https://www.chromium.org/chromium-os/chromiumos-design-docs |
| 205 | [Chromium design docs]: https://www.chromium.org/developers/design-documents |
| 206 | [Google C++ style guide]: https://google.github.io/styleguide/cppguide.html |
| 207 | [Chromium C++ style guide]: https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md |
| 208 | [C++11 use in Chromium]: https://chromium-cpp.appspot.com/ |
| 209 | [platform2 repository]: https://dev.chromium.org/chromium-os/getting-started-with-platform2 |
| 210 | [Linux kernel coding style]: https://github.com/torvalds/linux/blob/master/Documentation/process/coding-style.rst |
| 211 | [Kernel FAQ]: https://www.chromium.org/chromium-os/how-tos-and-troubleshooting/kernel-faq |
| 212 | [EC Development page]: https://www.chromium.org/chromium-os/ec-development |
| 213 | [src/platform2/init]: https://chromium.googlesource.com/chromiumos/platform2/+/master/init/ |
| 214 | [init STYLE file]: https://chromium.googlesource.com/chromiumos/platform2/+/master/init/STYLE |
| 215 | [init README file]: https://chromium.googlesource.com/chromiumos/platform2/+/master/init/README |
| 216 | [chromiumos-overlay repository]: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master |
| 217 | [Ebuild Writing]: https://devmanual.gentoo.org/ebuild-writing/index.html |
| 218 | [Ebuild File Format]: https://devmanual.gentoo.org/ebuild-writing/file-format/index.html |
| 219 | [Google shell style guide]: https://google.github.io/styleguide/shell.xml |
| 220 | [Chromium OS shell style guidelines]: https://www.chromium.org/chromium-os/shell-style-guidelines |
| 221 | [Google Python style guide]: https://google.github.io/styleguide/pyguide.html |
| 222 | [Chromium OS Python style guidelines]: https://www.chromium.org/chromium-os/python-style-guidelines |
| 223 | [autotest coding style]: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/docs/coding-style.md |
| 224 | [Google Test]: https://github.com/google/googletest |
| 225 | [Why Google C++ Testing Framework?]: https://github.com/google/googletest/blob/master/googletest/docs/Primer.md |
| 226 | [Google Test FAQ]: https://github.com/google/googletest/blob/master/googletest/docs/FAQ.md |
| 227 | [unit testing document]: https://www.chromium.org/chromium-os/testing/adding-unit-tests-to-the-build |
| 228 | [stub or fake]: https://martinfowler.com/articles/mocksArentStubs.html#TheDifferenceBetweenMocksAndStubs |
| 229 | [Autotest]: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/docs/user-doc.md |
| 230 | [Chromium code review policy]: https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md |
| 231 | [Developer Guide's code review instructions]: https://www.chromium.org/chromium-os/developer-guide#TOC-Upload-your-changes-and-get-a-code-review |
| 232 | [Gerrit notification settings]: https://chromium-review.googlesource.com/settings/#Notifications |
| 233 | [chromium issue tracker]: https://bugs.chromium.org/p/chromium/issues/list |
| 234 | [Chromium technical discussion groups]: https://www.chromium.org/developers/technical-discussion-groups |
| 235 | [Chromium OS technical discussion groups]: https://www.chromium.org/chromium-os/discussion-groups |
| 236 | [chromium-os-dev]: https://groups.google.com/a/chromium.org/forum/#!forum/chromium-os-dev |
| 237 | [chromium-dev]: https://groups.google.com/a/chromium.org/forum/#!forum/chromium-dev |