Deprecate `python_requirements` and `poetry_requirements` using old macro in favor of target generation #14075

Eric-Arellano · 2022-01-05T01:41:23Z

Benefits of switching to target generation

Closes Macros should be able to consume the engine to request files/processes #7022.
Target generators show up in ./pants help.
Consistent mechanism for target generation, as explained at https://www.pantsbuild.org/docs/targets.

Deprecation plan

This PR adds a new global option --use-deprecated-python-macros with a default of True. When True, parser.py is taught to load the old macros; else it loads the new targets.

The PR deprecates the default of --use-deprecated-python-macros so that it will default to False in Pants 2.11. Then, we'll deprecate the option outright and remove it in Pants 2.12, meaning the macros will be removed then too.

How users upgrade

❯ ./pants
17:06:21.30 [WARN] DEPRECATED: the option `--use-deprecated-python-macros` defaulting to true will be removed in version 2.11.0.dev0.

In Pants 2.11, the default for the global option `--use-deprecated-python-macros` will change to false.

To fix this deprecation, explicitly set `use_deprecated_python_macros = true` in the `[GLOBAL]` section of `pants.toml`. Or, When you are ready to upgrade to the improved target generation mechanism, follow these steps:

  1. Run `./pants update-build-files --fix-python-macros`
  2. Check the logs for an ERROR log to see if you have to manually add `name=` anywhere.
  3. Set `use_deprecated_python_macros = false` in `[GLOBAL]` in pants.toml.

(Why upgrade from the old macro mechanism to target generation? Among other benefits, it makes sure that the Pants daemon is properly invalidated when you change `requirements.txt` and `pyproject.toml`.)

stuhood

Thanks!

src/python/pants/backend/python/macros/common_fields.py

stuhood · 2022-01-05T18:50:37Z

src/python/pants/option/global_options.py

@@ -1268,6 +1268,24 @@ def register_bootstrap_options(cls, register):
            "may not be enabled.",
        )

+        register(
+            "--use-deprecated-python-macros",


Since this flag will land as un-deprecated, it's starting out misnamed. Maybe invert to --use-new-python-macros?

Eh I don't think it matters much. But in my head, it makes sense to see this in pants.toml:

[GLOBAL] use_deprecated_python_macros = false

It makes clear that you're opting out of deprecated functionality. Technically, the macros aren't yet formally deprecated. But in spirit, they are deprecated. It's only a logistical reason why we can't formally deprecate them more quickly.

Sounds good. What does the alternative look like..

[GLOBAL] use_new_python_macros = true

Makes it clear that you're opting in to use the new macros before that is the default thing.

Hmm.. nah, that option name is more wrong, the new thing is not macros at all..

…f `sources: list[str]` (#14082) To minimize behavior changes while implementing #14075, we want to make sure that `--changed-since=HEAD --changed-dependees=direct` will still claim that all generated `python_requirement` targets are impacted if the `requirements.txt` changed. This would happen naturally if a generated target depended on its target generator: #13118. But that will require a lot more design work and is a big change we're not ready to make. So we can work around this by still having `_python_requirements_file` even with target generators. But before doing that, we should change to `source: str` to actually represent the expected API of the target. Because this is a private API, we make a breaking change. [ci skip-rust] [ci skip-build-wheels]

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

[ci skip-rust] [ci skip-build-wheels]

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust]

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Eric-Arellano · 2022-01-13T23:13:32Z

Bump @stuhood, this needs a re-review as it was approved while still a WIP.

stuhood

Thanks a lot!

stuhood · 2022-01-13T23:29:18Z

pants.toml


+use_deprecated_python_macros = false


This is still weird to me (seems like a double-negative), but oh well!

…enerators [ci skip-rust] [ci skip-build-wheels]

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

stuhood · 2022-01-20T19:44:33Z

src/python/pants/option/global_options.py

+            "2.11.0.dev0",
+            "the option `--use-deprecated-python-macros` defaulting to true",


One thing that would be good to fix before 2.10.0 is that this warning currently triggers in repos without the python backend enabled.

Eric-Arellano mentioned this pull request Jan 5, 2022

_python_requirements_file target uses source: str field instead of sources: list[str] #14082

Merged

stuhood approved these changes Jan 5, 2022

View reviewed changes

Eric-Arellano added 2 commits January 5, 2022 13:22

Add --use-deprecated-python-macros

c368b09

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Add pipenv_requirements.py

87a7092

[ci skip-rust] [ci skip-build-wheels]

Eric-Arellano force-pushed the python-reqs-generators branch from a837300 to 66ba6f9 Compare January 5, 2022 20:22

Eric-Arellano added 5 commits January 5, 2022 13:27

Add python_requirements target generator

3c8add9

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Move Poetry helpers into poetry_requirements.py

1272709

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Add poetry_requirements

d80d8f3

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Add the deprecation and --update-build-files-python-macros

204f8c5

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust]

