conda-build
conda-build copied to clipboard
Stdlib: c_stdlib_version falsely determined as unset
While trying to rerender https://github.com/conda-forge/ctng-compiler-activation-feedstock with the newest conda-build, I'm running into a problem that's seemingly related to the new stdlib-jinja. However, that recipe does not use this function at all...?
The error looks as follows:
(builder) [...]\conda-forge\ctng-compiler-activation-feedstock>conda smithy rerender
INFO:conda_smithy.configure_feedstock:README rendering is skipped
INFO:conda_smithy.configure_feedstock:__pycache__ rendering is skipped
WARNING: Setting build platform. This is only useful when pretending to be on another platform, such as for rendering necessary dependencies on a non-native platform. I trust that you know what you're doing.
WARNING: Setting build arch. This is only useful when pretending to be on another arch, such as for rendering necessary dependencies on a non-native arch. I trust that you know what you're doing.
WARNING: No numpy version specified in conda_build_config.yaml. Falling back to default numpy value of 1.23
Adding in variants from internal_defaults
Adding in variants from C:\Users\[...]\AppData\Local\Temp\conda-smithy\conda_build_config.yaml
Adding in variants from C:\Users\[...]\dev\conda-forge\ctng-compiler-activation-feedstock\recipe\conda_build_config.yaml
Adding in variants from argument_variants
Traceback (most recent call last):
File "E:\miniforge\envs\builder\Scripts\conda-smithy-script.py", line 9, in <module>
sys.exit(main())
^^^^^^
File "E:\miniforge\envs\builder\Lib\site-packages\conda_smithy\cli.py", line 724, in main
args.subcommand_func(args)
File "E:\miniforge\envs\builder\Lib\site-packages\conda_smithy\cli.py", line 584, in __call__
self._call(args, tmpdir)
File "E:\miniforge\envs\builder\Lib\site-packages\conda_smithy\cli.py", line 589, in _call
configure_feedstock.main(
File "E:\miniforge\envs\builder\Lib\site-packages\conda_smithy\configure_feedstock.py", line 2697, in main
render_azure(env, config, forge_dir, return_metadata=True)
File "E:\miniforge\envs\builder\Lib\site-packages\conda_smithy\configure_feedstock.py", line 1664, in render_azure
return _render_ci_provider(
^^^^^^^^^^^^^^^^^^^^
File "E:\miniforge\envs\builder\Lib\site-packages\conda_smithy\configure_feedstock.py", line 828, in _render_ci_provider
) = conda_build.variants.get_package_combined_spec(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "E:\miniforge\envs\builder\Lib\site-packages\conda_build\variants.py", line 665, in get_package_combined_spec
validate_spec(f, spec)
File "E:\miniforge\envs\builder\Lib\site-packages\conda_build\variants.py", line 192, in validate_spec
raise ValueError(
ValueError: Variant configuration errors in C:\Users\[...]\AppData\Local\Temp\conda-smithy\conda_build_config.yaml:
zip_key entry c_stdlib_version in group frozenset({'c_stdlib_version', 'c_stdlib'}) does not have any settings
Aside from the recipe not using {{ stdlib("c") }}
, the specification is also definitely present already in conda-forge's global pinning.
I've tried monkey-patching my conda-build to contain #5195, as well as adding the stdlib-config to the local CBC, and even setting some value for the windows c_stdlib_version
(that's not set in the global pinning because it shouldn't be necessary). In all cases, I get the same error.
The error appears in https://github.com/conda/conda-build/blob/38fdc15d81bbdc9bf9377385f668ba9d4de5dfcd/conda_build/variants.py#L150-L163
but I can't quite yet tell why that would be the case.
CC @mbargull @beckermr
Hi @h-vetinari , I've added this to the 24.3.x release milestone list; would you consider this issue a blocker (i.e. conda-build should be considered broken or will significantly block conda-forge-related work etc.) if this isn't fixed in time to be included in the March release?
It depends a bit on the fallout. It's strange to me that a recipe that's not using any of the new functionality would stumble while being rendered - for that recipe, the outcome is quite bad, because we cannot change configuration etc., without doing very error-prone manual surgery.
So far I've just encountered one that failed to rerender, which I wouldn't consider a blocker. But if it's more widespread, then yes.
In any case, in the context of some urgency for the upgrades of glibc and macos deployment target this year, any bugs in the stdlib functionality have high priority to me, especially if the lead times accumulate (but I'll admit that my priorities are perhaps not 100% representative of everyone else 😅)
I doubt this is a one-off case and I think this is a blocker for the release. My worry is about the conda-build test suite. How did this bug leak through?
This is not an issue with conda-build. The conda-forge cbc.yml is invalid for linux-s390x
.
Using the following recipe/conda_build_config.yaml
:
c_stdlib:
- sysroot # [linux]
- macosx_deployment_target # [osx]
- vs # [win]
c_stdlib_version: # [unix]
- 2.12 # [linux64]
- 2.17 # [aarch64 or ppc64le]
- 10.9 # [osx and x86_64]
- 11.0 # [osx and arm64]
zip_keys:
- # [unix]
- c_stdlib # [unix]
- c_stdlib_version # [unix]
And this recipe/meta.yaml
:
package:
name: conda-forge-cbc
version: 1.0
build:
noarch: True
script:
- echo hello world
We can reproduce the error:
$ CONDA_SUBDIR=linux-s390x conda build recipe/
...
ValueError: Variant configuration errors in /Users/kodegard/scratch/conda-forge-cbc/recipe/conda_build_config.yaml:
zip_key entry c_stdlib_version in group frozenset({'c_stdlib', 'c_stdlib_version'}) does not have any settings
Versus:
$ CONDA_SUBDIR=linux-64 conda build recipe/
...
Thanks for the response @kenodegard!
So there's 3-4 different things happening here:
- a recipe that didn't use anything related to the new stdlib jinja broke. In conda-forge we'll be relatively fine (because closely involved with this change), but people in other channels or proprietary settings might be very confused if their recipes break, and the solution is not obvious (add
c_stdlib{,_version}
to CBC) - the fact that I missed the config for s390x - that's totally on me. In my defence, s390x is so rare that I forget it's presence - that failure mode didn't even occurr to me, sorry! I guess we'll go add something for it to the global pinning (or at least the CBC for the compiler feedstock).
- even when adding config for s390x, the rerender still fails, because now windows only has a stdlib but not a version. This should be made more lenient I think. Aside from helping out people who're not using stdlib, we shouldn't need to set the version (like we do for the compilers on windows as well)
- finally, even if I give windows a dummy version, the rerender still fails. I think this might be due to some other rare/obsolete architectures falling into the same trap as s390x; presumably I'm hitting
linux-32
here (or something else like armv7l)? This also exacerbates the breakage, because it can happen on architectures that the recipe is skipping...? I've hardened the CBC as follows:
and still the rerender fails with the same error. I've tried a local--- a/recipe/conda_build_config.yaml +++ b/recipe/conda_build_config.yaml @@ -44,8 +44,8 @@ c_stdlib: - vs # [win] c_stdlib_version: - 2.12 # [linux64] - - 2.17 # [aarch64 or ppc64le or s390x] - - 10.9 # [osx and x86_64] + - 2.17 # [linux and not linux64] + - 10.9 # [osx and not arm64] - 11.0 # [osx and arm64] - 2019 # [win]
CONDA_SUBDIR=<target> conda build recipe/
for all the targets I could think of, and I cannot determine where it's going wrong. The one thing that stood out to me though is that something is messing with the ordering though...$ CONDA_SUBDIR=linux-64 conda build recipe/ [...] Attempting to finalize metadata for binutils_linux-64 ✅ $ CONDA_SUBDIR=linux-aarch64 conda build recipe/ [...] Attempting to finalize metadata for binutils_linux-ppc64le ❌ $ CONDA_SUBDIR=linux-ppc64le conda build recipe/ [...] Attempting to finalize metadata for binutils_linux-64 ❌ $ CONDA_SUBDIR=linux-s390x conda build recipe/ [...] Attempting to finalize metadata for binutils_linux-s390x ✅ $ CONDA_SUBDIR=linux-armv7l conda build recipe/ [...] Attempting to finalize metadata for binutils_linux-ppc64le ❌
IMO the consequences for conda-build should be, at the very least:
- don't fail on missing
c_stdlib
unless{{ stdlib("c") }}
is used in the recipe for that architecture - IMO this should be considered a regression. - don't fail on missing
c_stdlib_version
in any case; it's something we can work around but shouldn't be necessary. Just don't insert a version pin after what's inserted throughc_stdlib
.
I did more digging here:
-
You have to run conda-smithy very carefully in order to not use the global conda-forge conda-build config.
-
I made a bug fix here: https://github.com/conda-forge/conda-forge-pinning-feedstock/pull/5592
-
If I run this command
conda-smithy rerender --no-check-uptodate -e ../conda_forge_pinning.yaml
on the
binutils-feedstock
it rerenders fine locally. -
Edit: I fixed the zipping with the cdt keys too. ~I do see something off with the c_stdlib_version for linux-64 in the rerender. However, I think this is due to conda-forge's setup and not conda-build itself.~
- don't fail on missing
c_stdlib
unless{{ stdlib("c") }}
is used in the recipe for that architecture - IMO this should be considered a regression.
None of this is specific to stdlib
. If we replace c_stdlib
and c_stdlib_version
with foo
and bar
we will get the same error.
I agree that we shouldn't necessarily fail when keys are incorrectly defined in cbc.yml and also unused in the recipe at hand but I'd also argue that we shouldn't allow usage of a broken cbc.yml and should handle it ASAP.
It would seem to me that it would be better for smithy to intercept the error and say something like "cbc.yml is broken for ____ platform".
- don't fail on missing c_stdlib_version in any case; it's something we can work around but shouldn't be necessary. Just don't insert a version pin after what's inserted through c_stdlib.
The error being thrown is valid. We've specified a zip_key
but one or more of the keys are undefined (after filtering on the selectors). That'd be the same as saying
for stdlib, version in zip(c_stdlib, c_stdlib_version):
print(stdlib, version)
and then wondering why nothing gets printed.
What you're suggesting is that zip_keys
is modified to work like zip_longest
instead of zip
and I'm very certain doing so will have unintended consequences elsewhere.
- I made a bug fix here: https://github.com/conda-forge/conda-forge-pinning-feedstock/pull/5592
Would it help if conda-build offered a subcommand to verify the cbc.yml for some list of platforms?
Thanks @kenodegard! Yes I agree with what you are saying above.
Here is my suggestion for how to proceed:
- I merged the pinning update PR above.
- Once that package is live, let's retest the rendering issues found by @h-vetinari. If they go away, then I think we can proceed with a conda-build release.
- At the same time, let's make some new issues to track some of your ideas in the discussions here. These could include improving the error reporting and providing a command to validate the CBC. I am a bit weary of suppressing errors with unused keys for the reasons mentioned above, namely that is an actual bug and might indicate other issues (like it did here!).
Thanks for the inputs and the pinning PR! With an unmodified recipe and the new pinning, the ctng-compiler-activation now rerenders again.
Per @beckermr 's comment here, we opened a new issue to track further improvements for cbc.yml error reporting and validations. Closing this ticket since the original issue discussed here has been resolved.