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

Support static linking of re2 library #64

Closed
wants to merge 1 commit into from

Conversation

stanhu
Copy link
Collaborator

@stanhu stanhu commented Jan 6, 2023

Currently when the re2 system library is updated by Homebrew, the dynamically linked library is removed and moved to a new location. This breaks the re2 gem and requires users to run gem pristine re2 to get back into a working state.

To avoid this hassle, this commit adds a --enable-static flag that will statically link the library in the gem's C extension library.

Homebrew doesn't currently install a static library for re2. This will be fixed via Homebrew/homebrew-core#119928.

NOTE: This doesn't work at the moment with Ubuntu's libre2-dev package since a number of objects aren't compiled with -fPIC: https://stackoverflow.com/a/4036497.

end

def resolve_static_library(arg)
libs_path = pkg_config('re2', 'libs-only-L')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On Linux, this appears to return an empty list.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like the static library isn't shipped in the libre2-5 package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

However, the static library is shipped in libre2-dev under /usr/lib/x86_64-linux-gnu/libre2.a.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, still getting:

g++ -shared -o re2.so re2.o -L. -L/home/stanhu/.rbenv/versions/2.7.5/lib -Wl,-rpath,/home/stanhu/.rbenv/versions/2.7.5/lib -L/usr/local/lib -Wl,-rpath,/usr/local/lib -L/opt/homebrew/lib -Wl,-rpath,/opt/homebrew/lib -L/usr/lib -Wl,-rpath,/usr/lib -L/usr/lib/x86_64-linux-gnu -Wl,-rpath,/usr/lib/x86_64-linux-gnu -L. -L/home/stanhu/.rbenv/versions/2.7.5/lib  -fstack-protector-strong -rdynamic -Wl,-export-dynamic -L/home/stanhu/.rbenv/versions/2.7.5/lib  -Wl,--compress-debug-sections=zlib    -Wl,-rpath,/home/stanhu/.rbenv/versions/2.7.5/lib -L/home/stanhu/.rbenv/versions/2.7.5/lib -lruby /usr/lib/x86_64-linux-gnu/libre2.a -lstdc++ -lm   -lc
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libre2.a(re2.o): relocation R_X86_64_PC32 against symbol `_ZTVSt9basic_iosIcSt11char_traitsIcEE@@GLIBCXX_3.4' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could be related to Ubuntu's compilation: google/re2#178

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perhaps we're better off doing #61.

Copy link
Collaborator Author

@stanhu stanhu Jan 6, 2023

Choose a reason for hiding this comment

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

Confirmed that the installed Ubuntu shared library appears to be missing -fPIC in some objects. Recompiling with CPPFLAGS=-fPIC make seems to work.

It's not the default because here we're making a shared object from a static library: https://stackoverflow.com/a/4036497

@stanhu stanhu force-pushed the sh-support-static-linking branch 3 times, most recently from 0ec04b0 to 0b151ac Compare January 6, 2023 21:00
Currently when the re2 system library is updated by Homebrew, the
dynamically linked library is removed and moved to a new location.
This breaks the re2 gem and requires users to run `gem pristine re2`
to get back into a working state.

To avoid this hassle, this commit adds a `--enable-static` flag that
will statically link the library in the gem's C extension library.

Homebrew doesn't currently install a static library for re2. This will
be fixed via Homebrew/homebrew-core#119928.
@stanhu stanhu force-pushed the sh-support-static-linking branch from 0b151ac to 59e3a28 Compare January 6, 2023 21:21
@mudge
Copy link
Owner

mudge commented Jan 7, 2023

Can you explain at a high-level how you statically link a library in a Ruby C extension? It looks like you’re replacing the -lre2 flag in $libs with the absolute path of a specific re2 file?

@stanhu
Copy link
Collaborator Author

stanhu commented Jan 7, 2023

Can you explain at a high-level how you statically link a library in a Ruby C extension? It looks like you’re replacing the -lre2 flag in $libs with the absolute path of a specific re2 file?

Yes. That's what Nokogiri does: https://github.com/sparklemotion/nokogiri/blob/b57c92686fbdb90e3e3947ceff654ae825caa2b9/ext/nokogiri/extconf.rb#L975-L986

@mudge
Copy link
Owner

mudge commented Jan 22, 2023

Would it be possible to add a static install to the CI build matrix? It’d be good to see how it behaves against an re2 install that has a static library as well as one that doesn’t. You can see https://github.com/mudge/re2-ci/blob/853bf95e8e099a91eb9bf06f2b02b2626129e01b/libre2-dev/16.04/entrypoint.sh#L10 for how the various re2 debs are built for GitHub Actions.

@stanhu
Copy link
Collaborator Author

stanhu commented Jan 22, 2023

@mudge At the moment, this doesn't work with Debian/Ubuntu because the re2 static library isn't compiled with -fPIC.

I think I'll try to implement #61 and then add this option there so that we have more control over how the library is built.

@stanhu
Copy link
Collaborator Author

stanhu commented Jul 11, 2023

Closing in favor of #67.

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