Switch Pants to target generators

235f591

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust]

Eric-Arellano force-pushed the python-reqs-generators branch from 66ba6f9 to 235f591 Compare January 6, 2022 00:12

Register the rules, oops!

9904503

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Eric-Arellano changed the title ~~[wip] Deprecate python_requirements and poetry_requirements using old macro in favor of target generation~~ Deprecate python_requirements and poetry_requirements using old macro in favor of target generation Jan 6, 2022

Stu review feedback about help messages

2b4dcad

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Eric-Arellano marked this pull request as ready for review January 6, 2022 00:29

Eric-Arellano requested a review from stuhood January 6, 2022 00:30

Fix tests

0d48329

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

stuhood approved these changes Jan 13, 2022

View reviewed changes

Merge branch 'main' of github.com:pantsbuild/pants into python-reqs-g…

33eb52a

…enerators [ci skip-rust] [ci skip-build-wheels]

Eric-Arellano enabled auto-merge (squash) January 14, 2022 00:54

Bump timeout for Scala Protobuf test

ad08c88

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Eric-Arellano merged commit 9ad9204 into pantsbuild:main Jan 14, 2022

Eric-Arellano deleted the python-reqs-generators branch January 14, 2022 18:59

stuhood reviewed Jan 20, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate `python_requirements` and `poetry_requirements` using old macro in favor of target generation #14075

Deprecate `python_requirements` and `poetry_requirements` using old macro in favor of target generation #14075

Eric-Arellano commented Jan 5, 2022 •

edited

stuhood left a comment

stuhood Jan 5, 2022

Eric-Arellano Jan 5, 2022

kaos Jan 5, 2022

Eric-Arellano commented Jan 13, 2022

stuhood left a comment

stuhood Jan 13, 2022

stuhood Jan 20, 2022

		"2.11.0.dev0",
		"the option `--use-deprecated-python-macros` defaulting to true",

Deprecate python_requirements and poetry_requirements using old macro in favor of target generation #14075

Deprecate python_requirements and poetry_requirements using old macro in favor of target generation #14075

Conversation

Eric-Arellano commented Jan 5, 2022 • edited

Benefits of switching to target generation

Deprecation plan

How users upgrade

stuhood left a comment

Choose a reason for hiding this comment

stuhood Jan 5, 2022

Choose a reason for hiding this comment

Eric-Arellano Jan 5, 2022

Choose a reason for hiding this comment

kaos Jan 5, 2022

Choose a reason for hiding this comment

Eric-Arellano commented Jan 13, 2022

stuhood left a comment

Choose a reason for hiding this comment

stuhood Jan 13, 2022

Choose a reason for hiding this comment

stuhood Jan 20, 2022

Choose a reason for hiding this comment

Deprecate `python_requirements` and `poetry_requirements` using old macro in favor of target generation #14075

Deprecate `python_requirements` and `poetry_requirements` using old macro in favor of target generation #14075

Eric-Arellano commented Jan 5, 2022 •

edited