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

Recipe modifications #47

Merged
merged 9 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
720 changes: 493 additions & 227 deletions pixi.lock

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ pre-commit = ">=3.7.1,<4"
pre-commit-hooks = ">=4.6.0,<5"
typos = ">=1.23.1,<2"
mypy = ">=1.10.1,<2"
types-pyyaml = ">=6.0.12.20240311,<6.0.13"
ruff = ">=0.5.0,<0.6"

[feature.lint.tasks]
Expand All @@ -43,6 +42,8 @@ pre-commit-run = "pre-commit run"

[feature.type-checking.dependencies]
mypy = ">=1.10.1,<2"
types-requests = ">=2.32.0.20240712,<3"
types-pyyaml = ">=6.0.12.20240311,<6.0.13"

[feature.type-checking.tasks]
type-check = "mypy src"
Expand Down
142 changes: 142 additions & 0 deletions src/rattler_build_conda_compat/modify_recipe.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
from __future__ import annotations

import hashlib
import io
import re
from typing import TYPE_CHECKING, Any, Generator

import requests
from ruamel.yaml import YAML

if TYPE_CHECKING:
from pathlib import Path

yaml = YAML()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt about extracting YAML loader configuration in one place so it can be used by other loaders?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I also want to get rid of any PyYAML and just use ruamel.YAML everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, that's the next PR that I wanted to send, so let's maybe wait until then?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me

yaml.preserve_quotes = True


def _update_build_number_in_context(recipe: dict[str, Any], new_build_number: int) -> bool:
for key in recipe.get("context", {}):
if key.startswith("build_") or key == "build":
recipe["context"][key] = new_build_number
return True
return False


def _update_build_number_in_recipe(recipe: dict[str, Any], new_build_number: int) -> bool:
is_modified = False
if "build" in recipe and "number" in recipe["build"]:
recipe["build"]["number"] = new_build_number
is_modified = True

if "outputs" in recipe:
for output in recipe["outputs"]:
if "build" in output and "number" in output["build"]:
output["build"]["number"] = new_build_number
is_modified = True

return is_modified


def update_build_number(file: Path, new_build_number: int) -> str:
wolfv marked this conversation as resolved.
Show resolved Hide resolved
# This function should be called to update the build number of the recipe
# in the meta.yaml file.
wolfv marked this conversation as resolved.
Show resolved Hide resolved
with file.open("r") as f:
data = yaml.load(f)
Comment on lines +63 to +64
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesnt it make more sense to leave the loading of the yaml to the caller? I can imagine that if we have more modification functions they will all require loading the yaml.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and no - we do want to minimally change the YAML so I think that should all be handled by us (leave comments in tact, order, formatting, ...).

build_number_modified = _update_build_number_in_context(data, new_build_number)
if not build_number_modified:
build_number_modified = _update_build_number_in_recipe(data, new_build_number)
wolfv marked this conversation as resolved.
Show resolved Hide resolved

with io.StringIO() as f:
yaml.dump(data, f)
return f.getvalue()


class CouldNotUpdateVersionError(Exception):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt about moving errors in separate modules?

NO_CONTEXT = "Could not find context in recipe"
NO_VERSION = "Could not find version in recipe context"

def __init__(self, message: str = "Could not update version") -> None:
self.message = message
super().__init__(self.message)


class Hash:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe let's move it in separate module? like hash.py for example?

def __init__(self, hash_type: str, hash_value: str) -> None:
self.hash_type = hash_type
self.hash_value = hash_value

def __str__(self) -> str:
return f"{self.hash_type}: {self.hash_value}"


def has_jinja_version(url: str) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this also can be moved in utils.py as it seems something useful potentially to have for other modules

"""Check if the URL has a jinja `${{ version }}` in it."""
pattern = r"\${{\s*version"
return re.search(pattern, url) is not None


def flatten_all_sources(sources: list[dict[str, Any]]) -> Generator[dict[str, Any], None, None]:
wolfv marked this conversation as resolved.
Show resolved Hide resolved
"""
Flatten all sources in a recipe. This is useful when a source is defined
with an if/else statement. Will yield both branches of the if/else
statement if it exists.
"""
for source in sources:
if "if" in source:
yield source["then"]
if "else" in source:
yield source["else"]
else:
yield source


def update_hash(source: dict[str, Any], url: str, hash_type: Hash | None) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can try to annotate it as a typeddict total = False where we expect sha256 and md5 key to be present

# kick out any hash that is not the one we are updating
wolfv marked this conversation as resolved.
Show resolved Hide resolved
potential_hashes = {"sha256", "md5"}
for key in potential_hashes:
if key in source:
del source[key]
if hash_type is not None:
source[hash_type.hash_type] = hash_type.hash_value
else:
# download and hash the file
hasher = hashlib.sha256()
with requests.get(url, stream=True, timeout=100) as r:
for chunk in r.iter_content(chunk_size=4096):
hasher.update(chunk)
source["sha256"] = hasher.hexdigest()


def update_version(file: Path, new_version: str, hash_type: Hash | None) -> str:
# This function should be called to update the version of the recipe
# in the meta.yaml file.

with file.open("r") as f:
data = yaml.load(f)
Comment on lines +140 to +141
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, maybe it would make sense to have a variant that takes the loaded dictionary as input.


if "context" not in data:
raise CouldNotUpdateVersionError(CouldNotUpdateVersionError.NO_CONTEXT)
if "version" not in data["context"]:
raise CouldNotUpdateVersionError(CouldNotUpdateVersionError.NO_VERSION)

