From c890ec15b341da9e86cbccda4d74cd265f9ba054 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Fri, 28 Jan 2022 07:33:44 -0700 Subject: [PATCH] Deprecate `generate-user-lockfile` and `[python].experimental_lockfile` in favor of `[python].experimental_resolves` (#14288) Turns out that Pants only needs a single resolve. Our testprojects resolve wasn't actually used by anything. (We could always add this back for testing purposes.) So, thanks to https://github.com/pantsbuild/pants/pull/14287, we can use `[python].experimental_resolves` just like `[python].experimental_lockfile` was being used: the feature currently supports a single resolve. Benefits to this change: 1. Lockfile is now generated via `generate-lockfiles` rather than `generate-user-lockfiles` 2. Less confusing code for us to deal with. Now we just have constraints files vs. resolves [ci skip-rust] --- 3rdparty/python/lockfiles/user_reqs.txt | 3 + .../bin/_generate_all_lockfiles_helper.py | 36 ++++----- build-support/bin/generate_all_lockfiles.sh | 6 +- pants.toml | 10 +-- .../experimental/python/user_lockfiles.py | 78 ++++--------------- .../pants/backend/python/goals/lockfile.py | 9 +-- .../pants/backend/python/subsystems/setup.py | 33 ++++---- .../python/util_rules/pex_from_targets.py | 15 ---- .../backend/python/util_rules/pex_test.py | 5 +- testprojects/3rdparty/BUILD | 4 - testprojects/3rdparty/python/BUILD | 7 -- testprojects/3rdparty/python/requirements.txt | 2 - 12 files changed, 54 insertions(+), 154 deletions(-) delete mode 100644 testprojects/3rdparty/BUILD delete mode 100644 testprojects/3rdparty/python/BUILD delete mode 100644 testprojects/3rdparty/python/requirements.txt diff --git a/3rdparty/python/lockfiles/user_reqs.txt b/3rdparty/python/lockfiles/user_reqs.txt index 70ed1daf77f..8b3b616288b 100644 --- a/3rdparty/python/lockfiles/user_reqs.txt +++ b/3rdparty/python/lockfiles/user_reqs.txt @@ -19,6 +19,7 @@ # "packaging==21.3", # "pex==2.1.65", # "psutil==5.9.0", +# "pydevd-pycharm==203.5419.8", # "pytest<6.3,>=6.2.4", # "requests[security]>=2.25.1", # "setproctitle==1.2.2", @@ -180,6 +181,8 @@ psutil==5.9.0; (python_version >= "2.6" and python_full_version < "3.0.0") or (p py==1.11.0; python_version >= "3.6" and python_full_version < "3.0.0" or python_full_version >= "3.5.0" and python_version >= "3.6" \ --hash=sha256:607c53218732647dff4acdfcd50cb62615cedf612e72d1724fb1a0cc6405b378 \ --hash=sha256:51c75c4126074b472f746a24399ad32f6053d1b34b68d2fa41e558e6f4a98719 +pydevd-pycharm==203.5419.8 \ + --hash=sha256:21e8035830546d89caf8669b0d01d4626c9c8d51799b591d522a6fd5a50b539c pyparsing==3.0.7; python_version >= "3.6" \ --hash=sha256:a6c06a88f252e6c322f65faf8f418b16213b51bdfaece0524c1c1bc30c63c484 \ --hash=sha256:18ee9022775d270c55187733956460083db60b37d0d0fb357445f3094eed3eea diff --git a/build-support/bin/_generate_all_lockfiles_helper.py b/build-support/bin/_generate_all_lockfiles_helper.py index 621cc9340fc..199f3a72449 100644 --- a/build-support/bin/_generate_all_lockfiles_helper.py +++ b/build-support/bin/_generate_all_lockfiles_helper.py @@ -167,24 +167,18 @@ def create_parser() -> argparse.ArgumentParser: return parser -def update_internal_lockfiles( - *, python_user_lockfile: bool = True, internal_tools: bool = True -) -> None: - args = ["./pants", "--concurrent"] - if python_user_lockfile: - args.extend( - [ - "--tag=-lockfile_ignore", - # `generate_all_lockfiles.sh` will have overridden this option to solve the chicken - # and egg problem from https://github.com/pantsbuild/pants/issues/12457. We must - # restore it here so that the lockfile gets generated properly. - "--python-experimental-lockfile=3rdparty/python/lockfiles/user_reqs.txt", - "generate-user-lockfile", - "::", - ] - ) - if internal_tools: - args.append("generate-lockfiles") +def update_internal_lockfiles(specified: list[str] | None) -> None: + args = [ + "./pants", + "--concurrent", + # `generate_all_lockfiles.sh` will have overridden this option to solve the chicken + # and egg problem from https://github.com/pantsbuild/pants/issues/12457. We must + # restore it here so that the lockfile gets generated properly. + "--python-enable-resolves", + "generate-lockfiles", + ] + if specified: + args.append(f"--resolve={repr(specified)}") subprocess.run(args, check=True) @@ -208,17 +202,17 @@ def main() -> None: args = create_parser().parse_args() if args.all: - update_internal_lockfiles() + update_internal_lockfiles(specified=None) update_default_lockfiles(specified=None) return if args.pex: - update_internal_lockfiles(internal_tools=False) + update_internal_lockfiles(specified=["python-default"]) update_default_lockfiles(specified=[Lambdex.options_scope]) return if args.internal: - update_internal_lockfiles() + update_internal_lockfiles(specified=None) if args.tool: update_default_lockfiles(specified=args.tool) diff --git a/build-support/bin/generate_all_lockfiles.sh b/build-support/bin/generate_all_lockfiles.sh index e2b2f72af11..9a5805b3e41 100755 --- a/build-support/bin/generate_all_lockfiles.sh +++ b/build-support/bin/generate_all_lockfiles.sh @@ -11,9 +11,9 @@ cd "$REPO_ROOT" || exit 1 source "${REPO_ROOT}/build-support/common.sh" # To solve the chicken and egg problem of https://github.com/pantsbuild/pants/issues/12457, we -# temporarily disable the lockfile. This means that `./pants run` will ignore the lockfile. While -# the script is then running, it will set the option again to generate the lockfile where we want. -export PANTS_PYTHON_EXPERIMENTAL_LOCKFILE="" +# temporarily disable resolves. This means that `./pants run` will not use a lockfile. While +# the script is then running, it will set the option again to generate the lockfile. +export PANTS_PYTHON_ENABLE_RESOLVES=false if is_macos_arm; then # Generate the lockfiles with the correct notated interpreter constraints, but make diff --git a/pants.toml b/pants.toml index e031cc8f670..9ccab487405 100644 --- a/pants.toml +++ b/pants.toml @@ -113,14 +113,8 @@ venv_use_symlinks = true [python] interpreter_constraints = [">=3.7,<3.10"] macos_big_sur_compatibility = true - -# To use resolves, switch which line is commented out. -experimental_lockfile = "3rdparty/python/lockfiles/user_reqs.txt" -#enable_resolves = true - -[python.experimental_resolves] -python-default = "3rdparty/python/lockfiles/user_reqs.lock" -python-testprojects = "testprojects/3rdparty/python/user_reqs.lock" +experimental_resolves = { python-default = "3rdparty/python/lockfiles/user_reqs.txt" } +enable_resolves = true [docformatter] args = ["--wrap-summaries=100", "--wrap-descriptions=100"] diff --git a/src/python/pants/backend/experimental/python/user_lockfiles.py b/src/python/pants/backend/experimental/python/user_lockfiles.py index ab3c16d6b7d..19d84151720 100644 --- a/src/python/pants/backend/experimental/python/user_lockfiles.py +++ b/src/python/pants/backend/experimental/python/user_lockfiles.py @@ -5,27 +5,19 @@ import logging -from pants.backend.python.goals.lockfile import GeneratePythonLockfile -from pants.backend.python.subsystems.setup import PythonSetup -from pants.backend.python.target_types import PythonRequirementsField -from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints -from pants.backend.python.util_rules.pex import PexRequirements -from pants.core.goals.generate_lockfiles import GenerateLockfileResult -from pants.engine.addresses import Addresses -from pants.engine.fs import Workspace +from pants.base.deprecated import warn_or_error from pants.engine.goal import Goal, GoalSubsystem -from pants.engine.rules import Get, collect_rules, goal_rule -from pants.engine.target import TransitiveTargets, TransitiveTargetsRequest -from pants.util.strutil import pluralize +from pants.engine.rules import collect_rules, goal_rule logger = logging.getLogger(__name__) -# 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)." + help = ( + "Deprecated: use the option `[python].experimental_resolves` and the " + "`generate-lockfiles` goal instead" + ) class GenerateUserLockfileGoal(Goal): @@ -33,59 +25,17 @@ class GenerateUserLockfileGoal(Goal): @goal_rule -async def generate_user_lockfile_goal( - addresses: Addresses, - python_setup: PythonSetup, - workspace: Workspace, -) -> GenerateUserLockfileGoal: - if python_setup.lockfile is None: - logger.warning( - "You ran `./pants generate-user-lockfile`, but `[python].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 GenerateUserLockfileGoal(exit_code=1) - - transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(addresses)) - req_strings = PexRequirements.create_from_requirement_fields( +async def generate_user_lockfile_goal() -> GenerateUserLockfileGoal: + warn_or_error( + "2.11.0.dev0", + "the `generate-user-lockfile` goal", ( - tgt[PythonRequirementsField] - # NB: By looking at the dependencies, rather than the closure, we only generate for - # requirements that are actually used in the project. - for tgt in transitive_targets.dependencies - if tgt.has_field(PythonRequirementsField) - ), - constraints_strings=(), - ).req_strings - - if not req_strings: - logger.warning( - "No third-party requirements found for the transitive closure, so a lockfile will not " - "be generated." - ) - return GenerateUserLockfileGoal(exit_code=0) - - result = await Get( - GenerateLockfileResult, - GeneratePythonLockfile( - requirements=req_strings, - # TODO(#12314): Use interpreter constraints from the transitive closure. - interpreter_constraints=InterpreterConstraints(python_setup.interpreter_constraints), - resolve_name="not yet implemented", - lockfile_dest=python_setup.lockfile, - _description=( - f"Generate lockfile for {pluralize(len(req_strings), 'requirement')}: " - f"{', '.join(req_strings)}" - ), - # TODO(12382): Make this command actually accurate once we figure out the semantics - # for user lockfiles. This is currently misleading. - _regenerate_command="./pants generate-user-lockfile ::", + "Instead, configure the option `[python].experimental_resolves`, then use the " + "`generate-lockfiles` goal. Read the deprecation message on " + "`[python].experimental_lockfile` for more information." ), ) - workspace.write_digest(result.digest) - logger.info(f"Wrote lockfile to {result.path}") - - return GenerateUserLockfileGoal(exit_code=0) + return GenerateUserLockfileGoal(exit_code=1) def rules(): diff --git a/src/python/pants/backend/python/goals/lockfile.py b/src/python/pants/backend/python/goals/lockfile.py index c536420bfc0..47c15b4fb3b 100644 --- a/src/python/pants/backend/python/goals/lockfile.py +++ b/src/python/pants/backend/python/goals/lockfile.py @@ -54,10 +54,6 @@ class GeneratePythonLockfile(GenerateLockfile): requirements: FrozenOrderedSet[str] interpreter_constraints: InterpreterConstraints use_pex: bool = False - # Only kept for `[python].experimental_lockfile`, which is not using the new - # "named resolve" semantics yet. - _description: str | None = None - _regenerate_command: str | None = None @classmethod def from_tool( @@ -163,7 +159,7 @@ async def generate_lockfile( *req.requirements, ), output_files=("lock.json",), - description=req._description or f"Generate lockfile for {req.resolve_name}", + description=f"Generate lockfile for {req.resolve_name}", # Instead of caching lockfile generation with LMDB, we instead use the invalidation # scheme from `lockfile_metadata.py` to check for stale/invalid lockfiles. This is # necessary so that our invalidation is resilient to deleting LMDB or running on a @@ -208,7 +204,7 @@ async def generate_lockfile( argv=("lock",), input_digest=_pyproject_toml_digest, output_files=("poetry.lock", "pyproject.toml"), - description=req._description or f"Generate lockfile for {req.resolve_name}", + description=f"Generate lockfile for {req.resolve_name}", cache_scope=ProcessCacheScope.PER_SESSION, ), ) @@ -236,7 +232,6 @@ async def generate_lockfile( initial_lockfile_digest_contents[0].content, regenerate_command=( generate_lockfiles_subsystem.custom_command - or req._regenerate_command or f"./pants generate-lockfiles --resolve={req.resolve_name}" ), ) diff --git a/src/python/pants/backend/python/subsystems/setup.py b/src/python/pants/backend/python/subsystems/setup.py index 72cea1a732c..005ea1c5404 100644 --- a/src/python/pants/backend/python/subsystems/setup.py +++ b/src/python/pants/backend/python/subsystems/setup.py @@ -82,8 +82,7 @@ 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` and " - "`[python].enable_resolves`." + "Mutually exclusive with `[python].enable_resolves`." ), ) register( @@ -106,18 +105,18 @@ def register_options(cls, register): register( "--experimental-lockfile", advanced=True, - # TODO(#11719): Switch this to a file_option once env vars can unset a value. type=str, metavar="", 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 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` and " - "`[python].enable_resolves`." + help="Deprecated.", + removal_version="2.11.0.dev0", + removal_hint=( + "Instead, use the improved `[python].experimental_resolves` mechanism. Read its " + "help message for more information.\n\n" + "If you want to keep using a single resolve like before, update " + "`[python].experimental_resolves` with a name for the resolve and the path to " + "its lockfile, or use the default. Then make sure that " + "`[python].experimental_default_resolve` is set to that resolve name." ), ) register( @@ -129,8 +128,7 @@ def register_options(cls, register): 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`." + "Mutually exclusive with `[python].requirement_constraints`." ), ) register( @@ -140,7 +138,9 @@ def register_options(cls, register): default={"python-default": "3rdparty/python/default_lock.txt"}, help=( "A mapping of logical names to lockfile paths used in your project.\n\n" - # TODO(#12314): explain how this feature works. + "For now, things only work properly if you define a single resolve and set " + "`[python].experimental_default_resolve` to that value. We are close to " + "properly supporting multiple (disjoint) resolves.\n\n" "To generate a lockfile, run `./pants generate-lockfiles --resolve=` or " "`./pants generate-lockfiles` to generate for all resolves (including tool " "lockfiles).\n\n" @@ -156,7 +156,6 @@ def register_options(cls, register): 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." ), @@ -287,10 +286,6 @@ def requirement_constraints(self) -> str | None: """Path to constraint file.""" return cast("str | None", self.options.requirement_constraints) - @property - 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) diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets.py b/src/python/pants/backend/python/util_rules/pex_from_targets.py index 4d9505ebe4d..55074ea2b01 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets.py @@ -467,21 +467,6 @@ async def get_repository_pex( platforms=request.platforms, additional_args=request.additional_lockfile_args, ) - elif python_setup.lockfile: - repository_pex_request = PexRequest( - description=f"Installing {python_setup.lockfile}", - output_filename="lockfile.pex", - internal_only=request.internal_only, - requirements=Lockfile( - file_path=python_setup.lockfile, - file_path_description_of_origin="the option `[python].experimental_lockfile`", - lockfile_hex_digest=None, - req_strings=None, - ), - interpreter_constraints=interpreter_constraints, - platforms=request.platforms, - additional_args=request.additional_lockfile_args, - ) return OptionalPexRequest(repository_pex_request) diff --git a/src/python/pants/backend/python/util_rules/pex_test.py b/src/python/pants/backend/python/util_rules/pex_test.py index a45e4569323..0e5744b0d62 100644 --- a/src/python/pants/backend/python/util_rules/pex_test.py +++ b/src/python/pants/backend/python/util_rules/pex_test.py @@ -776,10 +776,7 @@ def _run_pex_for_lockfile_test( rule_runner, interpreter_constraints=InterpreterConstraints([expected_constraints]), requirements=requirements, - additional_pants_args=( - "--python-experimental-lockfile=lockfile.txt", - f"--python-invalid-lockfile-behavior={behavior}", - ), + additional_pants_args=(f"--python-invalid-lockfile-behavior={behavior}",), ) diff --git a/testprojects/3rdparty/BUILD b/testprojects/3rdparty/BUILD deleted file mode 100644 index 6693b382151..00000000000 --- a/testprojects/3rdparty/BUILD +++ /dev/null @@ -1,4 +0,0 @@ -# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -files(name="python_directory", sources=["python/**/*"]) diff --git a/testprojects/3rdparty/python/BUILD b/testprojects/3rdparty/python/BUILD deleted file mode 100644 index 94aaf2aed84..00000000000 --- a/testprojects/3rdparty/python/BUILD +++ /dev/null @@ -1,7 +0,0 @@ -# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -python_requirements( - experimental_compatible_resolves=["python-testprojects"], - tags=["lockfile_ignore"], -) diff --git a/testprojects/3rdparty/python/requirements.txt b/testprojects/3rdparty/python/requirements.txt deleted file mode 100644 index 85cee779c6d..00000000000 --- a/testprojects/3rdparty/python/requirements.txt +++ /dev/null @@ -1,2 +0,0 @@ -checksumdir==1.1.7 -numpy==1.14.5