Use black (via pantsbuild)

[cognifloyd]

Background

This is another part of introducing pants, as discussed in the TSC Meetings on 12 July 2022, 02 Aug 2022, 06 Sept 2022, and 04 Oct 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:

• Initialize pantsbuild #5713
• Configure pantsbuild source roots #5724
• Enable pantsbuild language backends #5725
• Adjust .gitignore to work better with pantsbuild #5726
• Add GHA workflow to validate pants BUILD files #5732
• Configure pants_ignore and bump pants to v2.14.0rc1 #5733
• Explicitly disable pantsbuild opt-in anonymous telemetry #5737
• Create pantsbuild BUILD files with ./pants tailor :: #5738
• Add GHA workflow to lint with pantsbuild #5758
• Use shellcheck (via pantsbuild) #5751
• Use flake8 (via pantsbuild) #5776
• Use bandit (via pantsbuild) #5777
• Use regex-lint to provide circular dependency checking in pants #5778

Overview of this PR

This configures pants to use black when running the fmt and lint goals.

By default, pants uses black==22.6.0, but I told it to use the version we have it pinned to: black==22.3.0:

st2/test-requirements.txt

Line 8 in 83904f6

black==22.3.0

Pants uses a lockfile for each tool it uses to ensure we get consistent results. Since I changed [black].version in pants.toml, pants errors, saying that the lockfile (in this case the <default> lockfile distributed with pants) is out-of-date. Note how the message is very helpful in explaining exactly what has to happen to fix this:

$ ./pants lint ::
16:32:39.79 [INFO] Completed: Lint with Shellcheck - shellcheck succeeded.
16:32:43.17 [ERROR] 1 Exception encountered:

  InvalidLockfileError: You are using the `<default>` lockfile provided by Pants to install the tool `black`, but it is not compatible with your configuration:

- You have set different requirements than those used to generate the lockfile. You can fix this by updating `[black].version` and/or `[black].extra_requirements`, or by using a new custom lockfile.
In the input requirements, but not in the lockfile: ['black==22.3.0']
In the lockfile, but not in the input requirements: ['black==22.6.0']

To generate a custom lockfile based on your current configuration, set `[black].lockfile` to where you want to create the lockfile, then run `./pants generate-lockfiles --resolve=black`.

There will be a variety of lockfiles, so I created a directory to keep them all together somewhere other than the root directory of the project. I used lockfiles/, but we could also use something like build-support/python/ (other pants-based projects use something like this). I figure this is good enough for now. We can choose a different directory later, and reorganize the lockfiles, and possibly files like those under lint-configs/, into something that feels even cleaner.

NOTE: it is not safe to manually change the lockfiles. If we need to change any dependencies (including transitive deps), we need to use pants to regenerate the applicable lockfiles. Therefore, 442 lines of this change are auto-generated - you can review them for sanity, but we should not change them manually.

I also updated to the latest pants 2.14.0rc3, the latest 2.14 release candidate.

Relevant Pants documentation

• ./pants fmt goal
• ./pants generate-lockfiles goal
• ./pants lint goal
• Python Overview
  • Python Linters and Formatters
  • black backend
• source targets:
  • python_sources
  • python_source
• Video about how pants facilitates "Incrementally Adopting New Tools"

Things you can do with pantsbuild

I needed to generate the lockfiles/black lockfile, which I did with this:

$ ./pants generate-lockfiles --resolve=black  
16:34:29.04 [INFO] Completed: Generate lockfile for black
16:34:29.06 [INFO] Wrote lockfile for the resolve `black` to lockfiles/black

This is the first PR that adds a fmt backend. So, now you can run ./pants fmt :: to reformat files, or ./pants lint :: to see if black would make changes on fmt (the GHA Lint workflow runs ./pants lint ::). Note how pants says "black made no changes" more than once - that is because it runs tools concurrently wherever possible.

$ ./pants fmt ::
17:24:15.18 [INFO] Completed: Format with Black - black made no changes.
17:24:15.18 [INFO] Completed: Format with Black - black made no changes.
17:24:15.18 [INFO] Completed: Format with Black - black made no changes.
17:24:15.18 [INFO] Completed: Format with Black - black made no changes.
17:24:15.18 [INFO] Completed: Format with Black - black made no changes.
17:24:15.18 [INFO] Completed: Format with Black - black made no changes.
17:24:15.18 [INFO] Completed: Format with Black - black made no changes.
17:24:15.18 [INFO] Completed: Format with Black - black made no changes.

✓ black made no changes.

$ ./pants lint ::
17:24:20.84 [INFO] Completed: Format with Black - black made no changes.
17:24:20.84 [INFO] Completed: Format with Black - black made no changes.
17:24:20.84 [INFO] Completed: Format with Black - black made no changes.
17:24:20.84 [INFO] Completed: Format with Black - black made no changes.
17:24:20.84 [INFO] Completed: Format with Black - black made no changes.
17:24:20.84 [INFO] Completed: Format with Black - black made no changes.
17:24:20.84 [INFO] Completed: Lint with Shellcheck - shellcheck succeeded.
17:24:20.84 [INFO] Completed: Format with Black - black made no changes.
17:24:20.84 [INFO] Completed: Format with Black - black made no changes.

✓ black succeeded.
✓ shellcheck succeeded.

[cognifloyd]

This is in a git submodule, so we don't want black to try to change anything across the git boundaries.

[Eric-Arellano]

Maybe worth a comment?

[cognifloyd]

There's a comment mentioning the git submodule at the top of this file:

st2/st2tests/st2tests/fixtures/packs/BUILD

Lines 1 to 2 in 3e5d8df

	# The files in test_content_version* targets are in ./test_content_version
	# which is a git submodule.

I suppose we can extend that comment if needed, because there are 3 targets that cover that git submodule. The shell_sources target has skip_shellcheck=True for similar reasons: don't complain about things across the git boundaries.

[rush-skills]

Can have a comment here to explain this one off disable

[cognifloyd]

Oops. I missed this before merging. So, I added a comment in #5777: f443f39

[Eric-Arellano]

Maybe worth a comment?

[amanda11]

LGTM - but I think the vote is looking like preference for .lock extension on lockfiles.

[arm4b]

👍
Lock files are great for reproducible builds, - something we're missing at this moment.

[Eric-Arellano]

Lock files are great for reproducible builds

Helps with supply chain security too :)