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 static builds #25

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

Fix static builds #25

wants to merge 1 commit into from

Conversation

d-unsed
Copy link
Collaborator

@d-unsed d-unsed commented Nov 29, 2017

  • Use libdir for both static and shared libraries
  • Link CoreFoundation framework for Mac OS
  • Extract the name of static library from LIBRUBY_A

Those builds which test static linking will fail on Travis for now (see travis-ci/travis-rubies#43)

 - Use `libdir` for both static and dynamic builds
 - Link `CoreFoundation` framework for Mac OS
 - Extract the name of static library from `LIBRUBY_A`
@malept
Copy link
Contributor

malept commented Nov 29, 2017

What are the shortcomings of the current state of static libraries on macOS? I ask because I haven't had problems when using my branch you merged yesterday.

@d-unsed
Copy link
Collaborator Author

d-unsed commented Nov 29, 2017

Which version of MacOS and Ruby did you use? Was ruby installed with --enable-shared? Also did you use some ruby managers like rbenv/rvm?

I could not build static libs because of few things:

  1. The name of static library was not known to linker (this PR fetches it from LIBRUBY_A).
  2. No path to libdir (here it's set for both static and shared libs).
  3. CoreFoundation framework was not linked.

As for CoreFoundation, not linking it to ruby-sys produced this kind of errors in runtime:

  = note: Undefined symbols for architecture x86_64:
            "_kCFAllocatorDefault", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-6b9be01337d63d2b.rlib(file.o)
            "_CFStringCreateWithBytesNoCopy", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-6b9be01337d63d2b.rlib(file.o)
            "_CFStringGetLength", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-6b9be01337d63d2b.rlib(file.o)
            "_kCFAllocatorNull", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-6b9be01337d63d2b.rlib(file.o)
            "_CFStringCreateMutableCopy", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-6b9be01337d63d2b.rlib(file.o)
            "_CFStringGetBytes", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-6b9be01337d63d2b.rlib(file.o)
            "_CFStringNormalize", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-6b9be01337d63d2b.rlib(file.o)
            "_CFRelease", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-6b9be01337d63d2b.rlib(file.o)

error: aborting due to previous error

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:525:8

When I took a look at LIBRUBYARG_STATIC parameter, it had the following value: -framework CoreFoundation. And linking CoreFoundation to ruby-sys solved the problem for me.

Could you please try to test this branch on your machine when you have few minutes? Would be nice to test it against some other library, for example in the simplest case, to run cargo test on ruru. Thanks!

@malept
Copy link
Contributor

malept commented Nov 29, 2017

t12r is the gem, the static binaries are in GitHub Releases. I've tested this on Sierra and High Sierra (whatever version numbers those are) with Ruby 2.4.2.

@d-unsed
Copy link
Collaborator Author

d-unsed commented Dec 14, 2017

@malept, maybe you've had a chance to test this on your machine?

@andrewstucki
Copy link

Just wondering if there's any update on this?

@malept
Copy link
Contributor

malept commented Mar 8, 2018

@andrewstucki have you tried building extensions on macOS with and without this PR?

@andrewstucki
Copy link

@malept Sorry, long post ahead. So, I dug more into this and it looks a bit funky.

I ran the ruru unit tests to try this out.

Here's the compiler error I get on my machine building with ruby 2.4.1 installed via rbenv and ruby-build (after adjusting the name of my libruby-static artifact) running the tests with the master ruru branch:

  = note: Undefined symbols for architecture x86_64:
            "_kCFAllocatorDefault", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-357e4642a5d89297.rlib(file.o)
            "_CFStringCreateWithBytesNoCopy", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-357e4642a5d89297.rlib(file.o)
            "_CFStringGetLength", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-357e4642a5d89297.rlib(file.o)
            "_kCFAllocatorNull", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-357e4642a5d89297.rlib(file.o)
            "_CFStringCreateMutableCopy", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-357e4642a5d89297.rlib(file.o)
            "_CFStringGetBytes", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-357e4642a5d89297.rlib(file.o)
            "_CFStringNormalize", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-357e4642a5d89297.rlib(file.o)
            "_CFRelease", referenced from:
                _rb_str_append_normalized_ospath in libruby_sys-357e4642a5d89297.rlib(file.o)
          ld: symbol(s) not found for architecture x86_64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

Pointing ruru at this branch fixes the issues:

diff --git a/Cargo.toml b/Cargo.toml
index 446d6e0..a26a390 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -12,5 +12,5 @@ keywords = ["cruby", "mri", "ruby", "ruru"]
 license = "MIT"

 [dependencies]
-ruby-sys = "0.3.0"
+ruby-sys = { git = "http://github.com/steveklabnik/ruby-sys", branch = "fix/static_build" }
 lazy_static = "0.2.1"

Off the top of my head I'm not sure why the project you linked worked, but I'm willing to bet it has something to do with how Thermite (which that project is using) grabs and unpacks the ruby static library tarballs and how those libraries (naming convention and linkage) differ from whatever stock rbenv (what I'm using) is doing.

So, just for recreation purposes on a Mac, here's what I did:

rbenv install 2.4.1
rbenv global 2.4.1
RUBY_LIB_PATH=$(ruby -e "print RbConfig::CONFIG['libdir']")
cp $RUBY_LIB_PATH/libruby.2.4.1-static.a $RUBY_LIB_PATH/libruby-static.a
git clone https://github.com/d-unseductable/ruru && cd ruru
RUBY_STATIC=1 cargo test # watch this fail
vi Cargo.toml # adjust to above
RUBY_STATIC=1 cargo test # watch this pass

Now here's where things get a bit gross. I decided to download current ruby source to verify that rbenv wasn't modifying the static library names. grepping through the source shows me that the Makefile constructs the static library name with LIBRUBY_A='lib$(RUBY_SO_NAME)-static.a', where RUBY_SO_NAME is defined as RUBY_SO_NAME='$(RUBY_BASE_NAME).$(RUBY_PROGRAM_VERSION)'.

That being said, from what I can tell, the patch seems to support linking to both libraries named libruby-static.a and libruby.VERSION-static.a--so even if other projects, like Thermite, are relying on a convention of libruby-static.a this should still work.

Regarding the CoreFoundation linkage. Going through the Changelog shows me from 2013:

Fri Aug  9 22:51:10 2013  Nobuyoshi Nakada  <[email protected]>

        * configure.in (XLDFLAGS, LIBRUBYARG_STATIC): CoreFoundation framework
          option is now needed always, regardless enable-shared.
          [ruby-core:56467] [Bug #8759]

That being say, it points to the fact that what we might want to do is grab LIBRUBYARG_STATIC from RbConfig, which sure enough gives me this:

➜  ~ ruby -e "puts RbConfig::CONFIG['LIBRUBYARG_STATIC']"
-lruby.2.4.1-static -framework CoreFoundation

Interestingly enough switching to ruby 2.5.0 gives me the following:

➜  ~ rbenv shell 2.5.0
➜  ~ ruby -e "puts RbConfig::CONFIG['LIBRUBYARG_STATIC']"
-lruby.2.5.0-static -framework Foundation

Honestly, I have no idea what the difference is between CoreFoundation and Foundation, but what this is telling me is that it's probably worth using LIBRUBYARG_STATIC to determine the linkage rather than hardcoding a dependency on CoreFoundation.

@andrewstucki
Copy link

Also worth noting that it looks like there's also LIBRUBYARG_SHARED which might be worth using instead of RUBY_SO_NAME

@malept
Copy link
Contributor

malept commented Mar 8, 2018

Hmmm. I still have no idea why t12r works. Thermite doesn't do anything regarding static libraries, except distinguish between prebuilt Rust extensions. For the most part, it lets ruby-sys handle linker args.

@andrewstucki
Copy link

@malept I have no idea either, especially since as far as I can tell from taking a look at the build logs the ruby that the project appears to be using to build the libraries looks like it's coming from Travis's pre-built rubies which are here: http://rubies.travis-ci.org/osx/10.12/x86_64/ruby-2.4.2. If you download the tarball it looks like the static libraries aren't even included. At the same time, downloading the artifacts from t12r, the build seems to be generating a library that doesn't depend on libruby, so it's clearly linking statically.

@andrewstucki
Copy link

andrewstucki commented Mar 9, 2018

@malept So I think I figured out what's going on here.

It has to do with a couple of things -- first is the way ruru is running tests, the second is with how the Travis build of t12r is working along with thermite.

ruru

Because ruru's tests are written in Rust, in order for the tests to be executed, libruby must be linked to the testing binary or symbol resolution fails.

t12r

Is built using thermite. If you take a look here what's happening is that thermite is appending a number of linker flags onto Cargo's invocation of rustc (config.dynamic_linker_flags is passing the value of ruby -e "puts RbConfig::CONFIG['DLDFLAGS']"). One of the linker flags is -Wl,-undefined,dynamic_lookup. You can see the linker flag passed here.

That flag suppresses errors trying to resolve symbols at linking time. If you manually remove that and invoke that same line directly you'll see the a whole bunch of failed symbol resolutions a la:

  = note: Undefined symbols for architecture x86_64:
            "_rb_sym2id", referenced from:
                ruru::class::symbol::Symbol::to_string::h5cddbbc793411068 in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_id2name", referenced from:
                ruru::class::symbol::Symbol::to_string::h5cddbbc793411068 in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_string_value_cstr", referenced from:
                ruru::class::string::RString::to_string::h09b7bec462ad9901 in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_cObject", referenced from:
                ruru::class::class::Class::new::h7ecefae67db17fd3 in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
                ruru::result::Error::to_exception::h2f41c15369670e97 in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_const_get", referenced from:
                ruru::binding::util::get_constant::h6812aa75747dad0d in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_str_new", referenced from:
                ruru::class::string::RString::new::h76c8de6d621f5b34 in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_raise", referenced from:
                ruru::class::vm::VM::raise::hb57c486174abc78c in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_funcallv", referenced from:
                ruru::binding::util::call_method::h54a6d4cac0a6d4db in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_define_singleton_method", referenced from:
                _init_t12r in t12r.0.o
            "_rb_intern", referenced from:
                ruru::binding::util::get_constant::h6812aa75747dad0d in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
                ruru::binding::util::call_method::h54a6d4cac0a6d4db in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_define_class", referenced from:
                ruru::class::class::Class::new::h7ecefae67db17fd3 in libruru-201f3ed9a5276df2.rlib(ruru-201f3ed9a5276df2.0.o)
            "_rb_hash_foreach", referenced from:
                _transliterate in t12r.0.o
          ld: symbol(s) not found for architecture x86_64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

Why? Because we're not passing a dynamic linking reference to libruby anymore and we're not properly resolving the static library reference since:

  1. The above patch isn't there
  2. There is no static libruby on the Travis machine

What's interesting about all of this is that when you look at how Ruby native extensions are actually built what you see is that they always pass these linker flags in and basically never directly link to libruby. Instead it appears that because the Ruby interpreter itself has these symbols incorporated into it, at runtime, when the C-extensions are dynamically loaded the undefined symbols are resolved.

In other words, if you're actually executing extensions built with ruru you never actually have to link to libruby it's only if you're trying to execute Rust code without it being executed through the Ruby interpreter (like the ruru tests).

The reason your t12r CI passes is because the tests are written in Ruby, so the code always has the presence of the Ruby interpreter.

In other words it's actually kind of good news. We shouldn't actually be involving libruby in the linking process at all and, instead, just passing the flags from DLDFLAGS and LIBS as does as does mkmf. In other words what it looks like is that libruby shouldn't actually matter in a compiled extension!

EDIT

You can confirm that t12r works even without linking against libruby by just installing a version of Ruby without a static Ruby library installed and building and running its tests with ruby-sys built in static mode (as Travis is doing).

@andrewstucki
Copy link

Just did a further validation that the former explanation is indeed the case. Forked ruby-sys, ruru, and faster_path, adjusted the Cargo references, and built without linking to libruby. While tests written in Rust won't actually compile, bundle exec rake thermite:build && bundle exec rake mspec_full works like a charm. Furthermore:

➜  faster_path git:(master) ✗ otool -L lib/faster_path.so
lib/faster_path.so:
	/Users/andrew/faster_path/target/release/deps/libfaster_path.dylib (compatibility version 0.0.0, current version 0.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.60.2)
	/usr/local/opt/gmp/lib/libgmp.10.dylib (compatibility version 14.0.0, current version 14.2.0)
	/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
	/usr/lib/libresolv.9.dylib (compatibility version 1.0.0, current version 1.0.0)

So the limitation of not linking in libruby means that Rust code invoking methods in ruby-sys will fail when run as standalone tests, but the resulting binary (built with -Wl,-undefined,dynamic_lookup) will work perfectly fine when dynamically loaded by the Ruby interpreter.

If you want to recreate, just build the gem and run mspec for https://github.com/andrewstucki/faster_path.

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.

3 participants