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

fix(libextism): improve static linking pkgconfig #726

Merged
merged 5 commits into from
Jun 11, 2024
Merged

Conversation

G4Vi
Copy link
Contributor

@G4Vi G4Vi commented Jun 11, 2024

This includes two fixes:

@zshipko if you could verify make pkg-config-static and the resulting example executable in libextism works on Mac that would be greatly appreciated.

@G4Vi G4Vi requested a review from zshipko June 11, 2024 06:16
@zshipko
Copy link
Contributor

zshipko commented Jun 11, 2024

I just pushed a commit to fix the libextism example on macOS - the only thing I didn't fix is on macOS -framework Security also needs to be linked in the static pkg-config file - what do you think the best way of handling that is?

@G4Vi
Copy link
Contributor Author

G4Vi commented Jun 11, 2024

Thanks! The best way of handling it is probably adding another variable to the template, extism-static-pc.in, and having the cli fill it in like it does for the prefix. Normally it would be empty, but on mac it would have -framework Security

@zshipko
Copy link
Contributor

zshipko commented Jun 11, 2024

Alright I can add that - I think I can probably do something similar but avoid adding a variable so we maintain compatibility with existing CLI installations

@G4Vi
Copy link
Contributor Author

G4Vi commented Jun 11, 2024

Maintaining compatibility is better, good idea.

@zshipko
Copy link
Contributor

zshipko commented Jun 11, 2024

Alright, this seems to be working now - let me know if you see anything that looks off @G4Vi (I don't think you can "review" your own PR)

I also have extism/cli#94 ready to merge

@G4Vi
Copy link
Contributor Author

G4Vi commented Jun 11, 2024

Looks good to me. The only thing I wonder about is -framework Security being part of Libs instead of Libs.private, but if it needs to go before libextism when it has to. Have you seen any other pkgconfig that uses it?

If you review I can merge without bypassing branch protections.

@zshipko
Copy link
Contributor

zshipko commented Jun 11, 2024

Including it in Libs.private still resulted in the linker error but Libs worked - I'm not sure why though. It could be the order of the libraries, but based on this search https://github.com/search?q=%22-framework+Security%22+path%3A*.pc&type=code it looks like people are using both Libs and Libs.private.

@G4Vi G4Vi merged commit c4b82e3 into main Jun 11, 2024
5 checks passed
@G4Vi G4Vi deleted the fix-static-linking branch June 11, 2024 20:22
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