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, |
Daniel Erat | e3b67cd | 2017-03-29 00:22:42 | [diff] [blame] | 66 | non-persistent tasks that need to run periodically on Chrome OS systems, but |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 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. |
Daniel Erat | e3b67cd | 2017-03-29 00:22:42 | [diff] [blame] | 77 | * Portage `.ebuild` files, as in the [chromiumos-overlay repository]. We |
| 78 | follow the upstream guidelines; see the [Ebuild Writing] page and |
| 79 | specifically the [Ebuild File Format]. |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 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 |
Daniel Erat | e3b67cd | 2017-03-29 00:22:42 | [diff] [blame] | 84 | take precedence when writing those types of scripts. |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 85 | |
Mattias Nissler | 32b6aab | 2017-04-06 12:28:11 | [diff] [blame^] | 86 | For shell scripts that ship with the OS image, be extra careful. The shell |
| 87 | provides powerful features, but the flip side is that security pitfalls are |
| 88 | tricky to avoid. Think twice whether your shell statements can have unintended |
| 89 | side effects, in particular if your script runs with full privileges (as is the |
| 90 | case with init scripts). As a guideline, keep things simple and move more |
| 91 | complex processing to a properly sandboxed environment in an C++ daemon. |
| 92 | |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 93 | ### Python |
| 94 | |
| 95 | The Python interpreter is not included in production Chrome OS system images, |
| 96 | but Python is used heavily for development and testing. |
| 97 | |
| 98 | We largely follow the [Google Python style guide], but see the |
| 99 | [Chromium OS Python style guidelines] for important differences, particularly |
Daniel Erat | e3b67cd | 2017-03-29 00:22:42 | [diff] [blame] | 100 | around indenting and naming. For tests, see the [autotest coding style]. |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 101 | |
| 102 | ## Testing |
| 103 | |
Daniel Erat | 03138d4 | 2017-03-29 15:08:01 | [diff] [blame] | 104 | The [Chromium OS testing site] is the main repository of information about |
| 105 | testing. |
| 106 | |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 107 | ### Unit tests |
| 108 | |
| 109 | Unit tests should be added alongside new code. It's important to design your |
| 110 | code with testability in mind, as adding tests after-the-fact often requires |
| 111 | heavy refactoring. |
| 112 | |
| 113 | Good unit tests are fast, lightweight, reliable, and easy to run within the |
| 114 | chroot as part of your development workflow. We use [Google Test] (which is |
Daniel Erat | e3b67cd | 2017-03-29 00:22:42 | [diff] [blame] | 115 | comprised of the GoogleTest unit testing framework and the GoogleMock mocking |
| 116 | framework) to test C++ code. [Why Google C++ Testing Framework?] and the |
| 117 | [Google Test FAQ] are good introductions, and the [unit testing document] has |
| 118 | more details about how unit tests get run. |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 119 | |
| 120 | Note that although GoogleMock is permitted, there are some pitfalls: |
| 121 | |
| 122 | * When creating partial mocks of classes, it's easy to accidentally call |
| 123 | production code within unit tests. |
| 124 | * GoogleMock often produces spammy, hard-to-read output when a test calls |
| 125 | unimportant methods that haven't been mocked. |
Daniel Erat | e3b67cd | 2017-03-29 00:22:42 | [diff] [blame] | 126 | * On the other hand, it's tedious to maintain long lists of expectations for |
| 127 | methods that you don't actually care about. |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 128 | |
| 129 | GoogleMock can help verify complex interactions like multiple objects' methods |
| 130 | being called in a specific order, but for simpler cases it's often cleaner to |
| 131 | define interfaces for the tested class's dependencies and write your own |
| 132 | [stub or fake] implementations. |
| 133 | |
| 134 | ### Autotest |
| 135 | |
| 136 | [Autotest] is used to run tests on live Chrome OS systems. Autotests are useful |
| 137 | for performing integration testing (e.g. verifying that two processes are able |
| 138 | to communicate with each other over IPC), but they have heavy costs: |
| 139 | |
| 140 | * Autotests are harder to run than unit tests: you need either a dedicated |
| 141 | test device or a virtual machine. |
| 142 | * Autotests are much slower than unit tests: even a no-op test can take 30 |
| 143 | seconds or longer per run. |
| 144 | * Since autotests involve at least a controlling system and a test device, |
| 145 | they're susceptible to networking issues and hardware flakiness. |
| 146 | * Since autotests run on full, live systems, failures can be caused by issues |
| 147 | in components unrelated to the one that you're trying to test. |
| 148 | |
| 149 | Whenever you can get equivalent coverage from either unit tests or autotests, |
| 150 | use unit tests. Design your system with unit testing in mind. |
| 151 | |
| 152 | ## Code reviews |
| 153 | |
| 154 | The Chromium OS project follows the [Chromium code review policy]: all code and |
| 155 | data changes must be reviewed by another project member before being committed. |
| 156 | Note that Chromium OS's review process has some differences; see the |
| 157 | [Developer Guide's code review instructions]. |
| 158 | |
| 159 | OWNERS files are not (yet) enforced for non-browser parts of the Chromium OS |
| 160 | codebase, but please act as if they were. If there's an OWNERS file in the |
| 161 | directory that you're modifying or a parent directory, add at least one |
| 162 | developer that's listed in it to your code review and wait for their approval |
| 163 | before committing your change. |
| 164 | |
| 165 | Owners may want to consider setting up notifications for changes to their code. |
| 166 | To receive notifications of changes to `src/platform2/debugd`, open your |
| 167 | [Gerrit notification settings] and add a new entry for project |
| 168 | `chromiumos/platform2` and expression `file:"^debugd/.*"`. |
| 169 | |
| 170 | ## Issue trackers |
| 171 | |
| 172 | Public Chromium OS bugs and feature requests are tracked using the |
| 173 | [chromium issue tracker]. Note that this tracker is shared with the Chromium |
| 174 | project; most OS-specific issues are classified under an `OS>`-prefixed |
| 175 | component and have an `OS=Chrome` label. The `crbug.com` redirector makes it |
| 176 | easy to jump directly to an issue with a given ID; `https://crbug.com/123` will |
| 177 | redirect to issue #123, for instance. |
| 178 | |
| 179 | Keep discussion in the issue tracker instead of in email or over IM. Issues |
| 180 | remain permanently available and are viewable by people not present for the |
| 181 | original discussion; email and IM exchanges are harder to find after the fact |
| 182 | and may even be deleted automatically. `BUG=chromium:123` lines in commit |
| 183 | descriptions and `https://crbug.com/123` mentions in code comments also make it |
| 184 | easy to link back to the issue that describes the original motivation for a |
| 185 | change. |
| 186 | |
| 187 | Avoid discussing multiple problems in a single issue. It's not possible to split |
| 188 | an existing issue into multiple new issues, and it can take a long time to read |
| 189 | through an issue's full history to figure out what is currently being discussed. |
| 190 | |
| 191 | Similarly, do not reopen an old, closed issue in response to the reoccurrence of |
| 192 | a bug: the old issue probably contains now-irrelevant milestone and merge labels |
| 193 | and outdated information. Instead, create a new issue and refer to the prior |
| 194 | issue in its initial description. Text of the form `issue 123` will |
| 195 | automatically be turned into a link. |
| 196 | |
Daniel Erat | 03138d4 | 2017-03-29 15:08:01 | [diff] [blame] | 197 | There is much more information about filing bugs and using labels in the |
| 198 | [Chromium bug reporting guidelines]. |
| 199 | |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 200 | ## Mailing lists |
| 201 | |
| 202 | We use the [Chromium technical discussion groups] and |
| 203 | [Chromium OS technical discussion groups] for development-related |
| 204 | discussion. Most Chromium OS discussion takes place on [chromium-os-dev] (for |
| 205 | OS-related topics) and [chromium-dev] (for browser-related topics). You should |
| 206 | subscribe to chromium-os-dev if you're doing Chromium OS development, as |
| 207 | important PSAs are frequently sent there. |
| 208 | |
| 209 | ## Developing in the open |
| 210 | |
| 211 | Chromium OS is an open-source project. Whenever possible (i.e. when not |
| 212 | discussing private, partner-related information), use the public issue tracker |
| 213 | and mailing lists rather than the internal versions. |
| 214 | |
| 215 | [documentation guidelines]: https://chromium.googlesource.com/chromium/src/+/master/docs/documentation_guidelines.md |
| 216 | [documentation best practices]: https://chromium.googlesource.com/chromium/src/+/master/docs/documentation_best_practices.md |
| 217 | [Chromium OS design docs]: https://www.chromium.org/chromium-os/chromiumos-design-docs |
| 218 | [Chromium design docs]: https://www.chromium.org/developers/design-documents |
| 219 | [Google C++ style guide]: https://google.github.io/styleguide/cppguide.html |
| 220 | [Chromium C++ style guide]: https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md |
| 221 | [C++11 use in Chromium]: https://chromium-cpp.appspot.com/ |
| 222 | [platform2 repository]: https://dev.chromium.org/chromium-os/getting-started-with-platform2 |
| 223 | [Linux kernel coding style]: https://github.com/torvalds/linux/blob/master/Documentation/process/coding-style.rst |
| 224 | [Kernel FAQ]: https://www.chromium.org/chromium-os/how-tos-and-troubleshooting/kernel-faq |
| 225 | [EC Development page]: https://www.chromium.org/chromium-os/ec-development |
| 226 | [src/platform2/init]: https://chromium.googlesource.com/chromiumos/platform2/+/master/init/ |
| 227 | [init STYLE file]: https://chromium.googlesource.com/chromiumos/platform2/+/master/init/STYLE |
| 228 | [init README file]: https://chromium.googlesource.com/chromiumos/platform2/+/master/init/README |
| 229 | [chromiumos-overlay repository]: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master |
| 230 | [Ebuild Writing]: https://devmanual.gentoo.org/ebuild-writing/index.html |
| 231 | [Ebuild File Format]: https://devmanual.gentoo.org/ebuild-writing/file-format/index.html |
| 232 | [Google shell style guide]: https://google.github.io/styleguide/shell.xml |
| 233 | [Chromium OS shell style guidelines]: https://www.chromium.org/chromium-os/shell-style-guidelines |
| 234 | [Google Python style guide]: https://google.github.io/styleguide/pyguide.html |
| 235 | [Chromium OS Python style guidelines]: https://www.chromium.org/chromium-os/python-style-guidelines |
| 236 | [autotest coding style]: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/docs/coding-style.md |
Daniel Erat | 03138d4 | 2017-03-29 15:08:01 | [diff] [blame] | 237 | [Chromium OS testing site]: https://www.chromium.org/chromium-os/testing |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 238 | [Google Test]: https://github.com/google/googletest |
| 239 | [Why Google C++ Testing Framework?]: https://github.com/google/googletest/blob/master/googletest/docs/Primer.md |
| 240 | [Google Test FAQ]: https://github.com/google/googletest/blob/master/googletest/docs/FAQ.md |
| 241 | [unit testing document]: https://www.chromium.org/chromium-os/testing/adding-unit-tests-to-the-build |
| 242 | [stub or fake]: https://martinfowler.com/articles/mocksArentStubs.html#TheDifferenceBetweenMocksAndStubs |
| 243 | [Autotest]: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/docs/user-doc.md |
| 244 | [Chromium code review policy]: https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md |
| 245 | [Developer Guide's code review instructions]: https://www.chromium.org/chromium-os/developer-guide#TOC-Upload-your-changes-and-get-a-code-review |
| 246 | [Gerrit notification settings]: https://chromium-review.googlesource.com/settings/#Notifications |
| 247 | [chromium issue tracker]: https://bugs.chromium.org/p/chromium/issues/list |
Daniel Erat | 03138d4 | 2017-03-29 15:08:01 | [diff] [blame] | 248 | [Chromium bug reporting guidelines]: https://www.chromium.org/for-testers/bug-reporting-guidelines |
Daniel Erat | 0576ce7 | 2017-03-27 05:43:06 | [diff] [blame] | 249 | [Chromium technical discussion groups]: https://www.chromium.org/developers/technical-discussion-groups |
| 250 | [Chromium OS technical discussion groups]: https://www.chromium.org/chromium-os/discussion-groups |
| 251 | [chromium-os-dev]: https://groups.google.com/a/chromium.org/forum/#!forum/chromium-os-dev |
| 252 | [chromium-dev]: https://groups.google.com/a/chromium.org/forum/#!forum/chromium-dev |