Deprecate generate-user-lockfile and `[python].experimental_lockfi…

…le` in favor of `[python].experimental_resolves` (pantsbuild#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 pantsbuild#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]
TansyArron · Jan 28, 2022 · c890ec1 · c890ec1
1 parent 4356f46
commit c890ec1
Show file tree

Hide file tree

Showing 12 changed files with 54 additions and 154 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -5,87 +5,37 @@
 
 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):
     subsystem_cls = GenerateUserLockfileSubsystem
 
 
 @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
@@ -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
@@ -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="<file>",
             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=<name>` 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
@@ -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
@@ -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
diff --git a/testprojects/3rdparty/python/BUILD b/testprojects/3rdparty/python/BUILD
diff --git a/testprojects/3rdparty/python/requirements.txt b/testprojects/3rdparty/python/requirements.txt