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

ext: export only Init_sqlite3_native #371

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

flavorjones
Copy link
Member

@flavorjones flavorjones commented Jan 2, 2023

This works around new Darwin Ruby 3.2 symbol resolution issues, but also follows best practices around shipping shared libraries.

The result is that the only public (exported) symbol in the extension is Init_sqlite3_native, regardless of what platform or version of Ruby you're on.

See rake-compiler/rake-compiler-dock#87 for lots of context.

@flavorjones
Copy link
Member Author

See related proposal for Nokogiri at sparklemotion/nokogiri#2746

This works around new Darwin Ruby 3.2 symbol resolution issues, but
also follows best practices around shipping shared libraries.

Note that we remove the `-flat_namespace` hack because the latest
rake-compiler-docker images have the changes from
rake-compiler/rake-compiler-dock#94
@flavorjones flavorjones force-pushed the flavorjones-use-visibility-hidden-by-default branch from eac9d8a to c8c1eb3 Compare January 4, 2023 20:49
@flavorjones flavorjones merged commit 52b3cf3 into master Jan 4, 2023
@flavorjones flavorjones deleted the flavorjones-use-visibility-hidden-by-default branch January 4, 2023 21:56
@byroot
Copy link
Contributor

byroot commented Jan 5, 2023

The result is that the only public (exported) symbol in the extension is Init_sqlite3_native, regardless of what platform or version of Ruby you're on.

What about ruby_abi_version for dev versions of Ruby?

@flavorjones
Copy link
Member Author

@byroot I don't actually know much about how that symbol gets used, can you point me at docs to learn more?

C extensions on Windows have always only exported the target function (Init_*) so although the ABI version maybe should also be exported, that's something that's already broken on at least one platform. (If I'm mistaken, please let me know.)

I'm planning to propose something upstream (in Ruby) to formalize or automate exposing minimal symbols, since I think most symbols only get exported on Linux and Darwin by accident (it hasn't been an intentional decision to export them) and this recently caused problems with the RBS gem. I'll make sure to include ruby_abi_version in the discussion.

@byroot
Copy link
Contributor

byroot commented Jan 5, 2023

It's a symbol Peter added starting in 3.2.0dev to better handle ABI breaks. It was supposed to be exposed in 3.2.0 final too but somehow it didn't happen: ruby/ruby#5474

If needed I'm sure Peter can tell you more.

I'm planning to propose something upstream (in Ruby) to formalize or automate exposing minimal symbols

Huge 👍 . As you may remember this has caused issues with hiredis etc. I agree C-ext shouldn't expose anything by default. There is very little reason for exposing anything more than the init function.

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.

None yet

3 participants