Add patch for https://bugs.python.org/issue42015 (which affects pybind11) #402
Conversation
…da-forge-pinning 2020.10.13.19.09.25
This reverts commit f2cc154.
mbargull
requested review from
isuruf,
jakirkham,
jjhelmus,
mingwandroid,
msarahan,
ocefpaf,
pelson and
scopatz
as code owners
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
recipe/conda_build_config.yaml
Outdated
| python_impl: | ||
| - cpython | ||
| numpy: | ||
| - 1.19 |
There was a problem hiding this comment.
Without this conda-smithy=3.8.2 failed with
Adding in variants from internal_defaults
INFO:conda_build.variants:Adding in variants from internal_defaults
Adding in variants from /tmp/tmp600bzq31/conda_build_config.yaml
INFO:conda_build.variants:Adding in variants from /tmp/tmp600bzq31/conda_build_config.yaml
Adding in variants from /home/mbargull/code/src/conda-forge/git/python-feedstock/recipe/conda_build_config.yaml
INFO:conda_build.variants:Adding in variants from /home/mbargull/code/src/conda-forge/git/python-feedstock/recipe/conda_build_config.yaml
Traceback (most recent call last):
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/bin/conda-smithy", line 10, in <module>
sys.exit(main())
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/lib/python3.7/site-packages/conda_smithy/cli.py", line 613, in main
args.subcommand_func(args)
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/lib/python3.7/site-packages/conda_smithy/cli.py", line 419, in __call__
self._call(args, tmpdir)
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/lib/python3.7/site-packages/conda_smithy/cli.py", line 430, in _call
temporary_directory=temporary_directory,
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/lib/python3.7/site-packages/conda_smithy/configure_feedstock.py", line 1938, in main
render_travis(env, config, forge_dir, return_metadata=True)
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/lib/python3.7/site-packages/conda_smithy/configure_feedstock.py", line 1082, in render_travis
return_metadata=return_metadata,
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/lib/python3.7/site-packages/conda_smithy/configure_feedstock.py", line 602, in _render_ci_provider
os.path.join(forge_dir, forge_config["recipe_dir"]), config=config
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/lib/python3.7/site-packages/conda_build/variants.py", line 525, in get_package_combined_spec
combined_spec = combine_specs(specs, log_output=config.verbose)
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/lib/python3.7/site-packages/conda_build/variants.py", line 277, in combine_specs
log_output=log_output)
File "/home/mbargull/code/conda/conda-forge/envs/conda-smithy/lib/python3.7/site-packages/conda_build/variants.py", line 252, in _combine_spec_dictionaries
spec_source))
ValueError: variant config in /home/mbargull/code/src/conda-forge/git/python-feedstock/recipe/conda_build_config.yaml is ambiguous because it does not fully implement all zipped keys, or specifies a subspace that is not fully implemented.
|
due to non-existing https://github.com/phracker/MacOSX-SDKs/releases/download/10.15/MacOSX11.0.sdk.tar.xz |
|
Thanks, @mbargull! It's really great for us to see this getting patched so soon :-) |
|
@YannickJadoul, I saw your work at pybind/pybind11#2576 in which you added the workaround depending on the exact Python version (which makes sense giving that there's probably nothing else to go off of to see if the workaround is need or not). The inclusion of this patch in our |
Looking at the logs, we used the 11.0 SDK that the CI provided before. But Azure https://github.com/microsoft/azure-pipelines-image-generation/commit/1a6b857e3408a2318dec81dbc332ff2abecaab62#diff-7a1606bd717fc0cf55f9419157117d9ca306f91bd2fdfc294720687d7be1b2c7R5 their image and the Xcode 12 beta alongside it -- and apparently the SDK isn't available under the same path anymore. In a previous (successful) build we had Now we have I didn't read actions/runner-images#1646 yet, but there seems to be more information for this. |
|
@mbargull, right, that ís actually something to be discussed. Let's also bring in @henryiii into this, then. Whatever we decide, this patch will still be very useful for users stuck with older, pre-2.6.0 version of pybind11 that want to support 3.9! |
Yep, from conda-forge/conda-forge-ci-setup-feedstock#125 |
|
I'd vote for putting it in. It is hard to control the usage of pybind11, in part because a lot of software uses it directly from the headers, for example, via submodule, rather than using conda-forge's pybind11 package. SciPy, PyTorch, and more will segfault until they update unless we put this in. We can "un-patch" the conda-forge pybind11 package to make it slightly faster and avoid leaking on 3.9.0 if we want to, but I believe the leak is very small and the performance difference is minimal (a few clock cycles per callable destruction). |
|
Hi! This is the friendly conda-forge automerge bot! I considered the following status checks when analyzing this PR:
Thus the PR was passing and merged! Have a great day! |
|
python-feedstock/recipe/build_base.sh Lines 294 to 297 in c5af9fa |
|
I made a bad PR for latest conda-build. Fixed in conda/conda-build#4094. I'll add a workaround in the compiler activation recipe |
|
Thanks for this patch!! |
Checklist
0(if the version changed)conda-smithy(Use the phrase@conda-forge-admin, please rerenderin a comment in this PR for automated rerendering)This adds python/cpython#22674 which is a backport of python/cpython#22670 addressing https://bugs.python.org/issue42015 which was reported by the
pybind11maintainer @YannickJadoul in reference to pybind/pybind11#2558 and their workaround PR pybind/pybind11#2576.Given the small changeset and scope of the patch, it could make sense to include it in our
pythonbuild even before the Python 3.9.1 release (schedules for December).cc @isuruf, @conda-forge/pybind11, @henryiii