Skip to content
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

Add [python].enable_resolves and [python].experimental_default_resolve #13967

Merged
merged 1 commit into from
Dec 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/python/pants/backend/python/goals/lockfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,9 @@ class _UserLockfileRequests(Collection[PythonLockfileRequest]):
async def setup_user_lockfile_requests(
requested: _SpecifiedUserResolves, all_targets: AllTargets, python_setup: PythonSetup
) -> _UserLockfileRequests:
if not python_setup.enable_resolves:
return _UserLockfileRequests()

# First, associate all resolves with their consumers.
resolves_to_roots = defaultdict(list)
for tgt in all_targets:
Expand Down
54 changes: 45 additions & 9 deletions src/python/pants/backend/python/subsystems/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ def register_options(cls, register):
"See https://pip.pypa.io/en/stable/user_guide/#constraints-files for more "
"information on the format of constraint files and how constraints are applied in "
"Pex and pip.\n\n"
"Mutually exclusive with `[python].experimental_lockfile`."
"Mutually exclusive with `[python].experimental_lockfile` and "
"`[python].enable_resolves`."
),
)
register(
Expand All @@ -113,29 +114,56 @@ def register_options(cls, register):
# TODO(#11719): Switch this to a file_option once env vars can unset a value.
type=str,
metavar="<file>",
mutually_exclusive_group="constraints",
mutually_exclusive_group="lockfile",
help=(
"The lockfile to use when resolving requirements for your own code (vs. tools you "
"run).\n\n"
"This is highly experimental and will change, including adding support for "
"multiple lockfiles. This option's behavior may change without the normal "
"deprecation cycle.\n\n"
"This is highly experimental and will be replaced by `[python].enable_resolves`.\n\n"
"To generate a lockfile, activate the backend `pants.backend.experimental.python`"
"and run `./pants generate-user-lockfile ::`.\n\n"
"Mutually exclusive with `[python].requirement_constraints`."
"Mutually exclusive with `[python].requirement_constraints` and "
"`[python].enable_resolves`."
),
)
register(
"--enable-resolves",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be ambiguous to call this "require-lockfile" or something? It's not quite clear what "enabling resolves" would mean, and most users will only be enabling lockfiles, rather than enabling multiple resolves.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be a little confusing because we've in the past called "constraints" a "lockfile", so it's an overloaded term. And we have --experimental-lockfile which we know a few users are using.

I went with --enable-resolves because the overall new feature can be called "resolves" to differentiate from the status quo. Even if you only have a single resolve, you still have fields like compatible_resolves, resolve, and --default-resolve. "resolve" is the main way we refer to this feature.

advanced=True,
type=bool,
default=False,
mutually_exclusive_group="lockfile",
help=(
"Set to true to enable the multiple resolves mechanism. See "
"`[python].experimental_resolves` for an explanation of this feature.\n\n"
"Mutually exclusive with `[python].experimental_lockfile` and "
"`[python].requirement_constraints`."
),
)
register(
"--experimental-resolves",
advanced=True,
type=dict,
default={"python-default": "3rdparty/python/default_lock.txt"},
help=(
"A mapping of logical names to lockfile paths used in your project, e.g. "
"`{ default = '3rdparty/default_lockfile.txt', py2 = '3rdparty/py2.txt' }`.\n\n"
"A mapping of logical names to lockfile paths used in your project.\n\n"
# TODO(#12314): explain how this feature works.
"To generate a lockfile, run `./pants generate-lockfiles --resolve=<name>` or "
"`./pants generate-lockfiles` to generate for all resolves (including tool "
"lockfiles).\n\n"
"This is highly experimental and will likely change."
"Only applies if `[python].enable_resolves` is true.\n\n"
"This option is experimental and may change without the normal deprecation policy."
),
)
register(
"--experimental-default-resolve",
advanced=True,
type=str,
default="python-default",
help=(
"The default value used for the `experimental_resolve` and "
"`experimental_compatible_resolves` fields.\n\n"
"Only applies if `[python].enable_resolves` is true.\n\n"
"The name must be defined as a resolve in `[python].experimental_resolves`.\n\n"
"This option is experimental and may change without the normal deprecation policy."
),
)
register(
Expand Down Expand Up @@ -271,10 +299,18 @@ def requirement_constraints(self) -> str | None:
def lockfile(self) -> str | None:
return cast("str | None", self.options.experimental_lockfile)

@property
def enable_resolves(self) -> bool:
return cast(bool, self.options.enable_resolves)

@property
def resolves(self) -> dict[str, str]:
return cast("dict[str, str]", self.options.experimental_resolves)

@property
def default_resolve(self) -> str:
return cast(str, self.options.experimental_default_resolve)

@property
def invalid_lockfile_behavior(self) -> InvalidLockfileBehavior:
return cast(InvalidLockfileBehavior, self.options.invalid_lockfile_behavior)
Expand Down
24 changes: 15 additions & 9 deletions src/python/pants/backend/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,23 @@ def __init__(

class PythonResolveField(StringField, AsyncFieldMixin):
alias = "experimental_resolve"
# TODO(#12314): Figure out how to model the default and disabling lockfile, e.g. if we
# hardcode to `default` or let the user set it.
help = (
"The resolve from `[python].experimental_resolves` to use, if any.\n\n"
"This field is highly experimental and may change without the normal deprecation policy."
"The resolve from `[python].experimental_resolves` to use.\n\n"
"If not defined, will default to `[python].default_resolve`.\n\n"
"Only applies if `[python].enable_resolves` is true.\n\n"
"This field is experimental and may change without the normal deprecation policy."
# TODO: Document expectations for dependencies once we validate that.
)

def value_or_default(self, python_setup: PythonSetup) -> str:
return self.value or python_setup.default_resolve

def validate(self, python_setup: PythonSetup) -> None:
"""Check that the resolve name is recognized."""
if not self.value:
return None
if self.value not in python_setup.resolves:
resolve = self.value_or_default(python_setup)
if resolve not in python_setup.resolves:
raise UnrecognizedResolveNamesError(
[self.value],
[resolve],
python_setup.resolves.keys(),
description_of_origin=f"the field `{self.alias}` in the target {self.address}",
)
Expand All @@ -148,8 +151,11 @@ def resolve_and_lockfile(self, python_setup: PythonSetup) -> tuple[str, str] | N

Error if the resolve name is invalid.
"""
if not python_setup.enable_resolves:
return None
self.validate(python_setup)
return (self.value, python_setup.resolves[self.value]) if self.value is not None else None
resolve = self.value_or_default(python_setup)
return (resolve, python_setup.resolves[resolve])


# -----------------------------------------------------------------------------------------------
Expand Down
6 changes: 2 additions & 4 deletions src/python/pants/jvm/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ class JvmCompatibleResolvesField(StringSequenceField):
alias = "compatible_resolves"
required = False
help = (
"The set of resolve names that this target is compatible with.\n\n"
"The set of resolves from `[jvm].resolves` that this target is compatible with.\n\n"
"If not defined, will default to `[jvm].default_resolve`.\n\n"
"Each name must be defined as a resolve in `[jvm].resolves`.\n\n"
# TODO: Document expectations for dependencies once we validate that.
)

Expand All @@ -39,9 +38,8 @@ class JvmResolveField(StringField):
alias = "resolve"
required = False
help = (
"The name of the resolve to use when building this target.\n\n"
"The resolve from `[jvm].resolves` to use when compiling this target.\n\n"
"If not defined, will default to `[jvm].default_resolve`.\n\n"
"The name must be defined as a resolve in `[jvm].resolves`."
# TODO: Document expectations for dependencies once we validate that.
)

Expand Down