use temporary directory instead of managing the local feedstock dir manually

Author: ytausch

conda_forge_tick/auto_tick.py

@@ -4,7 +4,6 @@
 import logging
 import os
 import random
-import shutil
 import textwrap
 import time
 import traceback
@@ -30,7 +29,7 @@
 from conda_forge_tick.cli_context import CliContext
 from conda_forge_tick.deploy import deploy
 from conda_forge_tick.contexts import (
-    GIT_CLONE_DIR,
+    ClonedFeedstockContext,
     FeedstockContext,
     MigratorSessionContext,
 )
@@ -151,7 +150,7 @@ def _get_pre_pr_migrator_attempts(attrs, migrator_name, *, is_version):
 
 def _prepare_feedstock_repository(
     backend: GitPlatformBackend,
-    context: FeedstockContext,
+    context: ClonedFeedstockContext,
     branch: str,
     base_branch: str,
 ) -> bool:
@@ -196,7 +195,7 @@ def _prepare_feedstock_repository(
 
 def _commit_migration(
     cli: GitCli,
-    context: FeedstockContext,
+    context: ClonedFeedstockContext,
     commit_message: str,
     allow_empty_commits: bool = False,
     raise_commit_errors: bool = True,
@@ -252,7 +251,7 @@ class _RerenderInfo:
 
 
 def _run_rerender(
-    git_cli: GitCli, context: FeedstockContext, suppress_errors: bool = False
+    git_cli: GitCli, context: ClonedFeedstockContext, suppress_errors: bool = False
 ) -> _RerenderInfo:
     logger.info("Rerendering the feedstock")
 
@@ -398,7 +397,7 @@ def _handle_solvability_error(
 
 
 def _check_and_process_solvability(
-    migrator: Migrator, context: FeedstockContext, base_branch: str
+    migrator: Migrator, context: ClonedFeedstockContext, base_branch: str
 ) -> bool:
     """
     If the migration needs a solvability check, perform the check. If the recipe is not solvable, handle the error
@@ -442,20 +441,48 @@ def get_spoofed_closed_pr_info() -> PullRequestInfoSpecial:
     )
 
 
-def run(
+def run_with_tmpdir(
     context: FeedstockContext,
     migrator: Migrator,
     rerender: bool = True,
     base_branch: str = "main",
     dry_run: bool = False,
     **kwargs: typing.Any,
+) -> tuple[MigrationUidTypedDict, dict] | tuple[Literal[False], Literal[False]]:
+    """
+    For a given feedstock and migration run the migration in a temporary directory that will be deleted after the
+    migration is complete.
+
+    The parameters are the same as for the `run` function. The only difference is that you pass a FeedstockContext
+    instance instead of a ClonedFeedstockContext instance.
+
+    The exceptions are the same as for the `run` function.
+    """
+    with context.reserve_clone_directory() as cloned_context:
+        return run(
+            context=cloned_context,
+            migrator=migrator,
+            rerender=rerender,
+            base_branch=base_branch,
+            dry_run=dry_run,
+            **kwargs,
+        )
+
+
+def run(
+    context: ClonedFeedstockContext,
+    migrator: Migrator,
+    rerender: bool = True,
+    base_branch: str = "main",
+    dry_run: bool = False,
+    **kwargs: typing.Any,
 ) -> tuple[MigrationUidTypedDict, dict] | tuple[Literal[False], Literal[False]]:
     """For a given feedstock and migration run the migration
 
     Parameters
     ----------
-    context: FeedstockContext
-        The node attributes
+    context: ClonedFeedstockContext
+        The current feedstock context, already containing information about a temporary directory for the feedstock.
     migrator: Migrator instance
         The migrator to run on the feedstock
     rerender : bool
@@ -513,7 +540,6 @@ def run(
         logger.critical(
             f"Failed to migrate {context.feedstock_name}, {context.attrs.get('pr_info', {}).get('bad')}",
         )
-        shutil.rmtree(context.local_clone_dir)
         return False, False
 
     # We raise an exception if we don't plan to rerender and wanted an empty commit.
@@ -607,8 +633,6 @@ def run(
         context.attrs, migrator_name, is_version=is_version_migration
     )
 
-    logger.info("Removing feedstock dir")
-    shutil.rmtree(context.local_clone_dir)
     return migration_run_data["migrate_return_value"], pr_lazy_json
 
 
@@ -690,7 +714,7 @@ def _run_migrator_on_feedstock_branch(
     attrs,
     base_branch,
     migrator,
-    fctx,
+    fctx: FeedstockContext,
     dry_run,
     mctx,
     migrator_name,
@@ -702,7 +726,7 @@ def _run_migrator_on_feedstock_branch(
             fctx.attrs["new_version"] = attrs.get("version_pr_info", {}).get(
                 "new_version", None
             )
-            migrator_uid, pr_json = run(
+            migrator_uid, pr_json = run_with_tmpdir(
                 context=fctx,
                 migrator=migrator,
                 rerender=migrator.rerender,
@@ -1013,7 +1037,6 @@ def _run_migrator(migrator, mctx, temp, time_per, dry_run):
                     dump_graph(mctx.graph)
 
                 with filter_reprinted_lines("rm-tmp"):
-                    eval_cmd(["rm", "-rf", f"{GIT_CLONE_DIR}/*"])
                     for f in glob.glob("/tmp/*"):
                         if f not in temp:
                             try:

conda_forge_tick/contexts.py

@@ -1,5 +1,10 @@
+from __future__ import annotations
+
 import os
+import tempfile
 import typing
+from collections.abc import Iterator
+from contextlib import contextmanager
 from dataclasses import dataclass
 from pathlib import Path
 
@@ -12,9 +17,6 @@
     from conda_forge_tick.migrators_types import AttrsTypedDict
 
 
-GIT_CLONE_DIR = Path("feedstocks").resolve()
-
-
 if os.path.exists("all_feedstocks.json"):
     with open("all_feedstocks.json") as f:
         DEFAULT_BRANCHES = load(f).get("default_branches", {})
@@ -32,10 +34,10 @@ class MigratorSessionContext:
     dry_run: bool = True
 
 
-@dataclass
+@dataclass(frozen=True)
 class FeedstockContext:
     feedstock_name: str
-    attrs: "AttrsTypedDict"
+    attrs: AttrsTypedDict
     _default_branch: str = None
 
     @property
@@ -45,10 +47,6 @@ def default_branch(self):
         else:
             return self._default_branch
 
-    @default_branch.setter
-    def default_branch(self, v):
-        self._default_branch = v
-
     @property
     def git_repo_owner(self) -> str:
         return "conda-forge"
@@ -64,13 +62,6 @@ def git_href(self) -> str:
         """
         return f"https://github.com/{self.git_repo_owner}/{self.git_repo_name}"
 
-    @property
-    def local_clone_dir(self) -> Path:
-        """
-        The local path to the feedstock repository.
-        """
-        return GIT_CLONE_DIR / self.git_repo_name
-
     @property
     def automerge(self) -> bool | str:
         """
@@ -100,3 +91,29 @@ def check_solvable(self) -> bool:
             {},
             False,
         )
+
+    @contextmanager
+    def reserve_clone_directory(self) -> Iterator[ClonedFeedstockContext]:
+        """
+        Reserve a temporary directory for the feedstock repository that will be available within the context manager.
+        The returned context object will contain the path to the feedstock repository in local_clone_dir.
+        After the context manager exits, the temporary directory will be deleted.
+        """
+        with tempfile.TemporaryDirectory() as tmpdir:
+            local_clone_dir = Path(tmpdir) / self.git_repo_name
+            local_clone_dir.mkdir()
+            yield ClonedFeedstockContext(
+                **self.__dict__,
+                local_clone_dir=local_clone_dir,
+            )
+
+
+@dataclass(frozen=True, kw_only=True)
+class ClonedFeedstockContext(FeedstockContext):
+    """
+    A FeedstockContext object that has reserved a temporary directory for the feedstock repository.
+    """
+
+    # Implementation Note: Keep this class frozen or there will be consistency issues if someone modifies
+    # a ClonedFeedstockContext object in place - it will not be reflected in the original FeedstockContext object.
+    local_clone_dir: Path

conda_forge_tick/migration_runner.py

@@ -3,8 +3,9 @@
 import os
 import shutil
 import tempfile
+from pathlib import Path
 
-from conda_forge_tick.contexts import FeedstockContext
+from conda_forge_tick.contexts import ClonedFeedstockContext
 from conda_forge_tick.lazy_json_backends import LazyJson, dumps
 from conda_forge_tick.os_utils import (
     chmod_plus_rwX,
@@ -221,11 +222,14 @@ def run_migration_local(
           - pr_body: The PR body for the migration.
     """
 
-    feedstock_ctx = FeedstockContext(
+    # it would be better if we don't re-instantiate ClonedFeedstockContext ourselves and let
+    # FeedstockContext.reserve_clone_directory be the only way to create a ClonedFeedstockContext
+    feedstock_ctx = ClonedFeedstockContext(
         feedstock_name=feedstock_name,
         attrs=node_attrs,
+        _default_branch=default_branch,
+        local_clone_dir=Path(feedstock_dir),
     )
-    feedstock_ctx.default_branch = default_branch
     recipe_dir = os.path.join(feedstock_dir, "recipe")
 
     data = {

conda_forge_tick/migrators/arch.py

@@ -4,7 +4,7 @@
 
 import networkx as nx
 
-from conda_forge_tick.contexts import FeedstockContext
+from conda_forge_tick.contexts import ClonedFeedstockContext, FeedstockContext
 from conda_forge_tick.make_graph import (
     get_deps_from_outputs_lut,
     make_outputs_lut_from_graph,
@@ -213,7 +213,7 @@ def migrate(
     def pr_title(self, feedstock_ctx: FeedstockContext) -> str:
         return "Arch Migrator"
 
-    def pr_body(self, feedstock_ctx: FeedstockContext) -> str:
+    def pr_body(self, feedstock_ctx: ClonedFeedstockContext) -> str:
         body = super().pr_body(feedstock_ctx)
         body = body.format(
             dedent(
@@ -384,7 +384,7 @@ def migrate(
     def pr_title(self, feedstock_ctx: FeedstockContext) -> str:
         return "ARM OSX Migrator"
 
-    def pr_body(self, feedstock_ctx: FeedstockContext) -> str:
+    def pr_body(self, feedstock_ctx: ClonedFeedstockContext) -> str:
         body = super().pr_body(feedstock_ctx)
         body = body.format(
             dedent(

conda_forge_tick/migrators/broken_rebuild.py

@@ -2,6 +2,7 @@
 
 import networkx as nx
 
+from conda_forge_tick.contexts import ClonedFeedstockContext
 from conda_forge_tick.migrators.core import Migrator
 
 BROKEN_PACKAGES = """\
@@ -380,7 +381,7 @@ def migrate(self, recipe_dir, attrs, **kwargs):
         self.set_build_number(os.path.join(recipe_dir, "meta.yaml"))
         return super().migrate(recipe_dir, attrs)
 
-    def pr_body(self, feedstock_ctx) -> str:
+    def pr_body(self, feedstock_ctx: ClonedFeedstockContext) -> str:
         body = super().pr_body(feedstock_ctx)
         body = body.format(
             """\

conda_forge_tick/migrators/core.py

@@ -10,7 +10,7 @@
 import dateutil.parser
 import networkx as nx
 
-from conda_forge_tick.contexts import FeedstockContext
+from conda_forge_tick.contexts import ClonedFeedstockContext, FeedstockContext
 from conda_forge_tick.lazy_json_backends import LazyJson
 from conda_forge_tick.make_graph import make_outputs_lut_from_graph
 from conda_forge_tick.path_lengths import cyclic_topological_sort
@@ -455,7 +455,9 @@ def migrate(
         """
         return self.migrator_uid(attrs)
 
-    def pr_body(self, feedstock_ctx: FeedstockContext, add_label_text=True) -> str:
+    def pr_body(
+        self, feedstock_ctx: ClonedFeedstockContext, add_label_text=True
+    ) -> str:
         """Create a PR message body
 
         Returns

conda_forge_tick/migrators/migration_yaml.py

@@ -10,7 +10,7 @@
 
 import networkx as nx
 
-from conda_forge_tick.contexts import FeedstockContext
+from conda_forge_tick.contexts import ClonedFeedstockContext, FeedstockContext
 from conda_forge_tick.feedstock_parser import PIN_SEP_PAT
 from conda_forge_tick.make_graph import get_deps_from_outputs_lut
 from conda_forge_tick.migrators.core import GraphMigrator, Migrator, MiniMigrator
@@ -280,7 +280,7 @@ def migrate(
 
             return super().migrate(recipe_dir, attrs)
 
-    def pr_body(self, feedstock_ctx: "FeedstockContext") -> str:
+    def pr_body(self, feedstock_ctx: ClonedFeedstockContext) -> str:
         body = super().pr_body(feedstock_ctx)
         if feedstock_ctx.feedstock_name == "conda-forge-pinning":
             additional_body = (
@@ -534,7 +534,7 @@ def migrate(
 
         return super().migrate(recipe_dir, attrs)
 
-    def pr_body(self, feedstock_ctx: "FeedstockContext") -> str:
+    def pr_body(self, feedstock_ctx: ClonedFeedstockContext) -> str:
         body = (
             "This PR has been triggered in an effort to update the pin for"
             " **{name}**. The current pinned version is {current_pin}, "

conda_forge_tick/migrators/replacement.py

@@ -6,7 +6,7 @@
 
 import networkx as nx
 
-from conda_forge_tick.contexts import FeedstockContext
+from conda_forge_tick.contexts import ClonedFeedstockContext, FeedstockContext
 from conda_forge_tick.migrators.core import Migrator
 
 if typing.TYPE_CHECKING:
@@ -127,7 +127,7 @@ def migrate(
         self.set_build_number(os.path.join(recipe_dir, "meta.yaml"))
         return super().migrate(recipe_dir, attrs)
 
-    def pr_body(self, feedstock_ctx: FeedstockContext) -> str:
+    def pr_body(self, feedstock_ctx: ClonedFeedstockContext) -> str:
         body = super().pr_body(feedstock_ctx)
         body = body.format(
             "I noticed that this recipe depends on `%s` instead of \n"