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

Switch to rattler-build #303

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

pavelzw
Copy link
Member

@pavelzw pavelzw commented Jan 17, 2025

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

This will hopefully increase the iteration speed debugging the failing version updates since we don't need to wait ages for conda-build anymore

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 17, 2025

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe/recipe.yaml) and found some lint.

Here's what I've got...

For recipe/recipe.yaml:

  • ❌ Recipe name has invalid characters. only lowercase alpha, numeric, underscores, hyphens and dots allowed

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12949477992. Examine the logs at this URL for more detail.

@pavelzw
Copy link
Member Author

pavelzw commented Jan 18, 2025

❌ Recipe name has invalid characters. only lowercase alpha, numeric, underscores, hyphens and dots allowed

xref conda-forge/conda-smithy#2224

Comment on lines +58 to +60
- if: build_platform != target_platform and target_platform == "win-64"
then:
- mingw-w64-ucrt-x86_64-headers-git
Copy link
Member

Choose a reason for hiding this comment

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

You should not add this. We doing an MSVC based build and MinGW is a whole different toolchain

Copy link
Contributor

@ytausch ytausch left a comment

Choose a reason for hiding this comment

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

I checked the recipe for compatibility with the autotick-bot.

Updating the recipe after my suggested changes here should work, missing unit tests to the bot were contributed here: regro/cf-scripts#3605

source:
- if: polars_variant == 'polars'
then:
url: https://pypi.org/packages/source/p/polars/${{ polars_variant | replace('-', '_') }}-${{ version }}.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

The autotick-bot cannot read variant variables here.

Suggested change
url: https://pypi.org/packages/source/p/polars/${{ polars_variant | replace('-', '_') }}-${{ version }}.tar.gz
url: https://pypi.org/packages/source/p/polars/polars-${{ version }}.tar.gz

Copy link
Contributor

Choose a reason for hiding this comment

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

the pathway for adding support for your original proposal is here: prefix-dev/rattler-build-conda-compat#66

sha256: e8e9e3156fae02b58e276e5f2c16a5907a79b38617a9e2d731b533d87798f451
- if: polars_variant == 'polars-lts-cpu'
then:
url: https://pypi.org/packages/source/p/${{ polars_variant }}/${{ polars_variant | replace('-', '_') }}-${{ version }}.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
url: https://pypi.org/packages/source/p/${{ polars_variant }}/${{ polars_variant | replace('-', '_') }}-${{ version }}.tar.gz
url: https://pypi.org/packages/source/p/polars-lts-cpu/polars_lts_cpu-${{ version }}.tar.gz

see above

sha256: f8770fe1a752f60828ec73e6215c7dadcb2badd1f34dcb1def7a0f4ca0ac36f8
- if: polars_variant == 'polars-u64-idx'
then:
url: https://pypi.org/packages/source/p/${{ polars_variant }}/${{ polars_variant | replace('-', '_') }}-${{ version }}.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
url: https://pypi.org/packages/source/p/${{ polars_variant }}/${{ polars_variant | replace('-', '_') }}-${{ version }}.tar.gz
url: https://pypi.org/packages/source/p/polars-u64-idx/polars_u64_idx-${{ version }}.tar.gz

see above

Comment on lines +69 to +71
-c conda-forge/label/rust_dev \
-c conda-forge/label/cargo-patch-dev \
-c conda-forge \
Copy link
Member Author

Choose a reason for hiding this comment

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

@xhochy
Copy link
Member

xhochy commented Jan 24, 2025

I would like to trim down the build matrix to a single Python version here but sadly conda-smithy rerender doesn't work. I can see some differences locally but would like to verify that they pop up in CI, too, before I dive in too deep.

@xhochy
Copy link
Member

xhochy commented Jan 24, 2025

@pavelzw Were you able to reproduce this locally? For me, even the osx-64 -> osx-arm64 builds pass on my Intel Mac.

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.

4 participants