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

clang/find_clang.cr: Auto-detect if linking libclang using .a or .so #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Papierkorb
Copy link
Owner

@Papierkorb Papierkorb commented Jun 22, 2020

Hello,

Was toying with bindgen today and noticed that ArchLinux joined the
distros who want static linking of libclang. It annoyed me so in OSS
fashion I decided to do something about it.

If both static and dynamic versions are available it'll prefer the
static library, which is hopefully more stable when the user decides
to update their system. The user can still use BINDGEN_DYNAMIC=1/0
as before, but simply not setting it will trigger the auto-detection.

However this could break stuff horribly on non-ArchLinux machines,
so I PR'd this before I break master out of the blue. Does this work for
everyone?

Also, find_clang.cr has code to read the OS name to force the default
to static linking on a bunch of distros - Do we still need this code with
this change?

Cheers!

ArchLinux has changed their mind in the meantime.  Didn't feel like
messing with the environment variable, instead, it's supposed to
auto-detect now if libclang.a or libclang.so exist.

If both static and dynamic versions are available it'll prefer the
static library, which is hopefully more stable when the user decides
to update their system.

The user can still use BINDGEN_DYNAMIC=1/0 as before, but simply not
setting it will trigger the auto-detection.

Also, the pattern-matching style "case" statement of Crystal is some
good stuff!
@docelic
Copy link
Collaborator

docelic commented Jun 22, 2020

Hey @Papierkorb I think we already covered this previously and it was working in both cases. But @ZaWertun and @kalinon would be better to comment, since the current implementation came from them.

@docelic
Copy link
Collaborator

docelic commented Jun 22, 2020

(Or, on a second thought, maybe you are right. Currently there is no autodetection (DYNAMIC needs to be set manually), and the feature/change which I was thinking about was the one where we just made sure that we only linked against .a's or .so's, and not both.)

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