|
| 1 | +# Contributing to the MCUboot project |
| 2 | + |
| 3 | +When contributing to MCUboot, please first discuss the changes you |
| 4 | +wish to make via issue, email or slack. |
| 5 | + |
| 6 | +## Pull Request Process |
| 7 | + |
| 8 | +1. Please ensure that the simulator tests pass. If you are modifying |
| 9 | + existing functionality, it may be sufficient to just ensure that |
| 10 | + these tests pass. If you are adding new functionality, you must |
| 11 | + also extend the simulator to test this new functionality. |
| 12 | +1. Ensure your changes follow the coding and style conventions for the |
| 13 | + files you are modifying (see below). |
| 14 | +1. If you wish (or are required to), be sure that copyright statements |
| 15 | + are up to date. Note that we follow the general guidance used in |
| 16 | + the Linux kernel, and adding a copyright statement is generally |
| 17 | + only acceptable for "significant" changes to a file. |
| 18 | +1. Be sure that all of your commits are signed off. Contributions |
| 19 | + must be released under the Apache 2.0 license, following the |
| 20 | + [Developer Certificate of |
| 21 | + Origin](https://developercertificate.org/). |
| 22 | +1. All contributions should follow the project code of conduct found |
| 23 | + in the root of the project directory. |
| 24 | +1. Pull requests will trigger various builds and tests. These must |
| 25 | + all pass. |
| 26 | +1. Non-trivial changes must have a release-note stub (see below). |
| 27 | + |
| 28 | +## Coding style and guidelines |
| 29 | + |
| 30 | +### Mynewt coding |
| 31 | + |
| 32 | +A majority of the code in this project is written in C, and intended |
| 33 | +to run on embedded devices. This code began as a part of the mynewt |
| 34 | +RTOS, and still follows most of the [coding |
| 35 | +conventions](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md) |
| 36 | +of that project. The primary exception is the copyright header, which |
| 37 | +is not appropriate as this is not an Apache project. |
| 38 | + |
| 39 | +For files that came from MyNewt, we keep the existing Apache header, |
| 40 | +and add an additional header for MCUboot. In general, the following |
| 41 | +is sufficient. |
| 42 | + |
| 43 | +```c |
| 44 | +/* |
| 45 | + * SPDX-License-Identifier: Apache-2.0 |
| 46 | + */ |
| 47 | +``` |
| 48 | + |
| 49 | +this can be followed by individual contributor copyrights, if that is |
| 50 | +necessary. |
| 51 | + |
| 52 | +### General guidelines |
| 53 | + |
| 54 | +Although MCUboot needs to be small and compact, please try to hold |
| 55 | +back on the use of ifdefs in the code. It is better to factor out |
| 56 | +common code, and have different features in separate files, protected |
| 57 | +by a single ifdef than to have a large function with ifdefs sprinkled |
| 58 | +throughout it. |
| 59 | + |
| 60 | +## Simulator coding |
| 61 | + |
| 62 | +The simulator `sim` is an important part of what makes mcuboot reliable and |
| 63 | +robust. As such, it is important to make sure that changes made to the code are |
| 64 | +being tested by the simulator. |
| 65 | + |
| 66 | +The simulator puts the boot code through an intense stress test, testing |
| 67 | +thousands of possibly failures (including numerous power failure conditions). |
| 68 | +It would take very long, and be very difficult to test these cases on target, |
| 69 | +and most devices would wear out after just a few runs. |
| 70 | + |
| 71 | +As such, development should primarily be done using the simulator. Testing on |
| 72 | +actual targets should be done _after_ the code is working in the simulator, as a |
| 73 | +sanity test that it really works. |
| 74 | + |
| 75 | +Because of this, changes that change the behavior of the bootloader need to also |
| 76 | +be demonstrated to be tested by the simulator. For many changes, this means |
| 77 | +changing the simulator itself to test the new configuration. For example, a |
| 78 | +change that makes a new device work needs to have that device's flash |
| 79 | +configuration added to the simulator to demonstrate that it doesn't cause |
| 80 | +problems with corner cases and bad powerdowns. |
| 81 | + |
| 82 | +### Running the simulator |
| 83 | + |
| 84 | +CI will run the simulator through a multitude of configurations. See |
| 85 | +`.github/workflows/sim.yaml` for a full list. Individual configurations can |
| 86 | +easily be tested from the command line with something like: |
| 87 | + |
| 88 | +```bash |
| 89 | +$ cargo test \ |
| 90 | + --features "sig-ecdsa-mbedtls swap-move" \ |
| 91 | + -- --nocapture --test-threads 1 |
| 92 | +``` |
| 93 | + |
| 94 | +Additional arguments will be patterns to match the tests cases that can be run. |
| 95 | + |
| 96 | +To run one of these tests in the debugger, you can do something like: |
| 97 | + |
| 98 | +```bash |
| 99 | +$ cargo test --features "swap-move" --no-run |
| 100 | +``` |
| 101 | + |
| 102 | +This will print out the executables generated, and you will typically want the |
| 103 | +executable that starts with 'core-' followed by a hash. |
| 104 | + |
| 105 | +Running this executable, with the `--nocapture --test-threads 1` arguments, and |
| 106 | +possibly test filters can be done with the debugger to allow the debugger to be |
| 107 | +used on the code. |
| 108 | + |
| 109 | +## Release notes |
| 110 | + |
| 111 | +To facility creating release notes at release time, all non-trivial |
| 112 | +changes must include a release note snippet with the pull request. |
| 113 | +This can be either a separate commit, or as part of a single commit |
| 114 | +(generally, if there are multiple commits to the pull request, the |
| 115 | +release note snippet should be a separate commit, and a pull request |
| 116 | +that is a single commit can include the release note snippet in that |
| 117 | +commit). |
| 118 | + |
| 119 | +The release notes should be placed in the `docs/release-notes.d` |
| 120 | +directory. Please see the readme file in that directory for specifics. |
0 commit comments