-
-
Notifications
You must be signed in to change notification settings - Fork 636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[internal] Add [python-setup].experimental_resolves_to_lockfiles
and hook up to ./pants generate-lockfiles
#12703
[internal] Add [python-setup].experimental_resolves_to_lockfiles
and hook up to ./pants generate-lockfiles
#12703
Conversation
…d hook up to `./pants generate-lockfiles` # 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]
"`{ default = '3rdparty/default_lockfile.txt', py2 = '3rdparty/py2.txt' }`.\n\n" | ||
"To generate a lockfile, run `./pants generate-lockfiles --resolve=<name>` or " | ||
"`./pants generate-lockfiles` to generate for all resolves (including tool " | ||
"lockfiles).\n\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I plan to add more info once targets gain the resolve
field. So far I have drafted:
"These resolve names allow certain Python targets to choose which lockfile they "
"should use by setting the field `resolve='<name>'`. If you don't set the field, "
"they will default to your resolve named `default`.\n\n"
"Most projects will only need a single resolve for the whole project - this is a "
"sensible default.\n\n"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm no longer certain we would to force you into setting a default resolve, and for that resolve to be named default
. Per discussion in #12714, we may want a dedicated option to set the default resolve, if any.
This PR intentionally punts on committing to default
being a special thing. Here, it's just like any other name.
@@ -127,6 +127,19 @@ def register_options(cls, register): | |||
"Mutually exclusive with `[python-setup].requirement_constraints`." | |||
), | |||
) | |||
register( | |||
"--experimental-resolves-to-lockfiles", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I debated calling this only --experimental-resolves
. But I think there's value in including the word "lockfile" in the name. "Resolve" on its own is vague to most users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, just resolves
. The name alone will never be enough to skip reading the help, and the help should say that bit very clearly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you okay with waiting a little longer for this rename? #12714 is making me think we might want >1 resolve related option, e.g. default_resolve
to be a dedicated option. Unclear.
I'm open to renaming but would like to keep this for now while iterating. It helps me my mental model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all seems fine; some small clarity things, but no red flags
@@ -127,6 +127,19 @@ def register_options(cls, register): | |||
"Mutually exclusive with `[python-setup].requirement_constraints`." | |||
), | |||
) | |||
register( | |||
"--experimental-resolves-to-lockfiles", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, just resolves
. The name alone will never be enough to skip reading the help, and the help should say that bit very clearly.
The followup PR shows that we only need the resolve name in this helper function, not the lockfile path. This simplifies the code # 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]
Part of #11165 and builds off of #12703. Rather than having a single option `[python-setup].experimental_lockfile`, users set `[python-setup].experimental_resolves_to_lockfiles` to define 0-n "named resolves" that associate a lockfile with a name: ```toml [python-setup] experimental_resolves_to_lockfiles = { lock1 = "lock1.txt", lock2 = "lock2.txt" } ``` Then, individual `pex_binary` targets can specify which resolve to use: ```python pex_binary(name="reversion", entry_point="reversion.py", experimental_resolve="lock1") ``` In a followup, we'll add a mechanism to set the default resolve. Users can generate that lockfile with `./pants generate-lockfiles` (all resolves) or `./pants generate-lockfiles --resolve=<name>`: ``` ❯ ./pants generate-lockfiles --resolve=lock1 --resolve=lock2 15:55:56.60 [INFO] Completed: Generate lockfile for lock1 15:55:56.61 [INFO] Completed: Generate lockfile for lock2 15:55:57.02 [INFO] Wrote lockfile for the resolve `lock1` to lock1.txt 15:55:57.02 [INFO] Wrote lockfile for the resolve `lock2` to lock2.txt ``` Then, it will be consumed with `./pants package` and `./pants run`. Pants will extract the proper subset from that lockfile, meaning that the lockfile can safely be a superset of what is used for the particular build. ``` ❯ ./pants package build-support/bin: ... 15:56:33.87 [INFO] Completed: Installing lock1.txt for the resolve `lock1` 15:56:34.39 [INFO] Completed: Installing lock2.txt for the resolve `lock2` 15:56:34.48 [INFO] Completed: Extracting 1 requirement to build build-support.bin/generate_user_list.pex from lock1_lockfile.pex: pystache==0.5.4 ... ``` If the lockfile is incompatible, we will (soon) warn or error with instructions to either use a new resolve or regenerate the lockfile. In followups, this field will be hooked up to other targets like `python_awslambda` and `python_tests`. We will likely also add a new field `compatible_resolves` to `python_library`, per #12714, which is a list of resolves. "Root targets" like `python_tests` and `pex_binary` will validate that all their dependencies are compatible. When you operate directly on a `python_library` target, like running MyPy on it, we will choose any of the possible resolves. You will be able to set your own default for this field. [ci skip-rust] [ci skip-build-wheels]
This option does not currently do anything..but it will be where users define their "named resolve":
Then, users can set
resolve="data-science"
on targets.They can generate the lockfile with
./pants generate-lockfiles --resolve='data-science'
.--
This PR is prework to add the option and wire it up to
./pants generate-lockfiles
, including checking that the named resolve is not ambiguous with a tool's resolve name (its option scope).[ci skip-rust]
[ci skip-build-wheels]