Misc cargo builder improvements#5459
Conversation
deivid-rodriguez
force-pushed
the
cargo-builder-target
branch
from
b5c802b to
0b5f6c0
Compare
| end | ||
|
|
||
| def self.run(command, results, command_name = nil, dir = Dir.pwd) | ||
| def self.run(command, results, command_name = nil, dir = Dir.pwd, env = {}) |
There was a problem hiding this comment.
When cross-compiling with multiple ruby versions present, it's necessary to pass information about RbConfig to cargo so we can ensure proper linking. I found using environment variables is a simple and effective way to do this.
There was a problem hiding this comment.
If I understand it well, this argument is not used directly by RubyGems now. Can you confirm?
There was a problem hiding this comment.
As far as I understand, all RubyGems does is pass on the given environment to cargo. This feels nit to me, because it's more explicit than setting up the environment globally just so it is inherited by the cargo subprocess.
There was a problem hiding this comment.
RubyGems does not use this, other than Cargo builder. It allow us to pass env variables without setting them globally for the entire process.
| build_env | ||
| end | ||
|
|
||
| def cargo_command(cargo_dir, dest_path, args = []) |
There was a problem hiding this comment.
I want to expose the actual cargo command to run, so it can be used in a Makefile later.
|
Not sure why, but it looks like the Windows actions were interrupted causing CI to ❌. @simi could you trigger a re-run? |
|
@ianks Sorry for dropping the ball here, I will do my best to get this reviewed and shipped with the next release! |
|
All good @deivid-rodriguez! I need to make a couple of adjustments on my end as well to get CI passing |
|
@deivid-rodriguez Should be good to go. |
| assert_match "Finished release [optimized] target(s)", output | ||
| assert_ffi_handle bundle, 'Init_rust_ruby_example' | ||
| rescue Exception => e | ||
| pp output if output |
There was a problem hiding this comment.
Yes, it really does make all of the difference when debugging. I can remove it if you prefer though!
There was a problem hiding this comment.
Looks strange to me too, but I can see how it's useful, and the code was already there, so ok!
| [dependencies] | ||
| # Needed until bindgen has the `allowlist_file` feature | ||
| rb-sys = { git = "https://github.com/ianks/rb-sys", tag = "v0.9.0" } | ||
| rb-sys = "0.9.3" |
|
I have left minor comments. Otherwise looks good to me. |
deivid-rodriguez
left a comment
deivid-rodriguez
left a comment
There was a problem hiding this comment.
If @simi is fine with this, I'm fine too :)
|
Now I need to finish support for |
Misc cargo builder improvements (cherry picked from commit 632a8bc)
3 participants
Hey there!
After battle testing the cargo code which making Rust play nice with rake-compiler-dock, I've come up with some improvements that will fix some bugs, and make the development experience a bit better all around. Here's some of the things I adjusted
CARGO_BUILD_TARGETis detected-l:namespecformrunnerfor the extension builder, so I can print out logs more easily when debugging / developingRbConfig::CONFIGvalues and environment variables when invokingcargo rustc, so gem extensions do not have to figure out have to invoke the correct ruby binary to print out the correct configWith all of these changes, I am able to cross compile Rust extensions on the following platforms (ruby 2.4, 2.5, 2.6, 2.7, 3.0, 3.1):