-
Notifications
You must be signed in to change notification settings - Fork 90
Fix dep_updates for v1 recipes using python_min #5306
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
base: main
Are you sure you want to change the base?
Fix dep_updates for v1 recipes using python_min #5306
Conversation
This test case reproduces the issue where the dep_updates migrator fails for v1 recipes when grayskull generates a recipe with `skip: match(python, ...)` but the feedstock's variant config only has `python_min` (not `python`). The test adds: - dominodatalab feedstock as a git submodule (pinned to v1.4.7) - Test case that validates version update from 1.4.7 to 2.0.0 - New `assert_pr_body_not_contains` helper method to check PR body - Assertion that the PR body doesn't contain the dep_updates error message The test currently fails, demonstrating the bug where rattler-build skips all variants when the `python` variable is not in the variant config. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This test reproduces the issue where get_grayskull_comparison fails for v1 recipes when grayskull generates `skip: match(python, "<3.10")` but the variant config only has `python_min` (not `python`). The test is marked as xfail since it currently fails - rattler-build skips all variants when the `python` variable is not set in the variant config. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The xfail marker was causing pytest to consider the failing test as "passing" (expected failure). Removing it ensures the test actually fails in CI until the bug is fixed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace `match(python, ...)` with `match(python_min, ...)` in grayskull-generated v1 recipes. This fixes the dep_updates migrator for feedstocks using CFEP-25's python_min variable. Grayskull generates skip conditions like `skip: match(python, "<3.10")` but v1 recipe variant configs only define `python_min`, not `python`. This caused rattler-build to skip all variants with: "Failed to render recipe YAML! No output recipes found!" See: conda/grayskull#574 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add mock endpoint for version-specific PyPI API (/pypi/dominodatalab/2.0.0/json) - Include 'urls' field in main PyPI endpoint for grayskull sdist discovery - Add spdx.org/* to transparent URLs (grayskull license discovery) - Store full PyPI metadata response for reproducible testing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
12a2004 to
09ca7cc
Compare
Add documentation to help Claude Code understand the codebase structure, common commands, and integration test setup. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@wolfv rattler build should define python, right? Is this an issue somewhere in rattler-build or smithy maybe? |
…egration-test # Conflicts: # .gitmodules
conda_forge_tick/update_deps.py
Outdated
| # Grayskull generates `match(python, ...)` for noarch recipes, but v1 recipes | ||
| # use `python_min` in their variant configs (CFEP-25). Replace to make the | ||
| # recipe renderable. See: https://github.com/conda/grayskull/issues/574 | ||
| recipe_str = recipe_str.replace("match(python,", "match(python_min,") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This filter is rather broad and I am concerned it might sweep up other parts of the recipe with this syntax. I suggested we write a more targeted edit that ensures the change is in a noarch: python recipe.
Guard the match(python, ...) to match(python_min, ...) replacement with a check for package_is_noarch to avoid unintended changes to arch-specific recipes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename CLAUDE.md to AGENTS.md and create symlink for backwards compat - Make content agent-agnostic (remove Claude Code-specific references) - Remove "(recommended for development)" from environment.yml option - Use local conda-lock.yml instead of wget from GitHub - Remove unmaintained test feedstocks list Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5306 +/- ##
==========================================
- Coverage 80.62% 80.60% -0.02%
==========================================
Files 144 144
Lines 17005 17013 +8
==========================================
+ Hits 13710 13714 +4
- Misses 3295 3299 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| # Grayskull generates `match(python, ...)` for noarch recipes, but v1 recipes | ||
| # use `python_min` in their variant configs (CFEP-25). Replace to make the | ||
| # recipe renderable. See: https://github.com/conda/grayskull/issues/574 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue is now marked as resolved. Let's test this PR without the change below to see if it works now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, that's surprising! It had been open for more than a year.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@beckermr, we have to bump grayskull to v3.1.0:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grayskull is at 3.1.0 in the lockfile, so something is still broken or something else broke.
|
This error looks odd? |
|
/relock-conda |
Co-authored-by: Matthew R. Becker <beckermr@users.noreply.github.com>
|
@janjagusch We need to diagnose the error here, update the comment in the code, and report it upstream to grayskull before we can merge this. Do you know what is still broken in the generated recipe even in 3.1.0? |
Sure, I can report that the Grayskull patch seemingly doesn't work for us. It might take me a few days before I can get back to it, though. :) |
Summary
Fix the dep_updates migrator failing for v1 recipes that use
python_min(CFEP-25).Root cause: Grayskull generates
skip: match(python, "<3.10")for noarch recipes, but v1 recipe variant configs only definepython_min, notpython. When rattler-build tries to render the recipe, all variants are skipped becausepythonis undefined.Fix: Replace
match(python,withmatch(python_min,in grayskull-generated v1 recipes before rendering.Changes
_make_grayskull_recipe_v1to replacematch(python,withmatch(python_min,test_get_grayskull_comparison_v1_python_min_mismatchassert_pr_body_not_containshelper method for integration testsTest Evidence
Successful version update PR created by the bot during integration testing:
janjagusch-conda-forge-bot-staging/dominodatalab-feedstock#28
Related
python_minconda/grayskull#574Closes #5123
Unrelated addition: This PR also adds
CLAUDE.mdto provide guidance for Claude Code when working with this repository.🤖 Generated with Claude Code