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

work around ld64 issue #96

Merged
merged 19 commits into from
May 18, 2024

Conversation

matthiasdiener
Copy link
Contributor

@matthiasdiener matthiasdiener commented Mar 18, 2024

See also conda-forge/cctools-and-ld64-feedstock#66

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.

@matthiasdiener matthiasdiener self-assigned this Mar 18, 2024
@conda-forge-webservices
Copy link
Contributor

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) and found it was in an excellent condition.

@matthiasdiener matthiasdiener changed the title wrok around ld64 issue work around ld64 issue Mar 18, 2024
@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented Mar 18, 2024

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=898533&view=logs&jobId=ced5d8de-8227-5f3f-33a1-45cf1592c45a&j=4b3757f9-99c5-5891-6c0c-93d7030cdf7d&t=2ff5d027-ebfd-5364-5607-51694385a7af
(with the workaround reverted) illustrates the issue:

+ python demo.py
ld: dynamic main executables must link with libSystem.dylib for architecture x86_64
error: linker command failed with exit code 1 (use -v to see invocation)
Final linking of kernel sum failed.
/Users/runner/miniforge3/conda-bld/pocl-core_1710796486497/test_tmp/run_test.sh: line 11: 57149 Abort trap: 6           python demo.py

@matthiasdiener
Copy link
Contributor Author

What do you think of 83ed9a4 ?

recipe/meta.yaml Outdated
@@ -147,10 +147,15 @@ outputs:
- {{ pin_subpackage("pocl-remote", exact=True) }}
- {{ pin_subpackage("pocl-cuda", exact=True) }} # [enable_cuda]
test:
requires:
- pyopencl
Copy link
Contributor

Choose a reason for hiding this comment

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

In this comment, @isuruf specifically requested to not introduce dependency cycles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed in aba69ca

@conda-forge-webservices
Copy link
Contributor

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) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • You're setting a constraint on the __osx virtual package directly; this should now be done by adding a build dependence on {{ stdlib("c") }}, and overriding c_stdlib_version in recipe/conda_build_config.yaml for the respective platform as necessary. For further details, please see META: {{ stdlib("c") }} migration conda-forge.github.io#2102.

@conda-forge-webservices
Copy link
Contributor

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) and found it was in an excellent condition.

@@ -23,5 +23,5 @@ enable_cuda:
- False # [not (linux64 or ppc64le or aarch64)]
Copy link
Contributor Author

@matthiasdiener matthiasdiener May 17, 2024

Choose a reason for hiding this comment

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

side note: this package does not seem to be built at all anymore for hwloc v1, despite the entry in conda_build_config.yaml

recipe/conda_build_config.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Show resolved Hide resolved
recipe/meta.yaml Show resolved Hide resolved
@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented May 17, 2024

For aarch64, it seems both the Travis native and Azure emulation builds are broken at the moment. I have tested the build locally though and it seems to work fine, so we could go ahead with merging if the rest of the CI passes. This PR shouldn't affect anything related to aarch64.

@inducer inducer added the automerge Merge the PR when CI passes label May 18, 2024
@matthiasdiener matthiasdiener merged commit d51c72b into conda-forge:main May 18, 2024
10 checks passed
@matthiasdiener matthiasdiener deleted the constrain-ld64 branch May 18, 2024 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants