-
Notifications
You must be signed in to change notification settings - Fork 35
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
[Feature] install ruby if none exists #34
Conversation
3574bbf
to
d987f93
Compare
supported_versions = ["host", "2.6.3", "2.6.5"] | ||
if version in supported_versions: | ||
_ruby_runtime( | ||
name = "org_ruby_lang_ruby_toolchain", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes cross build impossible.
So it is still better to call this runtime org_ruby_lang_ruby_host
or ruby_runtime_host
or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yugui I think I am getting confused between two concepts.
- Cross compiling an interpreter
- Setting up an interpreter in the toolchain
As I understand it:
I can't think of a good reason to have a ruby interpreter in the toolchain that is compiled for another architecture. This would make running things like bundler
rspec
and rubocop
impossible because the toolchains ruby would not be able to run with something like bazel run :test
As I see in your latest changes to ruby_binary
you see the option of having host
as being more like dont_include_interpreter
, i.e. 1ruby_binary(...)would include the ruby interpreter if the toolchains
is_hostis
False`.
I think we should separate these concepts, and maybe introduce a rule ruby_runtime(version="2.5", arch="x64")
or something that we can put into a binary to bundle them together, e.g.
ruby_runtime(name="ruby_2_6_3", version="2.6.3")
ruby_binary(
...
runtime=":ruby_2_6_3"
)
For the majority of our use-cases (mostly due to the complexity of cross compilation + native gem building) we don't want the interpreter bundled with our binaries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your answer.
To be brief, now I agree to your naming org_ruby_lang_ruby_toolchain
.
I can't think of a good reason to have a ruby interpreter in the toolchain that is compiled for another architecture.
- IIUC, Bazel can distinguish the interpreter for the build environment and the foreign environments as far as their target platforms are correctly configured.
- Ruby allows us to cross-compile extensions with the headers and
rbconfig.rb
for the target platform
It is certainly a little hard to setup up a cross-compile environment, personally I'd like to keep the door open.
you see the option of having host as being more like
dont_include_interpreter
Right. It is better to rename the option.
ruby_runtime(version="2.5", arch="x64")
Good point. It is definitely a better API.
Also I started think we needed to distinguish toolchain and runtime.
- toolchain is used to fetch gems; and interpret helper ruby scripts, and provides a primary runtime
- runtime is used to build C extensions with it and optionally included into the
ruby_binary
.
Considering that, it should be safe to call the toolchain just org_ruby_lang_ruby_toolchain
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome.
install_path = "./build" | ||
print("./bin/ruby-build", "--verbose", version, install_path) | ||
ctx.execute( | ||
["./bin/ruby-build", "--verbose", version, install_path], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good enough to start with. But could you let me know what was wrong with #20?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yugui My arrival at this PR was:
- First I tried to use sorbets ruby BUILD file, which I got mostly working, there was one main issue with that approach, the bazel BUILD was executed after repository download (e.g. bundler) so then we would have to reimplement all scripts in
repository_rules
to be python or something, including bundler. - Next I tried building ruby directly from source. I also got this working, but there are load of complications, especially openssl integration requires downloading and compiling. I gave up on this when I realized the amount of complexity in building ruby.
ruby-build
removes a lot of that complexity by setting up all the args, and fixes other problems like finding and/or building libraries like zlib and openssl and if it can't find them. The fact that this project exists and is so popular demonstrates the complexity in building ruby.
Ultimately, the main issue with ruby is that there is no good distribution of pre-built ruby binaries for us to just download and use. This might be the ultimate solution to try and build "blessed" ruby versions to be distributed.
This solution has some nice value, a user can now build against any ruby version without having to install a tool like rbenv. But if the local version installed is good then the rules can just use that without having to build a new version.
This PR uses the version given to
ruby_register_toolchains(version="2.6.5")
to download the ruby version if necessary.ruby_register_toolchains
version
:host
which will use the currently installed rubyThe goals are:
The way it installs ruby is using the
rbenv/ruby-build
project, this is because building directly from source is pretty complicated, andruby-build
has made it a lot easier.This PR also:
bundle
toolchain
directory