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

Fix false positive when v1 recipe name is non-context variable #2248

Merged
merged 2 commits into from
Feb 25, 2025

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Feb 25, 2025

Checklist

  • Added a news entry
  • Regenerated schema JSON if schema altered (python conda_smithy/schema.py)

Fixes #2224

Skip checking v1 recipe name if it contains unresolved variables, in order to fix false positives when a variable
from conda_build_config.yaml is used.

Iterating over all possible variants and verifying the resulting package names is unlikely to be worth the extra complexity. This also roughly matches the linter behavior for v0 recipes, where {{ var_name }} is converted into var_name, and therefore is never properly linted anyway.

Skip checking v1 recipe name if it contains unresolved variables,
in order to fix false positives when a variable
from `conda_build_config.yaml` is used.

Iterating over all possible variants and verifying the resulting package
names is unlikely to be worth the extra complexity.  This also roughly
matches the linter behavior for v0 recipes, where `{{ var_name }}` is
converted into `var_name`, and therefore is never properly linted
anyway.

Fixes conda-forge#2224
@mgorny mgorny marked this pull request as ready for review February 25, 2025 16:15
@mgorny mgorny requested a review from a team as a code owner February 25, 2025 16:15
Copy link
Member

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

Why can't we load the variables? For v1 this should be very fast. For v0 I agree this would be too slow.

@mgorny
Copy link
Contributor Author

mgorny commented Feb 25, 2025

Why can't we load the variables? For v1 this should be very fast. For v0 I agree this would be too slow.

By "complexity", I meant the coding effort to make that happen.

Are you suggesting we re-lint the recipe for all variants, or just special-case the package name? I suppose the former could make sense, though we'd probably need to filter out duplicates somehow, and that probably isn't going to be easy.

@beckermr
Copy link
Member

Package name but ok. I'm not going to stand in the way here.

@beckermr beckermr merged commit fcd784c into conda-forge:main Feb 25, 2025
2 checks passed
@mgorny mgorny deleted the recipe-name-var-false-positive branch February 25, 2025 19:54
@mgorny
Copy link
Contributor Author

mgorny commented Feb 25, 2025

Thanks. I mean, if you insist I can look into doing it, but it feels a bit, well, inefficient, to call rattler-build to render all the variants from within an individual check.

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.

False positive in lint_recipe_name when using jinja in package.name
2 participants