-
Notifications
You must be signed in to change notification settings - Fork 16
use xcode ar and ranlib if present
#285
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
Conversation
|
includes, for expediency, a fix for the |
projects/fix-machos.com/package.yml
Outdated
| - cp {{deps.zlib.net.prefix}}/lib/libz.dylib '{{prefix}}/lib' | ||
|
|
||
| - run: install_name_tool -change @rpath/zlib.net/v{{deps.zlib.net.version}}/lib/libz.{{deps.zlib.net.version.marketing}}.dylib {{prefix}}/lib/libz.dylib fix-machos-test | ||
| # zlib is weird about its lib versioning it seems |
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.
what this about?
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.
with zlib 1.3(.0) it was linking libz-1.3.dylib. with 1.3.1 it links libz-1.3.1.dylib. it was messing up the name for changing the rpath.
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.
sorry, don't get you. What's it?
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.
The linkage was using v.marketing for zlib 1.3 and version for 1.3.1. so it broke this test, which assumed it would always be version.marketing.
bin/cmd/build
Outdated
| const d = config.path.home.join('toolchain').mkdir('p') | ||
| d.join('ar').ln('s', { target: '/usr/bin/ar' }) | ||
| d.join('ranlib').ln('s', { target: '/usr/bin/ranlib' }) | ||
| } |
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.
I think instead we should add shims for these tools that delegate on darwin and use gnu on linux. Asking for gnu binutils for a build is fine, it's just you would also want to specify gcc most likely as well.
Seems like on linux gnu binutils and llvm work then?
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 far as i can tell, yes.
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.
I can set up the shims
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.
we already have shims for these! k so is the problem binutils is needed for some other tool so it must remain on darwin?
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.
The problem is that if gcc is used it brings in binutils, which provides these, but they definitively break some builds. There are a few packages that had to explicitly override them via env.
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.
kk I get it. It's because our shims for ar etc. are removed when the pkg specifies gcc or llvm explicitly.
Which is probs the right call lol. Need more thought
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.
I don't even see how your fix works. We don’t include the toolchain directory in PATH if the pkg depends on gcc. So how come you adding symilnks there does anything?
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.
Urk. Sounds like I missed a step.
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.
well you say it fixed them and I believe you, I just don't see how lol
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.
sorry; replacing binutils ar and ranlib via ENV vars fixed the builds in the past. this (obviously incomplete) "fix" was intended to allow removal of that hack. just need to add toolchain to the PATH iff it is populated.
fixes #277
this is the shortest path to using
/usr/bin/arand/usr/bin/ranlibon darwin, if present, in place ofgnu.org/binutils's. one alternative would be to simply not ship those files (or replace them with symlinks) in the binutils build script, but that feels a tad off.other solutions are certainly possibly, including overring env.AR and env.RANLIB, or doing nothing.