• Home
  • Line#
  • Scopes#
  • Navigate#
  • Raw
  • Download
1.. _docs-contributing:
2
3============
4Contributing
5============
6We'd love to accept your patches and contributions to Pigweed. There are just a
7few small guidelines you need to follow.
8
9Before participating in our community, please take a moment to review our
10:ref:`docs-code-of-conduct`. We expect everyone who interacts with the project
11to respect these guidelines.
12
13Get started
14-----------
15See :ref:`docs-get-started-upstream`.
16
17Pigweed contribution overview
18-----------------------------
19.. note::
20
21  If you have any trouble with this flow, reach out in our `chat room
22  <https://discord.gg/M9NSeTA>`_ or on the `mailing list
23  <https://groups.google.com/forum/#!forum/pigweed>`_ for help.
24
25One-time contributor setup
26^^^^^^^^^^^^^^^^^^^^^^^^^^
27#. Sign the
28   `Contributor License Agreement <https://cla.developers.google.com/>`_.
29#. Verify that your Git user email (git config user.email) is either Google
30   Account email or an Alternate email for the Google account used to sign
31   the CLA (Manage Google account → Personal Info → email).
32#. Obtain a login cookie from Gerrit's
33   `new-password <https://pigweed.googlesource.com/new-password>`_ page.
34#. Install the :ref:`gerrit-commit-hook` to automatically add a
35   ``Change-Id: ...`` line to your commit.
36#. Install the Pigweed presubmit check hook with ``pw presubmit --install``.
37   Remember to :ref:`activate-pigweed-environment` first!
38
39Presubmission process
40^^^^^^^^^^^^^^^^^^^^^
41Before making or sending major changes or SEEDs, please reach out in our
42`chat room <https://discord.gg/M9NSeTA>`_ or on the `mailing list
43<https://groups.google.com/forum/#!forum/pigweed>`_ first to ensure the changes
44make sense for upstream. We generally go through a design phase before making
45large changes. See :ref:`SEED-0001` for a description of this process; but
46please discuss with us before writing a full SEED. Let us know of any
47priorities, timelines, requirements, and limitations ahead of time.
48
49For minor changes that don't fit the SEED process, follow the
50:ref:`docs-code_reviews-small-changes` guidance and the `Change submission
51process`_.
52
53.. warning::
54   Skipping communicating with us before doing large amounts of work risks
55   accepting your contribution. Communication is key!
56
57Change submission process
58^^^^^^^^^^^^^^^^^^^^^^^^^
59
60.. note::
61   A change is a single git commit, and is also known as Change List or CL for
62   short.
63
64.. tip::
65   Follow :ref:`docs-code_reviews-small-changes` for a smooth submission process
66
67#. Go through the `Presubmission process`_ and review this document's guidance.
68#. Ensure all files include the correct copyright and license headers.
69#. Include any necessary changes to the documentation.
70#. Run :ref:`module-pw_presubmit` to detect style or compilation issues before
71   uploading.
72#. Upload the change with ``git push origin HEAD:refs/for/main``.
73#. Add ``gwsq-pigweed@pigweed.google.com.iam.gserviceaccount.com`` as a
74   reviewer. This will automatically choose an appropriate person to review the
75   change.
76#. Address any reviewer feedback by amending the commit
77   (``git commit --amend``).
78#. Submit change to CI builders to merge. If you are not part of Pigweed's
79   core team, you can ask the reviewer to add the `+2 CQ` vote, which will
80   trigger a rebase and submit once the builders pass.
81
82Contributor License Agreement
83-----------------------------
84Contributions to this project must be accompanied by a Contributor License
85Agreement. You (or your employer) retain the copyright to your contribution;
86this simply gives us permission to use and redistribute your contributions as
87part of the project. Head over to <https://cla.developers.google.com/> to see
88your current agreements on file or to sign a new one.
89
90You generally only need to submit a CLA once, so if you've already submitted one
91(even if it was for a different project), you probably don't need to do it
92again.
93
94.. _docs-contributing-contribution-standards:
95
96Contribution Standards
97----------------------
98Contributions, i.e. CLs, are expected to be complete solutions, accompanied by
99documentation and unit tests when merited, e.g. new code, bug fixes, or code
100that changes some behavior. This also means that code changes must be tested.
101Tests will avoid or minimize any negative impact to Pigweed users, while
102documentation will help others learn about the new changes.
103
104We understand that you may have different priorities or not know the best way to
105complete your contribution. In that case, reach out via our `chat room
106<https://discord.gg/M9NSeTA>`_ or on the `mailing list
107<https://groups.google.com/forum/#!forum/pigweed>`_ for help or `File a bug
108<https://issues.pigweed.dev/issues?q=status:open>`_
109requesting fixes describing how this may be blocking you. Otherwise you risk
110working on a CL that does not match our vision and could be rejected. To keep
111our focus, we cannot adopt incomplete CLs.
112
113.. _gerrit-commit-hook:
114
115Build System Support
116--------------------
117Pigweed users are split across a number of build systems including:
118
119* `Bazel <https://bazel.build/>`_
120* `GN (a ninja generator) <https://gn.googlesource.com/gn/>`_
121* `CMake <https://cmake.org/>`_
122* `Soong (Android's build system) <https://source.android.com/docs/setup/build>`_
123
124In order to ensure parity between different build systems, contributions must
125include support for at least Bazel (``BUILD.bazel``), GN (``BUILD.gn``) and
126CMake (``CMakeLists.txt``).
127
128We understand that most people don't have experience with all of the build
129systems Pigweed supports, and that this requirement is a burden on contributors.
130We find most people are able to follow the patterns in existing build files to
131add support for their changes; but not always. We're happy to help with build
132questions on Discord for that reason. Don't be shy if you need help!
133
134Gerrit Commit Hook
135------------------
136Gerrit requires all changes to have a ``Change-Id`` tag at the bottom of each
137commit message. You should set this up to be done automatically using the
138instructions below.
139
140The commands below assume that your current working directory is the root
141of your Pigweed repository.
142
143**Linux/macOS**
144
145.. code-block:: bash
146
147   f=`git rev-parse --git-dir`/hooks/commit-msg ; mkdir -p $(dirname $f) ; curl -Lo $f https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; chmod +x $f
148
149**Windows**
150
151.. code-block:: batch
152
153   git rev-parse --git-dir > gitrepopath.txt & set /p "g="< gitrepopath.txt & del gitrepopath.txt & call set "f=%g%/hooks" & call mkdir "%f%" & call curl -Lo "%f%/commit-msg" https://gerrit-review.googlesource.com/tools/hooks/commit-msg
154
155Commit Message
156--------------
157See the :ref:`commit message section of the style
158guide<docs-pw-style-commit-message>` for how commit messages should look.
159
160Documentation
161-------------
162Most changes to Pigweed should have an associated documentation change.
163
164Building
165^^^^^^^^
166To build the documentation, follow the :ref:`getting
167started<docs-get-started-upstream>` guide so you can build Pigweed. Then:
168
169#. Change to your checkout directory and ``. activate.sh`` if necessary
170#. Run ``pw watch -C out`` to build the code, run tests, and build docs
171#. Wait for the build to finish (see a ``PASS``)
172#. Navigate to  ``<CHECKOUT>/out/docs/gen/docs/html/index.html``
173#. Edit the relevant ``.rst`` file. Save when ready
174#. Refresh your browser after the build completes
175
176Alternately, you can use the local webserver in watch; this works better for
177some pages that have external resources: ``pw watch --serve-docs`` then
178navigate to `http://localhost:8000 <http://localhost:8000>`_ in your browser.
179
180Submission checklist
181^^^^^^^^^^^^^^^^^^^^
182All Pigweed changes must either:
183
184#. Include updates to documentation, or
185#. Include ``No-Docs-Update-Reason: <reason>`` in a Gerrit comment on the CL.
186   For example:
187
188   * ``No-Docs-Update-Reason: formatting tweaks``
189   * ``No-Docs-Update-Reason: internal cleanups``
190   * ``No-Docs-Update-Reason: bugfix``
191
192It's acceptable to only document new changes in an otherwise underdocumented
193module, but it's not acceptable to not document new changes because the module
194doesn't have any other documentation.
195
196Code Reviews
197------------
198See :ref:`docs-code_reviews` for information about the code review process.
199
200Experimental Repository and Where to Land Code
201----------------------------------------------
202Pigweed's has an `Experimental Repository
203<https://pigweed.googlesource.com/pigweed/experimental>`_ which differs from
204our main repository in a couple key ways:
205
206* Code is not expected to become production grade.
207* Code review standards are relaxed to allow experimentation.
208* In general the value of the code in the repository is the knowledge gained
209  from the experiment, not the code itself.
210
211Good uses of the repo include:
212
213* Experimenting with using an API (ex. C++20 coroutines) with no plans to
214  turn it into production code.
215* One-off test programs to gather data.
216
217We would like to avoid large pieces of code being developed in the experimental
218repository then imported into the Pigweed repository. If large amounts of code
219end up needing to migrate from experimental to main, then it must be landed
220incrementally as a series of reviewable patches, typically no
221`larger than 500 lines each
222<https://google.github.io/eng-practices/review/developer/small-cls.html>`_.
223This creates a large code review burden that often results in poorer reviews.
224Therefore, if the eventual location of the code will be the main Pigweed
225repository, it is **strongly encouraged** that the code be developed in the
226**main repository under an experimental flag**.
227
228.. note::
229   The current organization of the experimental repository does not reflect
230   this policy. This will be re-organized once we have concrete recommendations
231   on its organization.
232
233Community Guidelines
234--------------------
235This project follows `Google's Open Source Community Guidelines
236<https://opensource.google/conduct/>`_ and the :ref:`docs-code-of-conduct`.
237
238Presubmit Checks and Continuous Integration
239-------------------------------------------
240All Pigweed change lists (CLs) must adhere to Pigweed's style guide and pass a
241suite of automated builds, tests, and style checks to be merged upstream. Much
242of this checking is done using Pigweed's ``pw_presubmit`` module by automated
243builders. These builders run before each Pigweed CL is submitted and in our
244continuous integration infrastructure (see `Pigweed's build console
245<https://ci.chromium.org/p/pigweed/g/pigweed/console>`_).
246
247Running Presubmit Checks
248^^^^^^^^^^^^^^^^^^^^^^^^
249To run automated presubmit checks on a pending CL, click the ``CQ DRY RUN``
250button in the Gerrit UI. The results appear in the Tryjobs section, below the
251source listing. Jobs that passed are green; jobs that failed are red.
252
253If all checks pass, you will see a ``Dry run: This CL passed the CQ dry run.``
254comment on your change. If any checks fail, you will see a ``Dry run: Failed
255builds:`` message. All failures must be addressed before submitting.
256
257In addition to the publicly visible presubmit checks, Pigweed runs internal
258presubmit checks that are only visible within Google. If any these checks fail,
259external developers will see a ``Dry run: Failed builds:`` comment on the CL,
260even if all visible checks passed. Reach out to the Pigweed team for help
261addressing these issues.
262
263Project Presubmit Checks
264^^^^^^^^^^^^^^^^^^^^^^^^
265In addition to Pigweed's presubmit checks, some projects that use Pigweed run
266their presubmit checks in Pigweed's infrastructure. This supports a development
267flow where projects automatically update their Pigweed submodule if their tests
268pass. If a project cannot build against Pigweed's tip-of-tree, it will stay on
269a fixed Pigweed revision until the issues are fixed. See the `examples
270<https://pigweed.googlesource.com/pigweed/examples/>`_ repo for an example of
271this.
272
273Pigweed does its best to keep builds passing for dependent projects. In some
274circumstances, the Pigweed maintainers may choose to merge changes that break
275dependent projects. This will only be done if
276
277* a feature or fix is needed urgently in Pigweed or for a different project,
278  and
279* the project broken by the change does not imminently need Pigweed updates.
280
281The downstream project will continue to build against their last working
282revision of Pigweed until the incompatibilities are fixed.
283
284In these situations, Pigweed's commit queue submission process will fail for all
285changes. If a change passes all presubmit checks except for known failures, the
286Pigweed team may permit manual submission of the CL. Contact the Pigweed team
287for submission approval.
288
289Code coverage in Gerrit
290^^^^^^^^^^^^^^^^^^^^^^^
291Unit test coverage data for C++ is computed by the ``coverage`` builder and
292displayed in Gerrit.
293
294.. image:: https://storage.googleapis.com/pigweed-media/gerrit_code_coverage.png
295   :width: 800
296   :alt: Code coverage display in Gerrit
297
298#. **When will coverage data be visible on my CL?** The coverage builder needs
299   to finish running (about 6 minutes), and then the data needs to be ingested
300   by the coverage pipeline (ran every 30 minutes).
301
302#. **What tests is this based on?** Only the C++ unit tests of the modules ran
303   as part of the GN build. (There's no coverage data for Python or Rust yet.)
304
305#. **Can I generate a coverage report locally?** Yes. Running ``pw
306   presubmit --step coverage`` will generate a HTML report at
307   ``out/presubmit/coverage/host_clang_coverage/obj/coverage_report/html/index.html``.
308
309#. **I'd love to have this in my Pigweed-based project!** See
310   :ref:`module-pw_build-gn-pw_coverage_report` for GN and
311   :ref:`docs-build_system-bazel_coverage` for Bazel.
312
313Running local presubmits
314------------------------
315To speed up the review process, consider adding :ref:`module-pw_presubmit` as a
316git push hook using the following command:
317
318Linux/macOS
319^^^^^^^^^^^
320.. code-block:: bash
321
322   $ pw presubmit --install
323
324This will be effectively the same as running the following command before every
325``git push``:
326
327.. code-block:: bash
328
329   $ pw presubmit
330
331
332.. image:: ../../pw_presubmit/docs/pw_presubmit_demo.gif
333  :width: 800
334  :alt: pw presubmit demo
335
336If you ever need to bypass the presubmit hook (due to it being broken, for
337example) you may push using this command:
338
339.. code-block:: bash
340
341   $ git push origin HEAD:refs/for/main --no-verify
342
343Presubmit and branch management
344^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
345When creating new feature branches, make sure to specify the upstream branch to
346track, e.g.
347
348.. code-block:: bash
349
350   $ git checkout -b myfeature origin/main
351
352When tracking an upstream branch, ``pw presubmit`` will only run checks on the
353modified files, rather than the entire repository.
354
355Presubmit flags
356^^^^^^^^^^^^^^^
357``pw presubmit`` can accept a number of flags
358
359``-b commit, --base commit``
360  Git revision against which to diff for changed files. Default is the tracking
361  branch of the current branch. Set commit to "HEAD" to check files added or
362  modified but not yet commited. Cannot be used with --full.
363
364``--full``
365  Run presubmit on all files, not just changed files. Cannot be used with
366  --base.
367
368``-e regular_expression, --exclude regular_expression``
369  Exclude paths matching any of these regular expressions, which are interpreted
370  relative to each Git repository's root.
371
372``-k, --keep-going``
373  Continue instead of aborting when errors occur.
374
375``--output-directory OUTPUT_DIRECTORY``
376  Output directory (default: <repo root>/out/presubmit)
377
378``--package-root PACKAGE_ROOT``
379  Package root directory (default: <output directory>/packages)
380
381``--clear, --clean``
382  Delete the presubmit output directory and exit.
383
384``-p, --program PROGRAM``
385  Which presubmit program to run
386
387``--step STEP``
388  Provide explicit steps instead of running a predefined program.
389
390``--install``
391  Install the presubmit as a Git pre-push hook and exit.
392
393.. _Sphinx: https://www.sphinx-doc.org/
394
395.. inclusive-language: disable
396
397.. _reStructuredText Primer: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html
398
399.. inclusive-language: enable
400
401.. _docs-contributing-presubmit-virtualenv-hashes:
402
403Updating Python dependencies in the virtualenv_setup directory
404^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
405If you update any of the requirements or constraints files in
406``//pw_env_setup/py/pw_env_setup/virtualenv_setup``, you must run this command
407to ensure that all of the hashes are updated:
408
409.. code-block:: console
410
411   pw presubmit --step update_upstream_python_constraints --full
412
413For Python packages that have native extensions, the command needs to be run 3
414times: once on Linux, once on macOS, and once on Windows. Please run it on the
415OSes that are available to you; a core Pigweed teammate will run it on the rest.
416See the warning about caching Python packages for multiple platforms in
417:ref:`docs-python-build-downloading-packages`.
418
419.. toctree::
420   :maxdepth: 1
421   :hidden:
422
423   ../embedded_cpp_guide
424   ../style_guide
425   ../code_reviews
426   ../code_of_conduct
427   docs/index
428