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

Use old ld64 linker on MacOS >= 15.0 #1083

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Conversation

lucianopaz
Copy link
Contributor

@lucianopaz lucianopaz commented Nov 11, 2024

Description

Add a flag to use Mac's older ld64 linker instead of the new ld_prime linker. This might be the underlying cause of the problems that are still coming out of #1005. These issues show that pytensor fails in its compilation phase. The exception is a bit masked, but it seems to indicate that the linker had problems and exited with a signal. In my case it was a segfault with signal 11. I've seen other cases that seem similar and complain about symbols that should be defined in libtapi.dylib. The problem is that I can't reproduce in this isolated environment. My attempt to check whether this problem exists on Mac was to bump the version of the macos runner to 15 (the bump was supposed to happen during december anyway).

Related Issue

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pytensor--1083.org.readthedocs.build/en/1083/

# XCode15 introduced ld_prime linker. At the time of writing, this linker
# leads to multiple issues, so we supply a flag to use the older dynamic
# linker: ld64
cxxflags.append("-ld64")
Copy link
Member

Choose a reason for hiding this comment

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

Is this back compatible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m not sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ll have to add a platform version check to add this flag or not

Copy link

codecov bot commented Nov 11, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.10%. Comparing base (fdbf3aa) to head (484bfdf).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pytensor/link/c/cmodule.py 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1083      +/-   ##
==========================================
- Coverage   82.10%   82.10%   -0.01%     
==========================================
  Files         183      183              
  Lines       47930    47932       +2     
  Branches     8633     8634       +1     
==========================================
+ Hits        39354    39355       +1     
  Misses       6410     6410              
- Partials     2166     2167       +1     
Files with missing lines Coverage Δ
pytensor/link/c/cmodule.py 60.48% <50.00%> (-0.02%) ⬇️

@lucianopaz
Copy link
Contributor Author

@ricardoV94, I patched this

@ricardoV94 ricardoV94 merged commit d9d8dba into pymc-devs:main Nov 12, 2024
61 of 62 checks passed
@lucianopaz lucianopaz changed the title Ld64 Use old ld64 linker on MacOS >= 15.0 Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants