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

support variant variables in sources #66

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ytausch
Copy link

@ytausch ytausch commented Jan 22, 2025

This PR is intended as more of a question than a concrete proposal.

Currently, render_all_sources does not render variables in source URLs defined by variant variables (as opposed to variables defined in the source section). However, rattler-build supports variant variables in the source section (see, e.g., https://github.com/conda-forge/polars-feedstock/pull/303/checks?check_run_id=35876450902).

Does it make sense to support variant variables here too? This would enable the autotick-bot to parse such recipes.

@@ -138,7 +138,7 @@ def render(template: str | list[str], context: dict[str, str]) -> str | list[str

context = recipe.get("context", {})
# render out the context section and retrieve dictionary
context_variables = load_recipe_context(context, env)
context_variables = combination | load_recipe_context(context, env)
Copy link
Author

Choose a reason for hiding this comment

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

This means that the variant variables are passed as context to the Source object below. Is this correct?

@@ -125,7 +125,7 @@ def render(template: str | list[str], context: dict[str, str]) -> str | list[str
if isinstance(template, list):
return [cast(str, render(t, context)) for t in template]
template = env.from_string(template)
return template.render(context_variables)
return template.render(context)
Copy link
Author

Choose a reason for hiding this comment

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

I think this was actually a bug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant