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

macho: implement -search_paths_first and -search_dylibs_first #11917

Merged
merged 7 commits into from
Jun 25, 2022

Conversation

motiejus
Copy link
Contributor

Ignore MachO-specific flag -search_paths_first, since it is the default
in zld and ld64.

Also see Jakub's comment1:

Changing topic slightly, @motiejus dunno if you noticed, with this
change building arm64 Zig binary fails on macos - it complains about
unknown -search_paths_first flag. This one is an easy fix: we should
ignore it since this is the default behaviour in both ld64 and zld.

@kubkon
Copy link
Member

kubkon commented Jun 23, 2022

Thanks for the PR @motiejus! I'm going to hijack your branch and fix another unhandled flag -headerpad_max_install_names, and push all in one go. However, before this happens, I wanna land #11910 first so that I can immediately add some relevant linker tests. Hope you don't mind!

motiejus and others added 5 commits June 24, 2022 19:03
Ignore MachO-specific flag -search_paths_first, since it is the default
in zld and ld64.

Also see Jakub's comment[1]:

    Changing topic slightly, @motiejus dunno if you noticed, with this
    change building arm64 Zig binary fails on macos - it complains about
    unknown -search_paths_first flag. This one is an easy fix: we should
    ignore it since this is the default behaviour in both ld64 and zld.

[1]: ziglang#11906 (comment)
Unlike targeting ELF-based OSes such as Linux, resolving system libs
on Darwin should follow one of two strategies: `-search_paths_first`
or `-search_dylibs_first` and hence we defer always forcing linking
a static library to the linker.
@kubkon kubkon force-pushed the wl-search-paths branch from 3e32563 to 24821dd Compare June 24, 2022 20:11
@kubkon kubkon removed their assignment Jun 24, 2022
@kubkon
Copy link
Member

kubkon commented Jun 24, 2022

Apologies for making you a reviewer on your own PR @motiejus, but would you mind having a look? Instead of implementing -headerpad_max_install_names, I've decided to implement -search_dylibs_first which is another possible search strategy to -search_paths_first in MachO linkers. I've also noticed we were substituting a static archive (if found) for a system library name (i.e., -la was getting transformed into <path>/liba.a in main.zig). This is actually wrong targeting MachO as we want to defer the resolution of all libs to the link stage so that we can correctly act on the -search_paths_first and -search_dylibs_first flags.

@kubkon kubkon changed the title zld: ignore -search_paths_first zld: implement -search_paths_first and -search_dylibs_first Jun 25, 2022
@kubkon kubkon changed the title zld: implement -search_paths_first and -search_dylibs_first macho: implement -search_paths_first and -search_dylibs_first Jun 25, 2022
@kubkon kubkon merged commit 0078d36 into ziglang:master Jun 25, 2022
Copy link
Contributor Author

@motiejus motiejus left a comment

Choose a reason for hiding this comment

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

Apologies for making you a reviewer on your own PR @motiejus, but would you mind having a look?

Sure! Sorry for not reviewing earlier; you posted this on the day of summer solstice :)

src/link/MachO.zig Show resolved Hide resolved
src/link/MachO.zig Show resolved Hide resolved
src/link/MachO.zig Show resolved Hide resolved
src/main.zig Show resolved Hide resolved
@motiejus motiejus deleted the wl-search-paths branch June 26, 2022 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants