Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Git-4) Refactor rerender in auto_tick module #3000

Merged
merged 6 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 90 additions & 75 deletions conda_forge_tick/auto_tick.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -223,6 +225,70 @@
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")

Check warning on line 250 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L250

Added line #L250 was not covered by tests

try:
rerender_msg = rerender_feedstock(str(context.local_clone_dir), timeout=900)
except Exception as e:
logger.error("RERENDER ERROR", exc_info=e)

Check warning on line 255 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L252-L255

Added lines #L252 - L255 were not covered by tests

if not suppress_errors:
raise

Check warning on line 258 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L257-L258

Added lines #L257 - L258 were not covered by tests

rerender_comment = textwrap.dedent(

Check warning on line 260 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L260

Added line #L260 was not covered by tests
"""
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
ytausch marked this conversation as resolved.
Show resolved Hide resolved
"""
)

return _RerenderInfo(

Check warning on line 270 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L270

Added line #L270 was not covered by tests
nontrivial_changes=False, rerender_comment=rerender_comment
)

if rerender_msg is None:
return _RerenderInfo(nontrivial_changes=False)

Check warning on line 275 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L274-L275

Added lines #L274 - L275 were not covered by tests

git_cli.commit(context.local_clone_dir, rerender_msg, all_=True, allow_empty=True)

Check warning on line 277 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L277

Added line #L277 was not covered by tests

# HEAD~ is the state before the last commit
changed_files = git_cli.diffed_files(context.local_clone_dir, "HEAD~")

Check warning on line 280 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L280

Added line #L280 was not covered by tests

recipe_dir = context.local_clone_dir / "recipe"

Check warning on line 282 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L282

Added line #L282 was not covered by tests

nontrivial_changes = any(

Check warning on line 284 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L284

Added line #L284 was not covered by tests
not file.is_relative_to(recipe_dir) and not file.name.startswith("README")
for file in changed_files
)

return _RerenderInfo(nontrivial_changes=nontrivial_changes)

Check warning on line 289 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L289

Added line #L289 was not covered by tests


def run_with_tmpdir(
context: FeedstockContext,
migrator: Migrator,
Expand Down Expand Up @@ -336,6 +402,20 @@
raise_commit_errors=migrator.allow_empty_commits and not rerender,
)

if rerender:

Check warning on line 405 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L405

Added line #L405 was not covered by tests
# for version migrations, without check solvable or automerge, we can suppress rerender errors
suppress_errors = (

Check warning on line 407 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L407

Added line #L407 was not covered by tests
is_version_migration
and not context.check_solvable
and not context.automerge
)

rerender_info = _run_rerender(

Check warning on line 413 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L413

Added line #L413 was not covered by tests
git_backend.cli, context, suppress_errors=suppress_errors
)
else:
rerender_info = _RerenderInfo(nontrivial_changes=False)

Check warning on line 417 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L417

Added line #L417 was not covered by tests

# 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
Expand All @@ -345,73 +425,13 @@

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"]

Check warning on line 429 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L429

Added line #L429 was not covered by tests
else:
has_automerge = getattr(
migrator, "automerge", False
) and feedstock_automerge in [True, "migration"]
has_automerge = getattr(migrator, "automerge", False) and context.automerge in [

Check warning on line 431 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L431

Added line #L431 was not covered by tests
True,
"migration",
]

migrator_check_solvable = getattr(migrator, "check_solvable", True)
feedstock_check_solvable = get_keys_default(
Expand All @@ -432,7 +452,7 @@
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:
Expand Down Expand Up @@ -505,7 +525,7 @@
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
Expand Down Expand Up @@ -538,15 +558,10 @@
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:

Check warning on line 561 in conda_forge_tick/auto_tick.py

View check run for this annotation

Codecov / codecov/patch

conda_forge_tick/auto_tick.py#L561

Added line #L561 was not covered by tests
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,
)

Expand Down
29 changes: 29 additions & 0 deletions conda_forge_tick/contexts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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]:
"""
Expand Down
23 changes: 22 additions & 1 deletion conda_forge_tick/git_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions conda_forge_tick/migrators_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions tests/test_contexts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading