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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 13, 2019
Merged

[Feature] install ruby if none exists #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d987f93
[Feature] install ruby if none exists
Dec 13, 2019
492dca3
fixing for review
Dec 13, 2019
2b01905
better failure report
Dec 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .ruby-version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2.6.5
1 change: 1 addition & 0 deletions examples/simple_script/.ruby-version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2.6.5
5 changes: 3 additions & 2 deletions examples/simple_script/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ruby_binary(
main = "script.rb",
deps = [
"//lib:foo",
"@bundle//:libs",
"@bundle//:awesome_print",
],
)

Expand All @@ -26,6 +26,7 @@ ruby_test(
rubyopt = ["-rrspec/autorun"], # require autorun because it is needed
deps = [
"//lib:foo",
"@bundle//:libs",
"@bundle//:awesome_print",
"@bundle//:rspec",
],
)
1 change: 1 addition & 0 deletions examples/simple_script/Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ source "https://rubygems.org"
git_source(:github) { |repo_name| "https://github.com/#{repo_name}" }

gem 'rspec', '~> 3.7.0'
gem 'awesome_print'
4 changes: 3 additions & 1 deletion examples/simple_script/Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
GEM
remote: https://rubygems.org/
specs:
awesome_print (1.8.0)
diff-lcs (1.3)
rspec (3.7.0)
rspec-core (~> 3.7.0)
Expand All @@ -20,7 +21,8 @@ PLATFORMS
ruby

DEPENDENCIES
awesome_print
rspec (~> 3.7.0)

