From 86bc802cdcc4e7449b4c4ce4ea74190ea6a40133 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Thu, 26 Aug 2021 16:37:24 -0700 Subject: [PATCH] [internal] Rename `./pants lock` to `./pants generate-user-lockfile` and `./pants tool-lock` to `./pants generate-lockfiles` (#12641) We currently have separate goals for user lockfiles vs tool lockfiles because the implementations are different right now, e.g. specs being different. It's easier to keep separate while iterating. We will unify them, but are not yet ready. We want to release tool lockfiles as stable in Pants 2.7, so it should be using the final proper name of `./pants generate-lockfiles` rather than `./pants tool-lock`. Once user lockfiles are stable in 2.8, we will kill `generate-user-lockfile` in favor of a unified `generate-lockfiles` goal. ## `generate-lockfiles` vs. `lock` vs `resolve` vs. something else See https://github.com/pantsbuild/pants/pull/12641#issuecomment-905777885 for why I went with `generate-lockfiles`, and the above thread for alternative proposals. [ci skip-rust] [ci skip-build-wheels] --- .../bin/_generate_all_lockfiles_helper.py | 6 +- .../backend/experimental/python/lockfile.py | 60 ++++++++----------- .../python/subsystems/python_tool_base.py | 2 +- src/python/pants/python/python_setup.py | 4 +- 4 files changed, 32 insertions(+), 40 deletions(-) diff --git a/build-support/bin/_generate_all_lockfiles_helper.py b/build-support/bin/_generate_all_lockfiles_helper.py index 50939f7c7e3..0d1c90a6045 100644 --- a/build-support/bin/_generate_all_lockfiles_helper.py +++ b/build-support/bin/_generate_all_lockfiles_helper.py @@ -42,8 +42,8 @@ def main() -> None: # and egg problem from https://github.com/pantsbuild/pants/issues/12457. We must # restore it here so that the lockfile gets generated properly. "--python-setup-experimental-lockfile=3rdparty/python/lockfiles/user_reqs.txt", - "lock", - "tool-lock", + "generate-lockfiles", + "generate-user-lockfile", "::", ], check=True, @@ -129,7 +129,7 @@ def main() -> None: f"--coverage-py-interpreter-constraints={repr(CoverageSubsystem.default_interpreter_constraints)}", f"--coverage-py-experimental-lockfile={CoverageSubsystem.default_lockfile_path}", # Run the goal. - "tool-lock", + "generate-lockfiles", ], check=True, ) diff --git a/src/python/pants/backend/experimental/python/lockfile.py b/src/python/pants/backend/experimental/python/lockfile.py index fd37ca4d2e3..97937deea92 100644 --- a/src/python/pants/backend/experimental/python/lockfile.py +++ b/src/python/pants/backend/experimental/python/lockfile.py @@ -112,7 +112,7 @@ def from_tool( ), dest=subsystem.lockfile, description=f"Generate lockfile for {subsystem.options_scope}", - regenerate_command="./pants tool-lock", + regenerate_command="./pants generate-lockfiles", ) @property @@ -188,35 +188,32 @@ async def generate_lockfile( # -------------------------------------------------------------------------------------- -class LockSubsystem(GoalSubsystem): - name = "lock" - help = "Generate a lockfile." +# TODO(#12314): Unify with the `generate-lockfiles` goal. Stop looking at specs and instead have +# an option like `--lock-resolves` with a list of named resolves (including tools). +class GenerateUserLockfileSubsystem(GoalSubsystem): + name = "generate-user-lockfile" + help = "Generate a lockfile for Python user requirements (experimental)." -class LockGoal(Goal): - subsystem_cls = LockSubsystem +class GenerateUserLockfileGoal(Goal): + subsystem_cls = GenerateUserLockfileSubsystem @goal_rule -async def lockfile_goal( +async def generate_user_lockfile_goal( addresses: Addresses, python_setup: PythonSetup, workspace: Workspace, -) -> LockGoal: +) -> GenerateUserLockfileGoal: if python_setup.lockfile is None: logger.warning( - "You ran `./pants lock`, but `[python-setup].experimental_lockfile` is not set. Please " - "set this option to the path where you'd like the lockfile for your code's " - "dependencies to live." + "You ran `./pants generate-user-lockfile`, but `[python-setup].experimental_lockfile` " + "is not set. Please set this option to the path where you'd like the lockfile for " + "your code's dependencies to live." ) - return LockGoal(exit_code=1) + return GenerateUserLockfileGoal(exit_code=1) - # TODO(#12314): Looking at the transitive closure to generate a single lockfile will not work - # when we have multiple user lockfiles supported. Ideally, `./pants lock ::` would mean - # "regenerate all unique lockfiles", whereas now it means "generate a single lockfile based - # on this transitive closure." transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(addresses)) - reqs = PexRequirements.create_from_requirement_fields( tgt[PythonRequirementsField] # NB: By looking at the dependencies, rather than the closure, we only generate for @@ -230,7 +227,7 @@ async def lockfile_goal( "No third-party requirements found for the transitive closure, so a lockfile will not " "be generated." ) - return LockGoal(exit_code=0) + return GenerateUserLockfileGoal(exit_code=0) result = await Get( PythonLockfile, @@ -251,7 +248,7 @@ async def lockfile_goal( workspace.write_digest(result.digest) logger.info(f"Wrote lockfile to {result.path}") - return LockGoal(exit_code=0) + return GenerateUserLockfileGoal(exit_code=0) # -------------------------------------------------------------------------------------- @@ -264,31 +261,26 @@ class PythonToolLockfileSentinel: pass -# TODO(#12314): Unify this goal with `lock` once we figure out how to unify the semantics, -# particularly w/ CLI specs. This is a separate goal only to facilitate progress. -class ToolLockSubsystem(GoalSubsystem): - name = "tool-lock" - help = "Generate a lockfile for a Python tool." +class GenerateLockfilesSubsystem(GoalSubsystem): + name = "generate-lockfiles" + help = "Generate lockfiles for Python third-party dependencies." required_union_implementations = (PythonToolLockfileSentinel,) -class ToolLockGoal(Goal): - subsystem_cls = ToolLockSubsystem +class GenerateLockfilesGoal(Goal): + subsystem_cls = GenerateLockfilesSubsystem @goal_rule -async def generate_all_tool_lockfiles( - workspace: Workspace, - union_membership: UnionMembership, -) -> ToolLockGoal: - # TODO(#12314): Add logic to inspect the Specs and generate for only relevant lockfiles. For - # now, we generate for all tools. +async def generate_lockfiles_goal( + workspace: Workspace, union_membership: UnionMembership +) -> GenerateLockfilesGoal: requests = await MultiGet( Get(PythonLockfileRequest, PythonToolLockfileSentinel, sentinel()) for sentinel in union_membership.get(PythonToolLockfileSentinel) ) if not requests: - return ToolLockGoal(exit_code=0) + return GenerateLockfilesGoal(exit_code=0) results = await MultiGet( Get(PythonLockfile, PythonLockfileRequest, req) @@ -300,7 +292,7 @@ async def generate_all_tool_lockfiles( for result in results: logger.info(f"Wrote lockfile to {result.path}") - return ToolLockGoal(exit_code=0) + return GenerateLockfilesGoal(exit_code=0) def rules(): diff --git a/src/python/pants/backend/python/subsystems/python_tool_base.py b/src/python/pants/backend/python/subsystems/python_tool_base.py index 5743ed9a1d9..dfcfc904b42 100644 --- a/src/python/pants/backend/python/subsystems/python_tool_base.py +++ b/src/python/pants/backend/python/subsystems/python_tool_base.py @@ -99,7 +99,7 @@ def register_options(cls, register): "recommend this, as lockfiles are essential for reproducible builds.\n\n" "To use a custom lockfile, set this option to a file path relative to the " "build root, then activate the backend_package " - "`pants.backend.experimental.python` and run `./pants tool-lock`.\n\n" + "`pants.backend.experimental.python` and run `./pants generate-lockfiles`.\n\n" "This option is experimental and will likely change. It does not follow the " "normal deprecation cycle." ), diff --git a/src/python/pants/python/python_setup.py b/src/python/pants/python/python_setup.py index cca2aee4649..9c2de048a13 100644 --- a/src/python/pants/python/python_setup.py +++ b/src/python/pants/python/python_setup.py @@ -123,7 +123,7 @@ def register_options(cls, register): "multiple lockfiles. This option's behavior may change without the normal " "deprecation cycle.\n\n" "To generate a lockfile, activate the backend `pants.backend.experimental.python`" - "and run `./pants lock ::`.\n\n" + "and run `./pants generate-user-lockfile ::`.\n\n" "Mutually exclusive with `[python-setup].requirement_constraints`." ), ) @@ -138,7 +138,7 @@ def register_options(cls, register): default=None, help=( "If set, Pants will instruct your users to run a custom command to regenerate " - "lockfiles, rather than running `./pants lock` and `./pants tool-lock` like normal." + "lockfiles, rather than running `./pants generate-lockfiles` like normal." "\n\nThis option is experimental and it may change at any time without the normal " "deprecation cycle." ),