Merge pull request #3000 from ytausch/continue-git-backend-part-4

conda_forge_tick/auto_tick.py

@@ -4,9 +4,11 @@
 import logging
 import os
 import random
+import textwrap
 import time
 import traceback
 import typing
+from dataclasses import dataclass
 from textwrap import dedent
 from typing import Literal, MutableMapping, cast
 from urllib.error import URLError
@@ -54,7 +56,7 @@
 from conda_forge_tick.migration_runner import run_migration
 from conda_forge_tick.migrators import MigrationYaml, Migrator, Version
 from conda_forge_tick.migrators.version import VersionMigrationError
-from conda_forge_tick.os_utils import eval_cmd, pushd
+from conda_forge_tick.os_utils import eval_cmd
 from conda_forge_tick.rerender_feedstock import rerender_feedstock
 from conda_forge_tick.solver_checks import is_recipe_solvable
 from conda_forge_tick.utils import (
@@ -223,6 +225,70 @@ def _commit_migration(
             raise
 
 
+@dataclass(frozen=True)
+class _RerenderInfo:
+    """
+    Additional information about a rerender operation.
+    """
+
+    nontrivial_changes: bool
+    """
+    True if any files which are not in the following list were changed during the rerender, False otherwise:
+    1. anything in the recipe directory
+    2. the README file
+    """
+    rerender_comment: str | None = None
+    """
+    If requested, a comment to be added to the PR to indicate an issue with the rerender.
+    None if no comment should be added.
+    """
+
+
+def _run_rerender(
+    git_cli: GitCli, context: ClonedFeedstockContext, suppress_errors: bool = False
+) -> _RerenderInfo:
+    logger.info("Rerendering the feedstock")
+
+    try:
+        rerender_msg = rerender_feedstock(str(context.local_clone_dir), timeout=900)
+    except Exception as e:
+        logger.error("RERENDER ERROR", exc_info=e)
+
+        if not suppress_errors:
+            raise
+
+        rerender_comment = textwrap.dedent(
+            """
+            Hi! This feedstock was not able to be rerendered after the version update changes. I
+            have pushed the version update changes anyways and am trying to rerender again with this
+            comment. Hopefully you all can fix this!
+
+            @conda-forge-admin rerender
+            """
+        )
+
+        return _RerenderInfo(
+            nontrivial_changes=False, rerender_comment=rerender_comment
+        )
+
+    if rerender_msg is None:
+        return _RerenderInfo(nontrivial_changes=False)
+
+    git_cli.commit(context.local_clone_dir, rerender_msg, all_=True, allow_empty=True)
+
+    # HEAD~ is the state before the last commit
+    changed_files = git_cli.diffed_files(context.local_clone_dir, "HEAD~")
+
+    recipe_dir = context.local_clone_dir / "recipe"
+
+    nontrivial_changes = any(
+        not file.is_relative_to(recipe_dir) and not file.name.startswith("README")
+        for file in changed_files
+    )
+
+    return _RerenderInfo(nontrivial_changes=nontrivial_changes)
+
+
 def run_with_tmpdir(
     context: FeedstockContext,
     migrator: Migrator,
@@ -336,6 +402,20 @@ def run(
         raise_commit_errors=migrator.allow_empty_commits and not rerender,
     )
 
+    if rerender:
+        # for version migrations, without check solvable or automerge, we can suppress rerender errors
+        suppress_errors = (
+            is_version_migration
+            and not context.check_solvable
+            and not context.automerge
+        )
+
+        rerender_info = _run_rerender(
+            git_backend.cli, context, suppress_errors=suppress_errors
+        )
+    else:
+        rerender_info = _RerenderInfo(nontrivial_changes=False)
+
     # This is needed because we want to migrate to the new backend step-by-step
     repo: github3.repos.Repository | None = github3_client().repository(
         context.git_repo_owner, context.git_repo_name
@@ -345,73 +425,13 @@ def run(
 
     feedstock_dir = str(context.local_clone_dir.resolve())
 
-    # rerender, maybe
-    diffed_files: typing.List[str] = []
-    with pushd(feedstock_dir):
-        if rerender:
-            head_ref = eval_cmd(["git", "rev-parse", "HEAD"]).strip()
-            logger.info("Rerendering the feedstock")
-
-            try:
-                rerender_msg = rerender_feedstock(feedstock_dir, timeout=900)
-                if rerender_msg is not None:
-                    eval_cmd(["git", "commit", "--allow-empty", "-am", rerender_msg])
-
-                make_rerender_comment = False
-            except Exception as e:
-                # I am trying this bit of code to force these errors
-                # to be surfaced in the logs at the right time.
-                print(f"RERENDER ERROR: {e}", flush=True)
-                if not isinstance(migrator, Version):
-                    raise
-                else:
-                    # for check solvable or automerge, we always raise rerender errors
-                    if get_keys_default(
-                        context.attrs,
-                        ["conda-forge.yml", "bot", "check_solvable"],
-                        {},
-                        False,
-                    ) or get_keys_default(
-                        context.attrs,
-                        ["conda-forge.yml", "bot", "automerge"],
-                        {},
-                        False,
-                    ):
-                        raise
-                    else:
-                        make_rerender_comment = True
-
-            # If we tried to run the MigrationYaml and rerender did nothing (we only
-            # bumped the build number and dropped a yaml file in migrations) bail
-            # for instance platform specific migrations
-            gdiff = eval_cmd(
-                ["git", "diff", "--name-only", f"{head_ref.strip()}...HEAD"]
-            )
-
-            diffed_files = [
-                _
-                for _ in gdiff.split()
-                if not (
-                    _.startswith("recipe")
-                    or _.startswith("migrators")
-                    or _.startswith("README")
-                )
-            ]
-        else:
-            make_rerender_comment = False
-
-    feedstock_automerge = get_keys_default(
-        context.attrs,
-        ["conda-forge.yml", "bot", "automerge"],
-        {},
-        False,
-    )
     if isinstance(migrator, Version):
-        has_automerge = feedstock_automerge in [True, "version"]
+        has_automerge = context.automerge in [True, "version"]
     else:
-        has_automerge = getattr(
-            migrator, "automerge", False
-        ) and feedstock_automerge in [True, "migration"]
+        has_automerge = getattr(migrator, "automerge", False) and context.automerge in [
+            True,
+            "migration",
+        ]
 
     migrator_check_solvable = getattr(migrator, "check_solvable", True)
     feedstock_check_solvable = get_keys_default(
@@ -432,7 +452,7 @@ def run(
     logger.info(
         f"""automerge and check_solvable status/settings:
     automerge:
-        feedstock_automerge: {feedstock_automerge}
+        feedstock_automerge: {context.automerge}
         migratror_automerge: {getattr(migrator, 'automerge', False)}
         has_automerge: {has_automerge} (only considers feedstock if version migration)
     check_solvable:
@@ -505,7 +525,7 @@ def run(
     pr_json: typing.Union[MutableMapping, None, bool]
     if (
         isinstance(migrator, MigrationYaml)
-        and not diffed_files
+        and not rerender_info.nontrivial_changes
         and context.attrs["name"] != "conda-forge-pinning"
     ):
         # spoof this so it looks like the package is done
@@ -538,15 +558,10 @@ def run(
                 pr_json = False
                 ljpr = False
 
-    if pr_json and pr_json["state"] != "closed" and make_rerender_comment:
+    if pr_json and pr_json["state"] != "closed" and rerender_info.rerender_comment:
         comment_on_pr(
             pr_json,
-            """\
-Hi! This feedstock was not able to be rerendered after the version update changes. I
-have pushed the version update changes anyways and am trying to rerender again with this
-comment. Hopefully you all can fix this!
-
-@conda-forge-admin rerender""",
+            rerender_info.rerender_comment,
             repo,
         )
 

conda_forge_tick/contexts.py

@@ -11,6 +11,7 @@
 
 from conda_forge_tick.lazy_json_backends import load
 from conda_forge_tick.migrators_types import AttrsTypedDict
+from conda_forge_tick.utils import get_keys_default
 
 if os.path.exists("all_feedstocks.json"):
     with open("all_feedstocks.json") as f:
@@ -54,6 +55,34 @@ def git_repo_owner(self) -> str:
     def git_repo_name(self) -> str:
         return f"{self.feedstock_name}-feedstock"
 
+    @property
+    def automerge(self) -> bool | str:
+        """
+        Get the automerge setting of the feedstock.
+        Note: A better solution to implement this is to use the NodeAttributes Pydantic
+        model for the attrs field. This can be done in the future.
+        """
+        return get_keys_default(
+            self.attrs,
+            ["conda-forge.yml", "bot", "automerge"],
+            {},
+            False,
+        )
+
+    @property
+    def check_solvable(self) -> bool:
+        """
+        Get the check_solvable setting of the feedstock.
+        Note: A better solution to implement this is to use the NodeAttributes Pydantic
+        model for the attrs field. This can be done in the future.
+        """
+        return get_keys_default(
+            self.attrs,
+            ["conda-forge.yml", "bot", "check_solvable"],
+            {},
+            False,
+        )
+
     @contextmanager
     def reserve_clone_directory(self) -> Iterator[ClonedFeedstockContext]:
         """

conda_forge_tick/git_utils.py

@@ -11,7 +11,7 @@
 from datetime import datetime
 from functools import cached_property
 from pathlib import Path
-from typing import Dict, Optional, Union
+from typing import Dict, Iterator, Optional, Union
 
 import backoff
 import github
@@ -327,6 +327,27 @@ def checkout_new_branch(
             ["checkout", "--quiet", "-b", branch] + start_point_option, git_dir
         )
 
+    def diffed_files(
+        self, git_dir: Path, commit_a: str, commit_b: str = "HEAD"
+    ) -> Iterator[Path]:
+        """
+        Get the files that are different between two commits.
+        :param git_dir: The directory of the git repository. This should be the root of the repository.
+            If it is a subdirectory, only the files in that subdirectory will be returned.
+        :param commit_a: The first commit.
+        :param commit_b: The second commit.
+        :return: An iterator over the files that are different between the two commits.
+        """
+
+        # --relative ensures that we do not assemble invalid paths below if git_dir is a subdirectory
+        ret = self._run_git_command(
+            ["diff", "--name-only", "--relative", commit_a, commit_b],
+            git_dir,
+            capture_text=True,
+        )
+
+        return (git_dir / line for line in ret.stdout.splitlines())
+
     @lock_git_operation()
     def clone_fork_and_branch(
         self,

conda_forge_tick/migrators_types.py

@@ -136,6 +136,7 @@ class AttrsTypedDict_(TypedDict, total=False):
     new_version: Union[str, bool]
     archived: bool
     PRed: List[PRedElementTypedDict]
+    version_pr_info: typing.Any
     # Legacy types in here
     bad: Union[bool, str]
     # TODO: ADD in

tests/test_contexts.py

@@ -56,6 +56,25 @@ def test_feedstock_context_git_repo_name():
     assert context.git_repo_name == "TEST-FEEDSTOCK-NAME-feedstock"
 
 
+@pytest.mark.parametrize("automerge", [True, False])
+def test_feedstock_context_automerge(automerge: bool):
+    context = FeedstockContext(
+        "TEST-FEEDSTOCK-NAME", demo_attrs_automerge if automerge else demo_attrs
+    )
+
+    assert context.automerge == automerge
+
+
+@pytest.mark.parametrize("check_solvable", [True, False])
+def test_feedstock_context_check_solvable(check_solvable: bool):
+    context = FeedstockContext(
+        "TEST-FEEDSTOCK-NAME",
+        demo_attrs_check_solvable if check_solvable else demo_attrs,
+    )
+
+    assert context.check_solvable == check_solvable
+
+
 @pytest.mark.parametrize("default_branch", [None, "", "feature"])
 @pytest.mark.parametrize(
     "attrs", [demo_attrs, demo_attrs_automerge, demo_attrs_check_solvable]