Create pantsbuild `BUILD` files with `./pants tailor ::` #5738

cognifloyd · 2022-09-20T20:40:39Z

Background

This is another part of introducing pants, as discussed in the TSC Meetings on 12 July 2022, 02 Aug 2022 and 06 Sept 2022. Pants has fine-grained per-file caching of results for lint, fmt (like black), test, etc. It also has lockfiles that work well for monorepos that have multiple python packages. With these lockfiles CI should not break when any of our dependencies or our transitive dependencies release new versions, because CI will continue to use the locked version until we explicitly relock with updates.

To keep PRs as manageable/reviewable as possible, introducing pants will take a series of PRs. I do not know yet how many PRs; I will break this up into logical steps with these goals:

introduce pants to the st2 repo, and
teach some (but not all) TSC members about pants step-by-step.

Other pants PRs include:

Overview of this PR

This PR does 3 things:

Run ./pants tailor :: (step 5 of the initial config docs)
Enable the workflow added in Add GHA workflow to validate pants BUILD files #5732
Fix an edge case with our BUILD metadata to work around our use of git submodules (the test_content_version fixture pack)

I will describe the generated BUILD file contents after I have briefly discussed each of them (the 3 things this PR does).

`./pants tailor ::`

All but one of the BUILD files in this PR were created by running ./pants tailor :: which is step 5 of the initial config docs.

As described in #5732, BUILD files are an important part of helping pants understand our codebase. As the docs say:

BUILD files provide metadata about your code (the timeout of a test, any dependencies which cannot be inferred, etc). BUILD files are typically located in the same directory as the code they describe. Unlike many other systems, Pants BUILD files are usually very succinct, as most metadata is either inferred from static analysis, assumed from sensible defaults, or generated for you.

In follow-up PRs, we will customize the BUILD metadata further.

GHA workflow

This PR also enables the GHA workflow added in #5732 (it also fixes a typo in the workflow. oops). This workflow will ensure that we have all the BUILD files that pants needs. This is very quick, and serves as an indicator on PRs that people might need to run ./pants tailor ::.

git submodule: `test_content_version` fixture pack

./pants tailor tried to add BUILD files in the git submodule. But, we cannot guarantee that git submodule is checked out when we run ./pants. So, I added some targets in the parent directory's BUILD file that covers everything in the git submodule. This way, even if the submodule is not checked out, the complete set of metadata is available for pants.

I wrote a plugin for pants that makes the developer UX even better. It checks to make sure the git submodule is checked out, and if not, it provides an error message with instructions on how to fix it. I will submit that in a later PR.

`BUILD` file metadata

BUILD files contain metadata that guide pants through our source code. That metadata consists of "targets" with "fields".

From the docs: https://www.pantsbuild.org/docs/targets

Targets are an addressable set of metadata describing your code.

Each target type has different fields, or individual metadata values.

One of the nice things about BUILD files, is that they are essentially just python files. In fact, ./pants update-build-files :: actually uses black to reformat them!

So, when looking at BUILD files, defining a target looks like a function call, and the fields are args and/or kwargs in that function call.

There are some restrictions on what python we can use in BUILD files. For example: there are no imports (Pants actually adds all of the required symbols based on which backends are enabled in pants.toml). When we need to add custom targets, we can do that with macros and pants plugins, both of which are also written in python. I have written both macros and pants plugins to add in follow-up PRs.

Targets

These are the targets that ./pants tailor :: added in this PR (linked to the relevant docs page):

python_requirements (see also: python_requirement)
python_sources (see also: python_source)
python_test_utils
python_tests (see also: python_test)
shell_sources (see also: shell_source)

I also added some of these:

resources (see also: shell_source)

Tailor cannot add resources or files targets, so those will be added manually in follow-up PRs.

cognifloyd · 2022-09-20T22:20:22Z

.github/workflows/pants.yaml

