-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Migrate to {{ stdlib("c") }}
#13
Conversation
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 ( |
{{ stdlib("c") }}
{{ stdlib("c") }}
@conda-forge-admin , please re-render |
@conda-forge-admin , please restart CI |
c_stdlib: | ||
- vs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird. Re-rendering shouldn't have removed this
Note that this is what leads to the CI error for Windows:
conda_libmamba_solver.conda_build_exceptions.ExplainedDependencyNeedsBuildingError: Unsatisfiable dependencies for platform win-64: {MatchSpec("c_win-64")}
Encountered problems while solving:
- nothing provides requested c_win-64
Will try re-rendering again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On closer inspection, it looks like re-rendering with conda-smithy is doing the right thing
The cuda-nvtx
package has a skip
for win-64
. As a result is ignored when generating the variants. Hence c_stdlib
is dropped
However for some reason it is still being considered by conda-build
when building cuda-nvtx-dev
@conda-forge-admin , please re-render |
recipe/meta.yaml
Outdated
@@ -51,6 +51,7 @@ outputs: | |||
build: | |||
- {{ compiler("c") }} | |||
- {{ compiler("cxx") }} | |||
- {{ stdlib("c") }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a workaround let's try adding a selector to limit {{ stdlib("c") }}
to Linux. This shouldn't be needed as the cuda-nvtx
package is Linux only. Though maybe this helps conda-build
in its parsing of this recipe
- {{ stdlib("c") }} | |
- {{ stdlib("c") }} # [linux] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this is still needed even with conda-build
version 24.3.0
- {{ stdlib("c") }} | |
- {{ stdlib("c") }} # [linux] |
@conda-forge-admin , please re-render |
Interesting apparently the bot had a no change commit: 0228ab7 According to the GHA job, some changes were made:
|
Testing locally, conda-smithy appears to be applying these changes ( conda-forge/conda-smithy#1923 ) to the GHA automerge workflow as expected Guessing this is just a bug with the re-rendering bot. Filed upstream in issue: conda-forge/conda-forge-webservices#706 |
00b550e
to
7fcdc2e
Compare
Seeing a new issue with the Windows build on CI (also attached log for posterity): Traceback (most recent call last):
File "C:\Miniforge\Scripts\conda-build-script.py", line 10, in <module>
sys.exit(execute())
File "C:\Miniforge\lib\site-packages\conda_build\cli\main_build.py", line 590, in execute
api.build(
File "C:\Miniforge\lib\site-packages\conda_build\api.py", line 250, in build
return build_tree(
File "C:\Miniforge\lib\site-packages\conda_build\build.py", line 3638, in build_tree
packages_from_this = build(
File "C:\Miniforge\lib\site-packages\conda_build\build.py", line 2696, in build
output_d["files"] = set(output_d["files"]) - to_remove
TypeError: 'frozendict.frozendict' object does not support item assignment Looks potentially related to an upstream change: conda/conda-build#5284 |
Reproduced independently of these changes in PR ( #16 ). Have summarized the results in comment ( #16 (comment) ). Also raised as upstream issue ( conda/conda-build#5342 ) |
Add stdlib
b2f9f93
to
527d726
Compare
@conda-forge-admin , please re-render |
…nda-forge-pinning 2024.05.14.14.54.30
To bring CI back into a working state, have pinned to Squashed and rebased the changes here. Plus cleaned out any workaround that were attempted Asked the bot to do a fresh re-render, which it did Think that should clear up the CI issues we have seen before so that we can merge this |
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! |
Workaround as `conda-build` considers the dependencies of this package even when it is skipped (as is the case on Windows).
@conda-forge-admin , please re-render |
Hi! This is the friendly automated conda-forge-webservice. I tried to rerender for you, but it looks like there was nothing to do. This message was generated by GitHub actions workflow run https://github.com/conda-forge/cuda-nvtx-feedstock/actions/runs/9087714664. |
The
sysroot*
syntax is getting phased out (conda-forge/conda-forge.github.io#2102).The recommendation is to move to
{{ stdlib("c") }}
.Ref conda-forge/cuda-feedstock#26