Align conda-forge recipe with normal recipe#285
Conversation
d5aa34a to
81591cb
Compare
There was a problem hiding this comment.
Pull request overview
Aligns the conda-forge recipe (conda-recipe-cf) metadata and CI behavior closer to the primary conda recipe (conda-recipe) to standardize versioning/build metadata and dependencies.
Changes:
- Update
conda-recipe-cf/meta.yamlto useGIT_DESCRIBE_TAG/GIT_DESCRIBE_NUMBER, addscript_env, and align several build/host/run requirements. - Adjust recipe metadata text (summary/description) and simplify import checks under
test:. - Remove the explicit MSVC setup step from the Windows conda-package workflows.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
conda-recipe-cf/meta.yaml |
Switch to git-describe versioning/build numbering; add build env forwarding and align dependency list/metadata. |
.github/workflows/conda-package.yml |
Remove MSVC environment setup step before Windows conda build. |
.github/workflows/conda-package-cf.yml |
Remove MSVC environment setup step before Windows conda build (NumPy 2.x matrix). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| script_env: | ||
| - WHEELS_OUTPUT_FOLDER |
There was a problem hiding this comment.
build.script_env now forwards WHEELS_OUTPUT_FOLDER, but the conda-forge recipe’s build scripts (conda-recipe-cf/build.sh and conda-recipe-cf/bld.bat) don’t reference this variable, so this setting is currently dead/unused and may confuse future maintainers. Either remove WHEELS_OUTPUT_FOLDER from script_env for this recipe or update the conda-forge build scripts to honor it (similar to conda-recipe/).
| script_env: | |
| - WHEELS_OUTPUT_FOLDER |
There was a problem hiding this comment.
It sounds as a valid comment. We don't pass WHEELS_OUTPUT_FOLDER env to the scripts. Probably we should remove that env here.
a8ebfdc to
888099b
Compare
e07ba98 to
11b97d9
Compare
| imports: | ||
| - mkl_fft | ||
| - mkl_fft.interfaces | ||
| - mkl_fft.interfaces.numpy_fft |
There was a problem hiding this comment.
It's present in the feedstock repo. Why do we plan to remove that?
| script_env: | ||
| - WHEELS_OUTPUT_FOLDER |
There was a problem hiding this comment.
It sounds as a valid comment. We don't pass WHEELS_OUTPUT_FOLDER env to the scripts. Probably we should remove that env here.
| c_stdlib: # [linux] | ||
| - sysroot # [linux] | ||
| c_stdlib_version: # [linux] | ||
| - '2.28' # [linux] |
There was a problem hiding this comment.
It's set to 2.17 in the feedstock. But I wonder if we need to align here...
| cxx_compiler: # [win] | ||
| - vs2022 # [win] | ||
| c_compiler: # [win] | ||
| - vs2022 # [win] |
There was a problem hiding this comment.
As an option we can list all python versions:
| - vs2022 # [win] | |
| - vs2022 # [win] | |
| numpy: | |
| - '2' | |
| python: | |
| - 3.10.* *_cpython | |
| - 3.11.* *_cpython | |
| - 3.12.* *_cpython | |
| - 3.13.* *_cp313 | |
| - 3.14.* *_cp314 |
it would mean if we run conda build --no-test -c conda-forge --override-channels conda-recipe-cf command (without numpy and python options passed) and remove strategy matrix from build jobs, each Linux and Windows will build mkl_fft packages with all supported python versions at once.
And then we will be able to remove python-gil from conda-recipe-cf/meta.yaml aligning with the feedstock in more details.
There was a problem hiding this comment.
Also, the feedstock adds:
pin_run_as_build:
python:
min_pin: x.x
max_pin: x.xwhich we would probably able to do as well.
| - name: Store conda paths as envs | ||
| shell: bash -l {0} | ||
| run: | | ||
| echo "CONDA_BLD=$CONDA/conda-bld/win-64/" | tr "\\\\" '/' >> "$GITHUB_ENV" |
There was a problem hiding this comment.
we should use proper CONDA_BLD value rather then installing conda-build to the base env
| echo "CONDA_BLD=$CONDA_PREFIX/conda-bld/win-64/" | tr "\\\\" '/' >> "$GITHUB_ENV" |
| activate-environment: ${{ env.TEST_ENV_NAME }} | ||
| python-version: ${{ matrix.python }} | ||
| channels: conda-forge | ||
| conda-remove-defaults: 'true' |
There was a problem hiding this comment.
conda-remove-defaults: still used in .github/workflows/build_pip.yaml, but can be removed
This PR proposes to align the
meta.yamlinconda-recipe-cfwithconda-recipeAlso removes MSVC setup step, instead using conda build configs to set VS version. This exposed mistake in conda build config, which was set to vs2017