@@ -42,11 +40,12 @@ jobs:
        #   Otherwise, we may need an additional workflow or job to delete old caches
        #   if they are not expiring fast enough, and we hit the GHA 10GB per repo max.
        with:
+          base-branch: master


The action defaults to using main as the base-branch (which I forgot, oops).

cognifloyd · 2022-09-20T22:20:32Z

.github/workflows/pants.yaml

          # To ignore a bad cache, bump the cache* integer.
          gha-cache-key: cache0-BUILD
          # This hash should include all of our lockfiles so that the pip/pex caches
          # get invalidated on any transitive dependency update.
-          named-caches-hash: ${{ hashFiles('requirements.txt' }}
+          named-caches-hash: ${{ hashFiles('requirements.txt') }}


cognifloyd · 2022-09-20T22:22:08Z

BUILD

+python_test_utils(
+    name="test_utils0",
+)


This is for conftest.py. I will rename the targets in this BUILD file (and others) in a follow-up PR.

cognifloyd · 2022-09-20T22:23:57Z

st2tests/st2tests/fixtures/packs/BUILD

+# The files in test_content_version* targets are in ./test_content_version
+# which is a git submodule.
+# The test_content_version* targets are dependencies of ./test_content_version_fixture
+
+resources(
+    name="test_content_version_metadata",
+    sources=[
+        "test_content_version/pack.yaml",
+        "test_content_version/**/*.yaml",
+        "test_content_version/icon.png",
+        "test_content_version/requirements.txt",
+    ],
+)
+
+shell_sources(
+    name="test_content_version_shell",
+    sources=[
+        "test_content_version/**/*.sh",
+    ],
+)
+
+python_sources(
+    name="test_content_version",
+    dependencies=[
+        ":test_content_version_metadata",
+        ":test_content_version_shell",
+    ],
+    sources=[
+        "test_content_version/**/*.py",
+    ],
+)


These are the targets that cover the test_content_version git submodule.

cognifloyd · 2022-09-20T22:24:58Z

st2tests/st2tests/fixtures/packs/test_content_version_fixture/BUILD

+python_sources(
+    dependencies=["st2tests/st2tests/fixtures/packs:test_content_version"],
+)


This manually declared dependency tells pants that fixture.py in this directory actually needs the files from test_content_version (the git submodule).

cognifloyd · 2022-09-20T22:33:15Z

st2api/tests/unit/controllers/v1/test_packs_views.py

@@ -133,7 +133,7 @@ def test_get_pack_files_and_pack_file_ref_doesnt_equal_pack_name(self):
        # Ref is not equal to the name, controller should still work
        resp = self.app.get("/v1/packs/views/files/dummy_pack_16")
        self.assertEqual(resp.status_int, http_client.OK)
-        self.assertEqual(len(resp.json), 3)
+        self.assertEqual(len(resp.json), 4)


I had to bump the file count in the pack because this PR added a BUILD file in the pack.

Eric-Arellano

FYI from our docs about why we recommend doing things this way with one BUILD file per directory:

We've found that it usually scales much better to use a single BUILD file per directory. Even if you start with using the defaults for everything, projects usually need to change some metadata over time, like adding a timeout to a test file or adding dependencies on resources.

It's useful for metadata to be as fine-grained as feasible, such as by using the overrides field to only change the files you need to. Fine-grained metadata is key to having smaller cache keys (resulting in more cache hits), and allows you to more accurately reflect the status of your project. We have found that using one BUILD file per directory encourages fine-grained metadata by defining the metadata adjacent to where the code lives.

rm st2tests/st2tests/fixtures/packs/test_content_version/**/BUILD A follow up will handle content in the submodule.

winem

Another great job!

cognifloyd added infrastructure: ci/cd maintenance pantsbuild labels Sep 20, 2022

cognifloyd added this to the pants milestone Sep 20, 2022

cognifloyd self-assigned this Sep 20, 2022

pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Sep 20, 2022

cognifloyd mentioned this pull request Sep 20, 2022

Explicitly disable pantsbuild opt-in anonymous telemetry #5737

Merged

cognifloyd force-pushed the pants-tailor branch 2 times, most recently from 019cbdc to 0169ed1 Compare September 20, 2022 21:27

cognifloyd commented Sep 20, 2022

View reviewed changes

cognifloyd force-pushed the pants-tailor branch from 63d61f6 to 7bf1c85 Compare September 20, 2022 22:40

cognifloyd marked this pull request as ready for review September 21, 2022 05:24

cognifloyd requested review from rush-skills, amanda11, arm4b, nzlosh and a team September 21, 2022 05:24

cognifloyd enabled auto-merge September 21, 2022 05:25

Eric-Arellano approved these changes Sep 21, 2022

View reviewed changes

cognifloyd added 6 commits September 21, 2022 09:34

./pants tailor ::

5a9b2ce

rm st2tests/st2tests/fixtures/packs/test_content_version/**/BUILD A follow up will handle content in the submodule.

update changelog entry

5ca8ee0

enable pants metadata validation GHA workflow

989e4ec

Add BUILD targets for test_content_version fixture

c17f692

pants tailor - ignore recommended python_requirements targets

3520186

Account for BUILD files in tests (count and file list)

080d40a

cognifloyd force-pushed the pants-tailor branch from 7bf1c85 to 080d40a Compare September 21, 2022 15:35

cognifloyd disabled auto-merge September 21, 2022 16:09

amanda11 approved these changes Sep 26, 2022

View reviewed changes

winem approved these changes Sep 28, 2022

View reviewed changes

cognifloyd merged commit eeddc49 into master Oct 3, 2022

cognifloyd deleted the pants-tailor branch October 3, 2022 17:24

This was referenced Oct 5, 2022

Use shellcheck (via pantsbuild) #5751

Merged

Add GHA workflow to lint with pantsbuild #5758

Merged

This was referenced Oct 13, 2022

Use black (via pantsbuild) #5774

Merged

Use flake8 (via pantsbuild) #5776

Merged

Use bandit (via pantsbuild) #5777

Merged

Use regex-lint to provide circular dependency checking in pants #5778

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create pantsbuild `BUILD` files with `./pants tailor ::` #5738

Create pantsbuild `BUILD` files with `./pants tailor ::` #5738

cognifloyd commented Sep 20, 2022 •

edited

Loading

cognifloyd Sep 20, 2022

cognifloyd Sep 20, 2022

cognifloyd Sep 20, 2022

cognifloyd Sep 20, 2022

cognifloyd Sep 20, 2022

cognifloyd Sep 20, 2022

Eric-Arellano left a comment

winem left a comment

Create pantsbuild BUILD files with ./pants tailor :: #5738

Create pantsbuild BUILD files with ./pants tailor :: #5738

Conversation

cognifloyd commented Sep 20, 2022 • edited Loading

Background

Overview of this PR

./pants tailor ::

GHA workflow

git submodule: test_content_version fixture pack

BUILD file metadata

Targets

cognifloyd Sep 20, 2022

Choose a reason for hiding this comment

cognifloyd Sep 20, 2022

Choose a reason for hiding this comment

cognifloyd Sep 20, 2022

Choose a reason for hiding this comment

cognifloyd Sep 20, 2022

Choose a reason for hiding this comment

cognifloyd Sep 20, 2022

Choose a reason for hiding this comment

cognifloyd Sep 20, 2022

Choose a reason for hiding this comment

Eric-Arellano left a comment

Choose a reason for hiding this comment

winem left a comment

Choose a reason for hiding this comment

Create pantsbuild `BUILD` files with `./pants tailor ::` #5738

Create pantsbuild `BUILD` files with `./pants tailor ::` #5738

cognifloyd commented Sep 20, 2022 •

edited

Loading

`./pants tailor ::`

git submodule: `test_content_version` fixture pack

`BUILD` file metadata