BUNDLED WITH
1.16.1
1.17.3
3 changes: 2 additions & 1 deletion examples/simple_script/WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ load(

ruby_rules_dependencies()

ruby_register_toolchains()
ruby_register_toolchains(version = "2.6.5")

load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")

Expand All @@ -27,4 +27,5 @@ bundle_install(
name = "bundle",
gemfile = "//:Gemfile",
gemfile_lock = "//:Gemfile.lock",
version = "2.0.2",
)
5 changes: 5 additions & 0 deletions examples/simple_script/script.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@

require 'openssl'
require 'lib/foo'
require "awesome_print"

def oss_rand
OpenSSL::BN.rand(512).to_s
end

puts Foo.aha + " " + oss_rand

puts $LOAD_PATH

ap Class
55 changes: 39 additions & 16 deletions ruby/private/bundle/bundle.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,39 @@
load("//ruby/private:constants.bzl", "RULES_RUBY_WORKSPACE_NAME")

def _get_interpreter_label(repository_ctx, ruby_sdk):
# TODO(yugui) Support windows as rules_nodejs does
return Label("%s//:ruby" % ruby_sdk)
def install_bundler(ctx, interpreter, install_bundler, dest, version):
args = ["env", "-i", interpreter, install_bundler, version, dest]
environment = {"RUBYOPT": "--disable-gems"}

def _get_bundler_label(repository_ctx, ruby_sdk):
# TODO(yugui) Support windows as rules_nodejs does
return Label("%s//:bundler/exe/bundler" % ruby_sdk)

def _get_bundler_lib_label(repository_ctx, ruby_sdk):
# TODO(yugui) Support windows as rules_nodejs does
return Label("%s//:bundler/lib" % ruby_sdk)
result = ctx.execute(args, environment = environment)
if result.return_code:
message = "Failed to evaluate ruby snippet with {}: {}".format(
interpreter,
result.stderr,
)
fail(message)

def bundle_install_impl(ctx):
ctx.symlink(ctx.attr.gemfile, "Gemfile")
ctx.symlink(ctx.attr.gemfile_lock, "Gemfile.lock")
ctx.symlink(ctx.attr._create_bundle_build_file, "create_bundle_build_file.rb")
ctx.symlink(ctx.attr._install_bundler, "install_bundler.rb")

# TODO(kig) Make Gemspec reference from Gemfile actually work
if ctx.attr.gemspec:
ctx.symlink(ctx.attr.gemspec, ctx.path(ctx.attr.gemspec).basename)

ruby = _get_interpreter_label(ctx, ctx.attr.ruby_sdk)
bundler = _get_bundler_label(ctx, ctx.attr.ruby_sdk)
ruby = ctx.attr.ruby_interpreter
interpreter_path = ctx.path(ruby)

install_bundler(
ctx,
interpreter_path,
"install_bundler.rb",
"bundler",
ctx.attr.version,
)

bundler = Label("//:bundler/exe/bundler")

# Install the Gems into the workspace
args = [
Expand All @@ -31,8 +42,8 @@ def bundle_install_impl(ctx):
ctx.path(ruby), # ruby
"--disable-gems", # prevent the addition of gem installation directories to the default load path
"-I", # Used to tell Ruby where to load the library scripts
ctx.path(bundler).dirname.dirname.get_child("lib"),
ctx.path(bundler), # run
"bundler/lib",
"bundler/exe/bundler", # run
"install", # > bundle install
"--deployment", # In the deployment mode, gems are dumped to --path and frozen; also .bundle/config file is created
"--standalone", # Makes a bundle that can work without depending on Rubygems or Bundler at runtime.
Expand All @@ -58,7 +69,7 @@ def bundle_install_impl(ctx):
ctx.path(ruby), # ruby interpreter
"--disable-gems", # prevent the addition of gem installation directories to the default load path
"-I", # -I lib (adds this folder to $LOAD_PATH where ruby searchesf for things)
ctx.path(bundler).dirname.dirname.get_child("lib"),
"bundler/lib",
"create_bundle_build_file.rb", # The template used to created bundle file
"BUILD.bazel", # Bazel build file (can be empty)
"Gemfile.lock", # Gemfile.lock where we list all direct and transitive dependencies
Expand All @@ -85,20 +96,32 @@ bundle_install = repository_rule(
implementation = bundle_install_impl,
attrs = {
"ruby_sdk": attr.string(
default = "@org_ruby_lang_ruby_host",
default = "@org_ruby_lang_ruby_toolchain",
),
"ruby_interpreter": attr.label(
default = "@org_ruby_lang_ruby_toolchain//:ruby",
),
"gemfile": attr.label(
allow_single_file = True,
),
"gemfile_lock": attr.label(
allow_single_file = True,
),
"version": attr.string(
default = "2.0.2",
),
"gemspec": attr.label(
allow_single_file = True,
),
"excludes": attr.string_list_dict(
doc = "List of glob patterns per gem to be excluded from the library",
),
"_install_bundler": attr.label(
default = "%s//ruby/private/bundle:install_bundler.rb" % (
RULES_RUBY_WORKSPACE_NAME
),
allow_single_file = True,
),
"_create_bundle_build_file": attr.label(
default = "%s//ruby/private/bundle:create_bundle_build_file.rb" % (
RULES_RUBY_WORKSPACE_NAME
Expand Down
0 ruby/private/install_bundler.rb → ruby/private/bundle/install_bundler.rb
View file
Open in desktop
File renamed without changes.
13 changes: 0 additions & 13 deletions ruby/private/bundler.bzl
View file
Open in desktop

This file was deleted.

23 changes: 14 additions & 9 deletions ruby/private/sdk.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
load(
":host_runtime.bzl",
_ruby_host_runtime = "ruby_host_runtime",
"@bazelruby_ruby_rules//ruby/private/toolchains:ruby_runtime.bzl",
_ruby_runtime = "ruby_runtime",
)

def _register_host_runtime():
_ruby_host_runtime(name = "org_ruby_lang_ruby_host")
def ruby_register_toolchains(version = "host"):
"""Registers ruby toolchains in the WORKSPACE file."""

supported_versions = ["host", "2.6.3", "2.6.5"]
if version in supported_versions:
_ruby_runtime(
name = "org_ruby_lang_ruby_toolchain",
Copy link
Contributor

@yugui yugui Dec 15, 2019 •
edited
Loading

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.

Copy link
Contributor Author

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.

  1. Cross compiling an interpreter
  2. 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 toolchainsis_hostisFalse`.

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.

Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome.

version = version,
)
else:
fail("ruby_register_toolchains: unsupported ruby version '%s' not in '%s'" % (version, supported_versions))

native.register_toolchains(
"@org_ruby_lang_ruby_host//:ruby_host",
"@org_ruby_lang_ruby_toolchain//:toolchain",
)

def ruby_register_toolchains():
"""Registersr ruby toolchains in the WORKSPACE file."""
_register_host_runtime()
9 changes: 0 additions & 9 deletions ruby/private/toolchain.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ def _ruby_toolchain_impl(ctx):
return [platform_common.ToolchainInfo(
ruby_runtime = RubyRuntimeInfo(
interpreter = ctx.attr.interpreter,
bundler = ctx.attr.bundler,
runtime = ctx.files.runtime,
rubyopt = ctx.attr.rubyopt,
is_host = ctx.attr.is_host,
Expand All @@ -31,12 +30,6 @@ _ruby_toolchain = rule(
executable = True,
cfg = "target",
),
"bundler": attr.label(
mandatory = True,
allow_files = True,
executable = True,
cfg = "target",
),
"runtime": attr.label(
mandatory = True,
allow_files = True,
Expand All @@ -52,7 +45,6 @@ _ruby_toolchain = rule(
def ruby_toolchain(
name,
interpreter,
bundler,
runtime,
rubyopt = [],
is_host = False,
Expand All @@ -62,7 +54,6 @@ def ruby_toolchain(
_ruby_toolchain(
name = impl_name,
interpreter = interpreter,
bundler = bundler,
runtime = runtime,
rubyopt = rubyopt,
is_host = is_host,
Expand Down
1 change: 1 addition & 0 deletions ruby/private/toolchains/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package(default_visibility = ["//ruby/private:__pkg__"])
42 changes: 1 addition & 41 deletions ruby/private/BUILD.host_runtime.tpl → ruby/private/toolchains/BUILD.runtime.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,8 @@ load(
package(default_visibility = ["//visibility:public"])

ruby_toolchain(
name = "ruby_host",
name = "toolchain",
interpreter = "//:ruby_bin",
bundler = "//:bundler",
rubyopt = [
"-I$(RUNFILES_DIR)/org_ruby_lang_ruby_host/bundler/lib",
],
runtime = "//:runtime",
is_host = True,
rules_ruby_workspace = "{rules_ruby_workspace}",
Expand All @@ -27,36 +23,6 @@ sh_binary(
data = [":runtime"],
)

sh_binary(
name = "irb",
srcs = ["irb_bin"],
)

sh_binary(
name = "erb",
srcs = ["erb_bin"],
)

sh_binary(
name = "rake",
srcs = ["rake_bin"],
)

sh_binary(
name = "rdoc",
srcs = ["rdoc_bin"],
)

sh_binary(
name = "ri",
srcs = ["ri_bin"],
)

sh_binary(
name = "gem",
srcs = ["gem_bin"],
)

cc_import(
name = "libruby",
hdrs = glob({hdrs}),
Expand All @@ -70,12 +36,6 @@ cc_library(
hdrs = glob({hdrs}),
)

filegroup(
name = "bundler",
srcs = ["bundler/exe/bundler"],
data = glob(["bundler/**/*.rb"]),
)

filegroup(
name = "runtime",
srcs = glob(
Expand Down
3 changes: 1 addition & 2 deletions ruby/private/interpreter_wrapper.tpl → ...rivate/toolchains/interpreter_wrapper.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ else
PATH_PREFIX=`cd $(dirname $0); pwd`/
fi

exec ${PATH_PREFIX}{rel_interpreter_path} "$@"

exec ${PATH_PREFIX}{rel_interpreter_path} "$@"
Loading