xref: /aosp_15_r20/external/bazelbuild-rules_python/CONTRIBUTING.md (revision 60517a1edbc8ecf509223e9af94a7adec7d736b8)
1*60517a1eSAndroid Build Coastguard Worker# How to contribute
2*60517a1eSAndroid Build Coastguard Worker
3*60517a1eSAndroid Build Coastguard WorkerWe'd love to accept your patches and contributions to this project. There are
4*60517a1eSAndroid Build Coastguard Workerjust a few small guidelines you need to follow.
5*60517a1eSAndroid Build Coastguard Worker
6*60517a1eSAndroid Build Coastguard Worker## Getting started
7*60517a1eSAndroid Build Coastguard Worker
8*60517a1eSAndroid Build Coastguard WorkerBefore we can work on the code, we need to get a copy of it and setup some
9*60517a1eSAndroid Build Coastguard Workerlocal environment and tools.
10*60517a1eSAndroid Build Coastguard Worker
11*60517a1eSAndroid Build Coastguard WorkerFirst, fork the code to your user and clone your fork. This gives you a private
12*60517a1eSAndroid Build Coastguard Workerplayground where you can do any edits you'd like. For this guide, we'll use
13*60517a1eSAndroid Build Coastguard Workerthe [GitHub `gh` tool](https://github.com/cli/cli)
14*60517a1eSAndroid Build Coastguard Worker([Linux install](https://github.com/cli/cli/blob/trunk/docs/install_linux.md)).
15*60517a1eSAndroid Build Coastguard Worker(More advanced users may prefer the GitHub UI and raw `git` commands).
16*60517a1eSAndroid Build Coastguard Worker
17*60517a1eSAndroid Build Coastguard Worker```shell
18*60517a1eSAndroid Build Coastguard Workergh repo fork bazelbuild/rules_python --clone --remote
19*60517a1eSAndroid Build Coastguard Worker```
20*60517a1eSAndroid Build Coastguard Worker
21*60517a1eSAndroid Build Coastguard WorkerNext, make sure you have a new enough version of Python installed that supports the
22*60517a1eSAndroid Build Coastguard Workervarious code formatters and other devtools. For a quick start,
23*60517a1eSAndroid Build Coastguard Worker[install pyenv](https://github.com/pyenv/pyenv-installer) and
24*60517a1eSAndroid Build Coastguard Workerat least Python 3.9.15:
25*60517a1eSAndroid Build Coastguard Worker
26*60517a1eSAndroid Build Coastguard Worker```shell
27*60517a1eSAndroid Build Coastguard Workercurl https://pyenv.run | bash
28*60517a1eSAndroid Build Coastguard Workerpyenv install 3.9.15
29*60517a1eSAndroid Build Coastguard Workerpyenv shell 3.9.15
30*60517a1eSAndroid Build Coastguard Worker```
31*60517a1eSAndroid Build Coastguard Worker
32*60517a1eSAndroid Build Coastguard Worker## Development workflow
33*60517a1eSAndroid Build Coastguard Worker
34*60517a1eSAndroid Build Coastguard WorkerIt's suggested that you create what is called a "feature/topic branch" in your
35*60517a1eSAndroid Build Coastguard Workerfork when you begin working on code you want to eventually send or code review.
36*60517a1eSAndroid Build Coastguard Worker
37*60517a1eSAndroid Build Coastguard Worker```
38*60517a1eSAndroid Build Coastguard Workergit checkout main # Start our branch from the latest code
39*60517a1eSAndroid Build Coastguard Workergit checkout -b my-feature # Create and switch to our feature branch
40*60517a1eSAndroid Build Coastguard Workergit push origin my-feature # Cause the branch to be created in your fork.
41*60517a1eSAndroid Build Coastguard Worker```
42*60517a1eSAndroid Build Coastguard Worker
43*60517a1eSAndroid Build Coastguard WorkerFrom here, you then edit code and commit to your local branch. If you want to
44*60517a1eSAndroid Build Coastguard Workersave your work to github, you use `git push` to do so:
45*60517a1eSAndroid Build Coastguard Worker
46*60517a1eSAndroid Build Coastguard Worker```
47*60517a1eSAndroid Build Coastguard Workergit push origin my-feature
48*60517a1eSAndroid Build Coastguard Worker```
49*60517a1eSAndroid Build Coastguard Worker
50*60517a1eSAndroid Build Coastguard WorkerOnce the code is in your github repo, you can then turn it into a Pull Request
51*60517a1eSAndroid Build Coastguard Workerto the actual rules_python project and begin the code review process.
52*60517a1eSAndroid Build Coastguard Worker
53*60517a1eSAndroid Build Coastguard Worker
54*60517a1eSAndroid Build Coastguard Worker## Running tests
55*60517a1eSAndroid Build Coastguard Worker
56*60517a1eSAndroid Build Coastguard WorkerRunning tests is particularly easy thanks to Bazel, simply run:
57*60517a1eSAndroid Build Coastguard Worker
58*60517a1eSAndroid Build Coastguard Worker```
59*60517a1eSAndroid Build Coastguard Workerbazel test //...
60*60517a1eSAndroid Build Coastguard Worker```
61*60517a1eSAndroid Build Coastguard Worker
62*60517a1eSAndroid Build Coastguard WorkerAnd it will run all the tests it can find. The first time you do this, it will
63*60517a1eSAndroid Build Coastguard Workerprobably take long time because various dependencies will need to be downloaded
64*60517a1eSAndroid Build Coastguard Workerand setup. Subsequent runs will be faster, but there are many tests, and some of
65*60517a1eSAndroid Build Coastguard Workerthem are slow. If you're working on a particular area of code, you can run just
66*60517a1eSAndroid Build Coastguard Workerthe tests in those directories instead, which can speed up your edit-run cycle.
67*60517a1eSAndroid Build Coastguard Worker
68*60517a1eSAndroid Build Coastguard Worker## Formatting
69*60517a1eSAndroid Build Coastguard Worker
70*60517a1eSAndroid Build Coastguard WorkerStarlark files should be formatted by
71*60517a1eSAndroid Build Coastguard Worker[buildifier](https://github.com/bazelbuild/buildtools/blob/master/buildifier/README.md).
72*60517a1eSAndroid Build Coastguard WorkerOtherwise the Buildkite CI will fail with formatting/linting violations.
73*60517a1eSAndroid Build Coastguard WorkerWe suggest using a pre-commit hook to automate this.
74*60517a1eSAndroid Build Coastguard WorkerFirst [install pre-commit](https://pre-commit.com/#installation),
75*60517a1eSAndroid Build Coastguard Workerthen run
76*60517a1eSAndroid Build Coastguard Worker
77*60517a1eSAndroid Build Coastguard Worker```shell
78*60517a1eSAndroid Build Coastguard Workerpre-commit install
79*60517a1eSAndroid Build Coastguard Worker```
80*60517a1eSAndroid Build Coastguard Worker
81*60517a1eSAndroid Build Coastguard Worker### Running buildifer manually
82*60517a1eSAndroid Build Coastguard Worker
83*60517a1eSAndroid Build Coastguard WorkerYou can also run buildifier manually. To do this,
84*60517a1eSAndroid Build Coastguard Worker[install buildifier](https://github.com/bazelbuild/buildtools/blob/master/buildifier/README.md),
85*60517a1eSAndroid Build Coastguard Workerand run the following command:
86*60517a1eSAndroid Build Coastguard Worker
87*60517a1eSAndroid Build Coastguard Worker```shell
88*60517a1eSAndroid Build Coastguard Worker$ buildifier --lint=fix --warnings=native-py -warnings=all WORKSPACE
89*60517a1eSAndroid Build Coastguard Worker```
90*60517a1eSAndroid Build Coastguard Worker
91*60517a1eSAndroid Build Coastguard WorkerReplace the argument "WORKSPACE" with the file that you are linting.
92*60517a1eSAndroid Build Coastguard Worker
93*60517a1eSAndroid Build Coastguard Worker## Contributor License Agreement
94*60517a1eSAndroid Build Coastguard Worker
95*60517a1eSAndroid Build Coastguard WorkerContributions to this project must be accompanied by a Contributor License
96*60517a1eSAndroid Build Coastguard WorkerAgreement. You (or your employer) retain the copyright to your contribution,
97*60517a1eSAndroid Build Coastguard Workerthis simply gives us permission to use and redistribute your contributions as
98*60517a1eSAndroid Build Coastguard Workerpart of the project. Head over to <https://cla.developers.google.com/> to see
99*60517a1eSAndroid Build Coastguard Workeryour current agreements on file or to sign a new one.
100*60517a1eSAndroid Build Coastguard Worker
101*60517a1eSAndroid Build Coastguard WorkerYou generally only need to submit a CLA once, so if you've already submitted one
102*60517a1eSAndroid Build Coastguard Worker(even if it was for a different project), you probably don't need to do it
103*60517a1eSAndroid Build Coastguard Workeragain.
104*60517a1eSAndroid Build Coastguard Worker
105*60517a1eSAndroid Build Coastguard Worker## Code reviews
106*60517a1eSAndroid Build Coastguard Worker
107*60517a1eSAndroid Build Coastguard WorkerAll submissions, including submissions by project members, require review. We
108*60517a1eSAndroid Build Coastguard Workeruse GitHub pull requests for this purpose. Consult [GitHub Help] for more
109*60517a1eSAndroid Build Coastguard Workerinformation on using pull requests.
110*60517a1eSAndroid Build Coastguard Worker
111*60517a1eSAndroid Build Coastguard Worker[GitHub Help]: https://help.github.com/articles/about-pull-requests/
112*60517a1eSAndroid Build Coastguard Worker
113*60517a1eSAndroid Build Coastguard Worker### Commit messages
114*60517a1eSAndroid Build Coastguard Worker
115*60517a1eSAndroid Build Coastguard WorkerCommit messages (upon merging) and PR messages should follow the [Conventional
116*60517a1eSAndroid Build Coastguard WorkerCommits](https://www.conventionalcommits.org/) style:
117*60517a1eSAndroid Build Coastguard Worker
118*60517a1eSAndroid Build Coastguard Worker```
119*60517a1eSAndroid Build Coastguard Workertype(scope)!: <summary>
120*60517a1eSAndroid Build Coastguard Worker
121*60517a1eSAndroid Build Coastguard Worker<body>
122*60517a1eSAndroid Build Coastguard Worker
123*60517a1eSAndroid Build Coastguard WorkerBREAKING CHANGE: <summary>
124*60517a1eSAndroid Build Coastguard Worker```
125*60517a1eSAndroid Build Coastguard Worker
126*60517a1eSAndroid Build Coastguard WorkerWhere `(scope)` is optional, and `!` is only required if there is a breaking change.
127*60517a1eSAndroid Build Coastguard WorkerIf a breaking change is introduced, then `BREAKING CHANGE:` is required; see
128*60517a1eSAndroid Build Coastguard Workerthe [Breaking Changes](#breaking-changes) section for how to introduce breaking
129*60517a1eSAndroid Build Coastguard Workerchanges.
130*60517a1eSAndroid Build Coastguard Worker
131*60517a1eSAndroid Build Coastguard WorkerCommon `type`s:
132*60517a1eSAndroid Build Coastguard Worker
133*60517a1eSAndroid Build Coastguard Worker* `build:` means it affects the building or development workflow.
134*60517a1eSAndroid Build Coastguard Worker* `docs:` means only documentation is being added, updated, or fixed.
135*60517a1eSAndroid Build Coastguard Worker* `feat:` means a user-visible feature is being added.
136*60517a1eSAndroid Build Coastguard Worker* `fix:` means a user-visible behavior is being fixed.
137*60517a1eSAndroid Build Coastguard Worker* `refactor:` means some sort of code cleanup that doesn't change user-visible behavior.
138*60517a1eSAndroid Build Coastguard Worker* `revert:` means a prior change is being reverted in some way.
139*60517a1eSAndroid Build Coastguard Worker* `test:` means only tests are being added.
140*60517a1eSAndroid Build Coastguard Worker
141*60517a1eSAndroid Build Coastguard WorkerFor the full details of types, see
142*60517a1eSAndroid Build Coastguard Worker[Conventional Commits](https://www.conventionalcommits.org/).
143*60517a1eSAndroid Build Coastguard Worker
144*60517a1eSAndroid Build Coastguard Worker## Generated files
145*60517a1eSAndroid Build Coastguard Worker
146*60517a1eSAndroid Build Coastguard WorkerSome checked-in files are generated and need to be updated when a new PR is
147*60517a1eSAndroid Build Coastguard Workermerged:
148*60517a1eSAndroid Build Coastguard Worker
149*60517a1eSAndroid Build Coastguard Worker* **requirements lock files**: These are usually generated by a
150*60517a1eSAndroid Build Coastguard Worker  `compile_pip_requirements` update target, which is usually in the same directory.
151*60517a1eSAndroid Build Coastguard Worker  e.g. `bazel run //docs:requirements.update`
152*60517a1eSAndroid Build Coastguard Worker
153*60517a1eSAndroid Build Coastguard Worker## Core rules
154*60517a1eSAndroid Build Coastguard Worker
155*60517a1eSAndroid Build Coastguard WorkerThe bulk of this repo is owned and maintained by the Bazel Python community.
156*60517a1eSAndroid Build Coastguard WorkerHowever, since the core Python rules (`py_binary` and friends) are still
157*60517a1eSAndroid Build Coastguard Workerbundled with Bazel itself, the Bazel team retains ownership of their stubs in
158*60517a1eSAndroid Build Coastguard Workerthis repository. This will be the case at least until the Python rules are
159*60517a1eSAndroid Build Coastguard Workerfully migrated to Starlark code.
160*60517a1eSAndroid Build Coastguard Worker
161*60517a1eSAndroid Build Coastguard WorkerPractically, this means that a Bazel team member should approve any PR
162*60517a1eSAndroid Build Coastguard Workerconcerning the core Python logic. This includes everything under the `python/`
163*60517a1eSAndroid Build Coastguard Workerdirectory except for `pip.bzl` and `requirements.txt`.
164*60517a1eSAndroid Build Coastguard Worker
165*60517a1eSAndroid Build Coastguard WorkerIssues should be triaged as follows:
166*60517a1eSAndroid Build Coastguard Worker
167*60517a1eSAndroid Build Coastguard Worker- Anything concerning the way Bazel implements the core Python rules should be
168*60517a1eSAndroid Build Coastguard Worker  filed under [bazelbuild/bazel](https://github.com/bazelbuild/bazel), using
169*60517a1eSAndroid Build Coastguard Worker  the label `team-Rules-python`.
170*60517a1eSAndroid Build Coastguard Worker
171*60517a1eSAndroid Build Coastguard Worker- If the issue specifically concerns the rules_python stubs, it should be filed
172*60517a1eSAndroid Build Coastguard Worker  here in this repository and use the label `core-rules`.
173*60517a1eSAndroid Build Coastguard Worker
174*60517a1eSAndroid Build Coastguard Worker- Anything else, such as feature requests not related to existing core rules
175*60517a1eSAndroid Build Coastguard Worker  functionality, should also be filed in this repository but without the
176*60517a1eSAndroid Build Coastguard Worker  `core-rules` label.
177*60517a1eSAndroid Build Coastguard Worker
178*60517a1eSAndroid Build Coastguard Worker(breaking-changes)=
179*60517a1eSAndroid Build Coastguard Worker## Breaking Changes
180*60517a1eSAndroid Build Coastguard Worker
181*60517a1eSAndroid Build Coastguard WorkerBreaking changes are generally permitted, but we follow a 3-step process for
182*60517a1eSAndroid Build Coastguard Workerintroducing them. The intent behind this process is to balance the difficulty of
183*60517a1eSAndroid Build Coastguard Workerversion upgrades for users, maintaining multiple code paths, and being able to
184*60517a1eSAndroid Build Coastguard Workerintroduce modern functionality.
185*60517a1eSAndroid Build Coastguard Worker
186*60517a1eSAndroid Build Coastguard WorkerThe general process is:
187*60517a1eSAndroid Build Coastguard Worker
188*60517a1eSAndroid Build Coastguard Worker1. In version `N`, introduce the new behavior, but it must be disabled by
189*60517a1eSAndroid Build Coastguard Worker   default. Users can opt into the new functionality when they upgrade to
190*60517a1eSAndroid Build Coastguard Worker   version `N`, which lets them try it and verify functionality.
191*60517a1eSAndroid Build Coastguard Worker2. In version `N+1`, the new behavior can be enabled by default. Users can
192*60517a1eSAndroid Build Coastguard Worker   opt out if necessary, but doing so causes a warning to be issued.
193*60517a1eSAndroid Build Coastguard Worker3. In version `N+2`, the new behavior is always enabled and cannot be opted out
194*60517a1eSAndroid Build Coastguard Worker   of. The API for the control mechanism can be removed in this release.
195*60517a1eSAndroid Build Coastguard Worker
196*60517a1eSAndroid Build Coastguard WorkerNote that the `+1` and `+2` releases are just examples; the steps are not
197*60517a1eSAndroid Build Coastguard Workerrequired to happen in immedially subsequent releases.
198*60517a1eSAndroid Build Coastguard Worker
199*60517a1eSAndroid Build Coastguard Worker
200*60517a1eSAndroid Build Coastguard Worker### How to control breaking changes
201*60517a1eSAndroid Build Coastguard Worker
202*60517a1eSAndroid Build Coastguard WorkerThe details of the control mechanism will depend on the situation. Below is
203*60517a1eSAndroid Build Coastguard Workera summary of some different options.
204*60517a1eSAndroid Build Coastguard Worker
205*60517a1eSAndroid Build Coastguard Worker* Environment variables are best for repository rule behavior. Environment
206*60517a1eSAndroid Build Coastguard Worker  variables can be propagated to rules and macros using the generated
207*60517a1eSAndroid Build Coastguard Worker  `@rules_python_internal//:config.bzl` file.
208*60517a1eSAndroid Build Coastguard Worker* Attributes are applicable to macros and regular rules, especially when the
209*60517a1eSAndroid Build Coastguard Worker  behavior is likely to vary on a per-target basis.
210*60517a1eSAndroid Build Coastguard Worker* [User defined build settings](https://bazel.build/extending/config#user-defined-build-settings)
211*60517a1eSAndroid Build Coastguard Worker  (aka custom build flags) are applicable for rules when the behavior change
212*60517a1eSAndroid Build Coastguard Worker  generally wouldn't vary on a per-target basis. They also have the benefit that
213*60517a1eSAndroid Build Coastguard Worker  an entire code base can have them easily enabled by a bazel command line flag.
214*60517a1eSAndroid Build Coastguard Worker* Allowlists allow a project to centrally control if something is
215*60517a1eSAndroid Build Coastguard Worker  enabled/disabled. Under the hood, they are basically a specialized custom
216*60517a1eSAndroid Build Coastguard Worker  build flag.
217*60517a1eSAndroid Build Coastguard Worker
218*60517a1eSAndroid Build Coastguard WorkerNote that attributes and flags can seamlessly interoperate by having the default
219*60517a1eSAndroid Build Coastguard Workercontrolled by a flag, and an attribute can override the flag setting. This
220*60517a1eSAndroid Build Coastguard Workerallows a project to enable the new behavior by default while they work to fix
221*60517a1eSAndroid Build Coastguard Workerproblematic cases to prepare for the next upgrade.
222*60517a1eSAndroid Build Coastguard Worker
223*60517a1eSAndroid Build Coastguard Worker### What is considered a breaking change?
224*60517a1eSAndroid Build Coastguard Worker
225*60517a1eSAndroid Build Coastguard WorkerPrecisely defining what constitutes a breaking change is hard because it's
226*60517a1eSAndroid Build Coastguard Workereasy for _someone, somewhere_ to depend on _some_ observable behavior, despite
227*60517a1eSAndroid Build Coastguard Workerour best efforts to thoroughly document what is or isn't supported and hiding
228*60517a1eSAndroid Build Coastguard Workerany internal details.
229*60517a1eSAndroid Build Coastguard Worker
230*60517a1eSAndroid Build Coastguard WorkerIn general, something is considered a breaking change when it changes the
231*60517a1eSAndroid Build Coastguard Workerdirect behavior of a supported public API. Simply being able to observe a
232*60517a1eSAndroid Build Coastguard Workerbehavior change doesn't necessarily mean it's a breaking change.
233*60517a1eSAndroid Build Coastguard Worker
234*60517a1eSAndroid Build Coastguard WorkerLong standing undocumented behavior is a large grey area and really depends on
235*60517a1eSAndroid Build Coastguard Workerhow load-bearing it has become and what sort of reasonable expectation of
236*60517a1eSAndroid Build Coastguard Workerbehavior there is.
237*60517a1eSAndroid Build Coastguard Worker
238*60517a1eSAndroid Build Coastguard WorkerHere's some examples of what would or wouldn't be considered a breaking change.
239*60517a1eSAndroid Build Coastguard Worker
240*60517a1eSAndroid Build Coastguard WorkerBreaking changes:
241*60517a1eSAndroid Build Coastguard Worker  * Renaming an function argument for public functions.
242*60517a1eSAndroid Build Coastguard Worker  * Enforcing stricter validation than was previously required when there's a
243*60517a1eSAndroid Build Coastguard Worker    sensible reason users would run afoul of it.
244*60517a1eSAndroid Build Coastguard Worker  * Changing the name of a public rule.
245*60517a1eSAndroid Build Coastguard Worker
246*60517a1eSAndroid Build Coastguard WorkerNot breaking changes:
247*60517a1eSAndroid Build Coastguard Worker  * Upgrading dependencies
248*60517a1eSAndroid Build Coastguard Worker  * Changing internal details, such as renaming an internal file.
249*60517a1eSAndroid Build Coastguard Worker  * Changing a rule to a macro.
250*60517a1eSAndroid Build Coastguard Worker
251*60517a1eSAndroid Build Coastguard Worker## FAQ
252*60517a1eSAndroid Build Coastguard Worker
253*60517a1eSAndroid Build Coastguard Worker### Installation errors when during `git commit`
254*60517a1eSAndroid Build Coastguard Worker
255*60517a1eSAndroid Build Coastguard WorkerIf you did `pre-commit install`, various tools are run when you do `git commit`.
256*60517a1eSAndroid Build Coastguard WorkerThis might show as an error such as:
257*60517a1eSAndroid Build Coastguard Worker
258*60517a1eSAndroid Build Coastguard Worker```
259*60517a1eSAndroid Build Coastguard Worker[INFO] Installing environment for https://github.com/psf/black.
260*60517a1eSAndroid Build Coastguard Worker[INFO] Once installed this environment will be reused.
261*60517a1eSAndroid Build Coastguard Worker[INFO] This may take a few minutes...
262*60517a1eSAndroid Build Coastguard WorkerAn unexpected error has occurred: CalledProcessError: command: ...
263*60517a1eSAndroid Build Coastguard Worker```
264*60517a1eSAndroid Build Coastguard Worker
265*60517a1eSAndroid Build Coastguard WorkerTo fix, you'll need to figure out what command is failing and why. Because these
266*60517a1eSAndroid Build Coastguard Workerare tools that run locally, its likely you'll need to fix something with your
267*60517a1eSAndroid Build Coastguard Workerenvironment or the installation of the tools. For Python tools (e.g. black or
268*60517a1eSAndroid Build Coastguard Workerisort), you can try using a different Python version in your shell by using
269*60517a1eSAndroid Build Coastguard Workertools such as [pyenv](https://github.com/pyenv/pyenv).
270