data["context"]["version"] = new_version

sources = data.get("source", [])
if isinstance(sources, dict):
sources = [sources]

for source in flatten_all_sources(sources):
if has_jinja_version(source.get("url", "")):
# render the whole URL and find the hash
urls = source["url"]
if not isinstance(urls, list):
urls = [urls]

rendered_url = urls[0].replace("${{ version }}", new_version)

update_hash(source, rendered_url, hash_type)

with io.StringIO() as f:
yaml.dump(data, f)
return f.getvalue()
5 changes: 5 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
import pytest


@pytest.fixture()
def data_dir() -> Path:
return Path(__file__).parent / "data"


@pytest.fixture()
def python_recipe(tmpdir: Path) -> str:
recipe_dir = tmpdir / "recipe"
Expand Down
10 changes: 10 additions & 0 deletions tests/data/build_number/test_1/expected.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# set the build number to something
context:
build: 0

package:
name: recipe_1
version: "0.1.0"

build:
number: ${{ build }}
10 changes: 10 additions & 0 deletions tests/data/build_number/test_1/recipe.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# set the build number to something
context:
build: 123

package:
name: recipe_1
version: "0.1.0"

build:
number: ${{ build }}
11 changes: 11 additions & 0 deletions tests/data/build_number/test_2/expected.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# set the build number to something
package:
name: recipe_1
version: "0.1.0"

# set the build number to something directly in the recipe text
build:
number: 0

source:
- url: foo
11 changes: 11 additions & 0 deletions tests/data/build_number/test_2/recipe.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# set the build number to something
package:
name: recipe_1
version: "0.1.0"

# set the build number to something directly in the recipe text
build:
number: 321

source:
- url: foo
11 changes: 11 additions & 0 deletions tests/data/version/future_tests/r1.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
context:
name: pytest-aio
version: 1.9.0

package:
name: ${{ name|lower }}
version: ${{ version }}

source:
url: https://pypi.io/packages/source/${{ name[0] }}/${{ name }}/${{ name.replace('-', '_') }}-${{ version }}.tar.gz
sha256: aa72e6ca4672b7f5a08ce44e7c6254dca988d3d578bf0c9485a47c3bff393ac1
14 changes: 14 additions & 0 deletions tests/data/version/future_tests/r2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
context:
version: "1.1-30"
posix: ${{ 'm2' if win else '' }}
native: ${{ 'm2w64' if win else '' }}

package:
name: r-systemfit
version: ${{ version|replace("-", "_") }}

source:
url:
- ${{ cran_mirror }}/src/contrib/systemfit_{{ version }}.tar.gz
- ${{ cran_mirror }}/src/contrib/Archive/systemfit/systemfit_{{ version }}.tar.gz
sha256: 5994fbb81f1678325862414f58328cdc2c46d47efa1f23218e9416a4da431ce2
12 changes: 12 additions & 0 deletions tests/data/version/test_1/expected.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
context:
name: xtensor
version: "0.25.0"


package:
name: ${{ name|lower }}
version: ${{ version }}

source:
url: https://github.com/xtensor-stack/xtensor/archive/${{ version }}.tar.gz
sha256: 32d5d9fd23998c57e746c375a544edf544b74f0a18ad6bc3c38cbba968d5e6c7
12 changes: 12 additions & 0 deletions tests/data/version/test_1/recipe.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
context:
name: xtensor
version: "0.23.5"


package:
name: ${{ name|lower }}
version: ${{ version }}

source:
url: https://github.com/xtensor-stack/xtensor/archive/${{ version }}.tar.gz
sha256: 0811011e448628f0dfa6ebb5e3f76dc7bf6a15ee65ea9c5a277b12ea976d35bc
15 changes: 15 additions & 0 deletions tests/data/version/test_2/expected.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
context:
name: xtensor
version: "0.25.0"


package:
name: ${{ name|lower }}
version: ${{ version }}

source:
# please update the version here.
- if: target_platform == linux-64
then:
url: https://github.com/xtensor-stack/xtensor/archive/${{ version }}.tar.gz
sha256: 32d5d9fd23998c57e746c375a544edf544b74f0a18ad6bc3c38cbba968d5e6c7
15 changes: 15 additions & 0 deletions tests/data/version/test_2/recipe.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
context:
name: xtensor
version: "0.23.5"


package:
name: ${{ name|lower }}
version: ${{ version }}

source:
# please update the version here.
- if: target_platform == linux-64
then:
url: https://github.com/xtensor-stack/xtensor/archive/${{ version }}.tar.gz
sha256: 0811011e448628f0dfa6ebb5e3f76dc7bf6a15ee65ea9c5a277b12ea976d35bc
23 changes: 23 additions & 0 deletions tests/test_recipe_modification.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
from pathlib import Path

from rattler_build_conda_compat.modify_recipe import update_build_number, update_version


def test_build_number_mod(data_dir: Path) -> None:
tests = data_dir / "build_number"
result = update_build_number(tests / "test_1/recipe.yaml", 0)
expected = tests / "test_1/expected.yaml"
assert result == expected.read_text()

result = update_build_number(tests / "test_2/recipe.yaml", 0)
expected = tests / "test_2/expected.yaml"
assert result == expected.read_text()


def test_version_mod(data_dir: Path) -> None:
tests = data_dir / "version"
test_recipes = tests.glob("**/recipe.yaml")
for recipe in test_recipes:
result = update_version(recipe, "0.25.0", None)
expected = recipe.parent / "expected.yaml"
assert result == expected.read_text()