Skip to content

Fix libjulia install name and libjulia-internal rpath on OS X#47220

Merged
ViralBShah merged 1 commit intoJuliaLang:masterfrom
jonathan-conder-sm:mac_embedding_main
Dec 7, 2022
Merged

Fix libjulia install name and libjulia-internal rpath on OS X#47220
ViralBShah merged 1 commit intoJuliaLang:masterfrom
jonathan-conder-sm:mac_embedding_main

Conversation

@jonathan-conder-sm
Copy link
Copy Markdown
Contributor

@jonathan-conder-sm jonathan-conder-sm commented Oct 19, 2022

Follow-up to #47053, which explains the rationale

@giordano giordano added building Build system, or building Julia or its dependencies system:mac Affects only macOS labels Oct 19, 2022
@ViralBShah
Copy link
Copy Markdown
Member

ViralBShah commented Oct 31, 2022 •
edited
Loading

Should we merge this? Do we need the DARWIN_FRAMEWORK stuff? I thought we removed (or were planning to remove) the framework build option.

@jonathan-conder-sm
Copy link
Copy Markdown
Contributor Author

jonathan-conder-sm commented Oct 31, 2022

That might be a holdover from the 1.6 branch, I can check tomorrow

@ViralBShah
Copy link
Copy Markdown
Member

ViralBShah commented Oct 31, 2022

I do see a lot of DRAWIN_FRAMEWORK on master - so it didn't happen yet.

@ViralBShah
Copy link
Copy Markdown
Member

ViralBShah commented Dec 6, 2022 •
edited
Loading

@staticfloat / @giordano Should we merge this? I think it is good to merge since @staticfloat approved it in #47053.

@ViralBShah ViralBShah added the backport 1.9 Change should be backported to release-1.9 label Dec 6, 2022
@ViralBShah
Copy link
Copy Markdown
Member

ViralBShah commented Dec 6, 2022 •
edited
Loading

@jonathan-conder-sm Can you check my merge conflict resolution is done right?

@ViralBShah ViralBShah added the merge me PR is reviewed. Merge when all tests are passing label Dec 6, 2022
@jonathan-conder-sm
Copy link
Copy Markdown
Contributor Author

jonathan-conder-sm commented Dec 6, 2022 •
edited
Loading

the merge looks fine to me. still has DARWIN_FRAMEWORK but looks like master still has it too

@jonathan-conder-sm
Copy link
Copy Markdown
Contributor Author

jonathan-conder-sm commented Dec 6, 2022

actually I'm not sure, looks like RPATH_LIB was removed and this adds it back, unsure if that's the right thing to do or not

@jonathan-conder-sm
Copy link
Copy Markdown
Contributor Author

jonathan-conder-sm commented Dec 6, 2022

I don't think the RPATH of libjulia.dylib is relevant to my changes: only the install name. So I've kept RPATH_LIB as-is and rebased on master

@ViralBShah ViralBShah merged commit de4f1c3 into JuliaLang:master Dec 7, 2022
This was referenced Dec 7, 2022
Closed
Closed
KristofferC pushed a commit that referenced this pull request Dec 8, 2022
@jonathan-conder-sm @KristofferC
Fix libjulia install name and libjulia-internal rpath on OS X (#47220)
712a123 
(cherry picked from commit de4f1c3)
This was referenced Dec 8, 2022
Merged
Merged
@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label Dec 15, 2022
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Dec 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@andreea199515 andreea199515 andreea199515 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

building Build system, or building Julia or its dependencies system:mac Affects only macOS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jonathan-conder-sm @ViralBShah @andreea199515 @giordano @KristofferC