Skip to content

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Mar 7, 2025

Description:

Add support for v1 recipes in CrossRBaseMigrator. Here the logic
made it easy to output a single combined condition:

    - if: build_platform != target_platform
      then:
        - cross-r-base ${{ r_base }}
        - r-rlang

Checklist:

  • Pydantic model updated or no update needed

Cross-refs, links to issues, etc:

Part of bug #3642

mgorny added 2 commits March 7, 2025 17:39
`CrossRBaseMigrator` is not actually using anything
from `CrossCompilationMigratorBase`, besides `post_migration` attribute,
and calling the superclass `filter()` method yields incorrect results.
Make the class inherit `MiniMigrator` instead to make it more consistent
with other minimigrators.
Add support for v1 recipes in `CrossRBaseMigrator`.  Here the logic
made it easy to output a single combined condition:

```yaml
    - if: build_platform != target_platform
      then:
        - cross-r-base ${{ r_base }}
        - r-rlang
```

Part of bug regro#3642
Comment on lines 356 to 361
[
" " * nspaces
+ "- if: build_platform != target_platform\n",
" " * nspaces + " then:\n",
" " * nspaces + " - cross-r-base ${{ r_base }}\n",
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWICS we could just append all the lines as a single element here, but I've figured out that since it's a list of lines by design, it's cleaner to add them separately.

Copy link

codecov bot commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.03%. Comparing base (91cc9b6) to head (342e595).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
conda_forge_tick/migrators/cross_compile.py 93.75% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3836   +/-   ##
=======================================
  Coverage   77.02%   77.03%           
=======================================
  Files         138      138           
  Lines       15384    15394   +10     
=======================================
+ Hits        11850    11859    +9     
- Misses       3534     3535    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@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.

I think we want the flaky decorator inside the pytest parameterize decorator again.

@mgorny
Copy link
Contributor Author

mgorny commented Mar 7, 2025

I think we want the flaky decorator inside the pytest parameterize decorator again.

Sorry about that. Remember about it for Python, failed to remember here.

@beckermr
Copy link
Contributor

beckermr commented Mar 7, 2025

We can merge this despite the target_platform versus host_platform issue since we'll need to change that everywhere including rattler later, and this won't happen for a while.

Is that OK @isuruf?

@isuruf
Copy link
Member

isuruf commented Mar 17, 2025

I don't see a reason to merge this as it is. It's a very simple change that I don't think requires a lot of work.

@mgorny
Copy link
Contributor Author

mgorny commented Mar 17, 2025

Okay, I've just checked rattler-build and host_platform seems to work as well. I'll update both pull requests.

@mgorny
Copy link
Contributor Author

mgorny commented Mar 24, 2025

@beckermr, gentle ping. Is there anything left blocking this PR?

@beckermr beckermr enabled auto-merge March 25, 2025 14:25
@beckermr beckermr added this pull request to the merge queue Mar 25, 2025
Merged via the queue into regro:main with commit 81f946a Mar 25, 2025
9 checks passed
@mgorny mgorny deleted the cross-rbase-v1 branch March 25, 2025 15:46
@mgorny
Copy link
Contributor Author

mgorny commented Mar 25, 2025

Thanks!

@beckermr beckermr mentioned this pull request Apr 7, 2025
33 tasks
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.

3 participants