From 27bcdd09276fb1152b727a79cfb354b26259cf50 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Sun, 5 Jan 2020 17:17:04 -0800 Subject: [PATCH 1/2] This PR refactors bundle steps and :bin generation --- examples/simple_script/BUILD.bazel | 11 +- ruby/private/binary_wrapper.tpl | 2 +- ruby/private/bundle/bundle.bzl | 65 ++++++----- .../bundle/create_bundle_build_file.rb | 108 ++++++++++++------ ruby/private/rspec.bzl | 1 + ruby/tests/BUILD.bazel | 2 +- ruby/tests/container_test.sh | 9 +- 7 files changed, 120 insertions(+), 78 deletions(-) diff --git a/examples/simple_script/BUILD.bazel b/examples/simple_script/BUILD.bazel index 0c82763..94bd0b1 100644 --- a/examples/simple_script/BUILD.bazel +++ b/examples/simple_script/BUILD.bazel @@ -54,7 +54,7 @@ ruby_binary( deps = [ ":lib", "//lib:foo", - "@bundle//:gems", + "@bundle//:bin", ], ) @@ -77,10 +77,7 @@ ruby_test( main = "spec/spec_helper.rb", rubyopt = ["-rrspec/autorun"], deps = [ - "@bundle//:awesome_print", - "@bundle//:colored2", - "@bundle//:rspec", - "@bundle//:rspec-its", + "@bundle//:gems", ], ) @@ -103,6 +100,7 @@ ruby_test( main = "@bundle//:bin/rspec", deps = [ "@bundle//:awesome_print", + "@bundle//:bin", "@bundle//:colored2", "@bundle//:rspec", "@bundle//:rspec-its", @@ -138,7 +136,6 @@ ruby_binary( srcs = [ ".relaxed-rubocop-2.4.yml", ".rubocop.yml", - "@bundle//:bin/rubocop", ], args = [ "-c", @@ -150,6 +147,6 @@ ruby_binary( deps = [ ":lib", "//lib:foo", - "@bundle//:rubocop", + "@bundle//:bin", ], ) diff --git a/ruby/private/binary_wrapper.tpl b/ruby/private/binary_wrapper.tpl index bfedfff..70e1ce7 100644 --- a/ruby/private/binary_wrapper.tpl +++ b/ruby/private/binary_wrapper.tpl @@ -98,7 +98,7 @@ def main(args) loadpaths = create_loadpath_entries(custom_loadpaths, runfiles) loadpaths += get_repository_imports(runfiles) loadpaths += ENV['RUBYLIB'].split(':') if ENV.key?('RUBYLIB') - ENV['RUBYLIB'] = loadpaths.join(':') + ENV['RUBYLIB'] = loadpaths.sort.uniq.join(':') runfiles_envkey, runfiles_envvalue = runfiles_envvar(runfiles) ENV[runfiles_envkey] = runfiles_envvalue if runfiles_envkey diff --git a/ruby/private/bundle/bundle.bzl b/ruby/private/bundle/bundle.bzl index 9c56e57..afe51ce 100644 --- a/ruby/private/bundle/bundle.bzl +++ b/ruby/private/bundle/bundle.bzl @@ -13,19 +13,23 @@ load("//ruby/private/tools:deprecations.bzl", "deprecated_attribute") # Runs bundler with arbitrary arguments # eg: run_bundler(runtime_ctx, [ "lock", " --gemfile", "Gemfile.rails5" ]) -def run_bundler(runtime_ctx, bundler_arguments): - #print("BUNDLE RUN", bundler_arguments) - +def run_bundler(runtime_ctx, bundler_arguments, previous_result): # Now we are running bundle install + bundler_command = bundler_arguments[0] + bundler_args = [] + + # add --verbose to all commands except install + if bundler_command != "install": + bundler_args += ["--verbose"] + + bundler_args += bundler_arguments[1:] + args = [ runtime_ctx.interpreter, # ruby - "--enable=gems", # bundler must run with rubygems enabled - "-I", - ".", "-I", # Used to tell Ruby where to load the library scripts BUNDLE_PATH, # Add vendor/bundle to the list of resolvers BUNDLE_BINARY, # our binary - ] + bundler_arguments + ] + [bundler_command] + bundler_args # print("Bundler Command:\n\n", args) @@ -41,7 +45,7 @@ def run_bundler(runtime_ctx, bundler_arguments): # $ bundle config --local | --global config-option config-value # # @config_category can be either 'local' or 'global' -def set_bundler_config(runtime_ctx, config_category = "local"): +def set_bundler_config(runtime_ctx, previous_result, config_category = "local"): # Bundler is deprecating various flags in favor of the configuration. # HOWEVER — for reasons I can't explain, Bazel runs "bundle install" *prior* # to setting these flags. So the flags are then useless until we can force the @@ -52,16 +56,20 @@ def set_bundler_config(runtime_ctx, config_category = "local"): bundler_config = { "deployment": "true", "standalone": "true", + "force": "false", + "redownload": "false", "frozen": "true", - "without": "development,test", "path": BUNDLE_PATH, "jobs": "20", + "shebang": runtime_ctx.interpreter, } - for option, value in bundler_config.items(): - args = ["config", "--%s" % (config_category), option, value] + last_result = previous_result - result = run_bundler(runtime_ctx, args) + for option, value in bundler_config.items(): + args = ["config", "set", "--%s" % (config_category), option, value] + result = run_bundler(runtime_ctx, args, last_result) + last_result = result if result.return_code: message = "Failed to set bundle config {} to {}: {}".format( option, @@ -70,8 +78,7 @@ def set_bundler_config(runtime_ctx, config_category = "local"): ) fail(message) - # The new way to generate binstubs is via the binstubs command, not config option. - return run_bundler(runtime_ctx, ["binstubs", "--path", BUNDLE_BIN_PATH]) + return last_result # This function is called "pure_ruby" because it downloads and unpacks the gem # file into a given folder, which for gems without C-extensions is the same @@ -96,6 +103,8 @@ def install_pure_ruby_gem(runtime_ctx, gem_name, gem_version, folder): result.stderr, ) fail(message) + else: + return result def install_bundler(runtime_ctx, bundler_version): return install_pure_ruby_gem( @@ -105,21 +114,26 @@ def install_bundler(runtime_ctx, bundler_version): "bundler", ) -def bundle_install(runtime_ctx): +def bundle_install(runtime_ctx, previous_result): result = run_bundler( runtime_ctx, [ - "install", # bundle install - "--standalone", # Makes a bundle that can work without depending on Rubygems or Bundler at runtime. - "--binstubs={}".format(BUNDLE_BIN_PATH), # Creates a directory and place any executables from the gem there. - "--path={}".format(BUNDLE_PATH), # The location to install the specified gems to. + "install", + "--binstubs={}".format(BUNDLE_BIN_PATH), + "--path={}".format(BUNDLE_PATH), + "--deployment", + "--standalone", + "--frozen", ], + previous_result, ) if result.return_code: fail("bundle install failed: %s%s" % (result.stdout, result.stderr)) + else: + return result -def generate_bundle_build_file(runtime_ctx): +def generate_bundle_build_file(runtime_ctx, previous_result): # Create the BUILD file to expose the gems to the WORKSPACE # USAGE: ./create_bundle_build_file.rb BUILD.bazel Gemfile.lock repo-name [excludes-json] workspace-name args = [ @@ -160,19 +174,16 @@ def _ruby_bundle_impl(ctx): ) # 1. Install the right version of the Bundler Gem - install_bundler(runtime_ctx, bundler_version) - - # Create label for the Bundler executable - bundler = Label("//:" + BUNDLE_BINARY) + result = install_bundler(runtime_ctx, bundler_version) # 2. Set Bundler config in the .bundle/config file - set_bundler_config(runtime_ctx) + result = set_bundler_config(runtime_ctx, result) # 3. Run bundle install - bundle_install(runtime_ctx) + result = bundle_install(runtime_ctx, result) # 4. Generate the BUILD file for the bundle - generate_bundle_build_file(runtime_ctx) + generate_bundle_build_file(runtime_ctx, result) ruby_bundle = repository_rule( implementation = _ruby_bundle_impl, diff --git a/ruby/private/bundle/create_bundle_build_file.rb b/ruby/private/bundle/create_bundle_build_file.rb index a23cd6f..9854fe8 100755 --- a/ruby/private/bundle/create_bundle_build_file.rb +++ b/ruby/private/bundle/create_bundle_build_file.rb @@ -1,7 +1,7 @@ #!/usr/bin/env ruby # frozen_string_literal: true -TEMPLATE = <<~MAIN_TEMPLATE +BUILD_HEADER = <<~MAIN_TEMPLATE load( "{workspace_name}//ruby:defs.bzl", "ruby_library", @@ -33,30 +33,33 @@ name = "{name}", srcs = glob( include = [ - "lib/ruby/{ruby_version}/gems/{name}-{version}/**/*", - "bin/*" + ".bundle/config", + "{gem_lib_files}", + {gem_binaries} ], exclude = {exclude}, ), deps = {deps}, includes = ["lib/ruby/{ruby_version}/gems/{name}-{version}/lib"], - rubyopt = ["{bundler_setup}"], ) GEM_TEMPLATE ALL_GEMS = <<~ALL_GEMS ruby_library( name = "gems", - srcs = glob( - {gems_lib_files}, - ), + srcs = glob([{gems_lib_files}]) + glob(["bin/*"]), includes = {gems_lib_paths}, - rubyopt = ["{bundler_setup}"], + ) + + ruby_library( + name = "bin", + srcs = glob(["bin/*"]), + deps = {gems_with_binaries} ) ALL_GEMS -GEM_LIB_PATH = ->(ruby_version, gem_name, gem_version) do - "lib/ruby/#{ruby_version}/gems/#{gem_name}-#{gem_version}/lib" +GEM_PATH = ->(ruby_version, gem_name, gem_version) do + "lib/ruby/#{ruby_version}/gems/#{gem_name}-#{gem_version}" end require 'bundler' @@ -84,7 +87,7 @@ def pink; colorize(35); end def light_blue; colorize(36); end - def orange; colorize(41); end + def orange; colorize(52); end # @formatter:on end @@ -117,6 +120,8 @@ def buildify! system("/usr/bin/env bash -c '#{command} 1>#{output_file} 2>&1'") code = $? + return unless File.exist?(output_file) + output = File.read(output_file).strip.gsub(Dir.pwd, '.').yellow begin FileUtils.rm_f(output_file) @@ -129,7 +134,7 @@ def buildify! else raise BuildifierFailedError, 'Generated BUILD file failed buildifier, with error — '.red + "\n\n" + - File.read(output_file).yellow + output.yellow end end end @@ -142,6 +147,10 @@ class BundleBuildFileGenerator :excludes, :ruby_version + DEFAULT_EXCLUDES = ['**/* *.*', '**/* */*'].freeze + + EXCLUDED_EXECUTABLES = %w(console setup).freeze + def initialize(workspace_name:, repo_name:, build_file: 'BUILD.bazel', @@ -161,35 +170,31 @@ def initialize(workspace_name:, def generate! # when we append to a string many times, using StringIO is more efficient. template_out = StringIO.new - - # In Bazel we want to use __FILE__ because __dir__points to the actual sources, and we are - # using symlinks here. - # - # rubocop:disable Style/ExpandPathArguments - bin_folder = File.expand_path('../bin', __FILE__) - binaries = Dir.glob("#{bin_folder}/*").map do |binary| - 'bin/' + File.basename(binary) if File.executable?(binary) - end - # rubocop:enable Style/ExpandPathArguments - - template_out.puts TEMPLATE + template_out.puts BUILD_HEADER .gsub('{workspace_name}', workspace_name) .gsub('{repo_name}', repo_name) .gsub('{ruby_version}', ruby_version) - .gsub('{binaries}', binaries.to_s) .gsub('{bundler_setup}', bundler_setup_require) # strip bundler version so we can process this file remove_bundler_version! + # Append to the end specific gem libraries and dependencies bundle = Bundler::LockfileParser.new(Bundler.read_file(gemfile_lock)) gem_lib_paths = [] - bundle.specs.each { |spec| register_gem(spec, template_out, gem_lib_paths) } + gems_binaries = {} # gem-name => [ gem's binaries ], ... + gems = bundle.specs.map(&:name) + + bundle.specs.each { |spec| register_gem(spec, template_out, gem_lib_paths, gems_binaries) } template_out.puts ALL_GEMS - .gsub('{gems_lib_files}', gem_lib_paths.map { |p| "#{p}/**/*.rb" }.to_s) + .gsub('{gems_lib_files}', to_flat_string(gem_lib_paths.map { |p| "#{p}/**/*" })) + .gsub('{gems_binaries}', gems_binaries.values.flatten.to_s) .gsub('{gems_lib_paths}', gem_lib_paths.to_s) .gsub('{bundler_setup}', bundler_setup_require) + .gsub('{gems_deps}', gems.map { |g| ":#{g}" }.to_s) + .gsub('{gems_with_binaries}', gems_binaries.keys.map { |g| ":#{g}" }.to_s) + .gsub('{exclude}', DEFAULT_EXCLUDES.to_s) ::File.open(build_file, 'w') { |f| f.puts template_out.string } end @@ -216,17 +221,23 @@ def remove_bundler_version! ::FileUtils.move(temp_gemfile_lock, gemfile_lock, force: true) end - def register_gem(spec, template_out, gem_lib_paths) - gem_lib_paths << GEM_LIB_PATH[ruby_version, spec.name, spec.version] + def register_gem(spec, template_out, gem_lib_paths, gems_binaries) + gem_path = GEM_PATH[ruby_version, spec.name, spec.version] + gem_lib_paths << gem_lib_path = gem_path + '/lib' + + # paths to search for executables + gem_binaries = find_gems_binaries(gem_path) + gems_binaries[spec.name] = gem_binaries unless gem_binaries.nil? || gem_binaries.empty? + deps = spec.dependencies.map { |d| ":#{d.name}" } - deps += [':bundler_setup'] - exclude_array = excludes[spec.name] || [] - # We want to exclude files and folder with spaces in them - exclude_array += ['**/* *.*', '**/* */*'] + warn("registering gem #{spec.name} with binaries: #{gem_binaries}") if gems_binaries.key?(spec.name) template_out.puts GEM_TEMPLATE - .gsub('{exclude}', exclude_array.to_s) + .gsub('{gem_lib_path}', gem_lib_path) + .gsub('{gem_lib_files}', gem_lib_path + '/**/*') + .gsub('{gem_binaries}', to_flat_string(gem_binaries)) + .gsub('{exclude}', exclude_array(spec.name).to_s) .gsub('{name}', spec.name) .gsub('{version}', spec.version.to_s) .gsub('{deps}', deps.to_s) @@ -234,6 +245,30 @@ def register_gem(spec, template_out, gem_lib_paths) .gsub('{ruby_version}', ruby_version) .gsub('{bundler_setup}', bundler_setup_require) end + + def find_gems_binaries(gem_path) + gem_bin_paths = %W(#{gem_path}/bin #{gem_path}/exe) + + gem_bin_paths + .map do |bin_path| + Dir # grab all files under bin/ and exe/ inside the gem folder + .glob("#{bin_path}/*") # convert to File object + .map { |b| f = File.new(b); File.executable?(f) ? f : nil } + .compact # remove non-executables, take basename, minus binary defaults + .map { |f| File.basename(f.path) } - EXCLUDED_EXECUTABLES # that bundler installs with bundle gem e - warn("ERROR running buildifier on the generated build file [#{build_file}] ➔ \n#{e.message.orange}") + warn("ERROR running buildifier on the generated build file [#{build_file}] ➔ #{e.message.orange}") end end diff --git a/ruby/private/rspec.bzl b/ruby/private/rspec.bzl index 334d9c3..e86603b 100644 --- a/ruby/private/rspec.bzl +++ b/ruby/private/rspec.bzl @@ -46,6 +46,7 @@ def ruby_rspec( rspec_gems = ["%s:%s" % (bundle, gem) for gem in DEFAULT_RSPEC_GEMS] deps += rspec_gems + deps += ["%s:bin" % bundle] ruby_rspec_test( name = name, diff --git a/ruby/tests/BUILD.bazel b/ruby/tests/BUILD.bazel index d48f6be..f1fe900 100644 --- a/ruby/tests/BUILD.bazel +++ b/ruby/tests/BUILD.bazel @@ -111,7 +111,7 @@ sh_test( # 6a) case 3 in the context of a genrule # # Also all of the cases above must correctly configure environment variables -# so that their subprocesses whose binaries are generated by Bazel can run with +# so that their sub-processes with binaries generated by Bazel can run with # their runfiles. ruby_binary( diff --git a/ruby/tests/container_test.sh b/ruby/tests/container_test.sh index 35326ac..bb827c2 100755 --- a/ruby/tests/container_test.sh +++ b/ruby/tests/container_test.sh @@ -7,15 +7,16 @@ if [[ $# -lt 2 ]]; then exit 1 fi -# check if we are running inside a Docker container, and skip this test if so. -if [[ -n "$(docker info 2>/dev/null| grep 'Cannot connect')" ]]; then +# check if we are running without access to Docker Server (eg, on CI +# within its own Docker container) and if so — skip this test. +if [[ -n "$(docker info 2>/dev/null | grep 'Cannot connect')" ]]; then echo "No Docker runtime detected, skipping tests." exit 0 else CONTAINER_IMAGE_LOADER="$1" CONTAINER_IMAGE_NAME="$2" - if [[ -n $(command -v ${CONTAINER_IMAGE_LOADER}) ]] ; then + if [[ -n $(command -v ${CONTAINER_IMAGE_LOADER}) ]]; then ${CONTAINER_IMAGE_LOADER} docker run "${CONTAINER_IMAGE_NAME}" else @@ -23,5 +24,3 @@ else exit 2 fi fi - - From ecf0fc5d1e065c7027474598cd3d3439c42a2702 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Thu, 16 Jan 2020 14:06:23 -0800 Subject: [PATCH 2/2] Fixing bad naming by replacing gems_.. with bundle_... --- examples/simple_script/.ruby-version | 2 +- examples/simple_script/WORKSPACE | 2 +- .../bundle/create_bundle_build_file.rb | 38 +++++++++---------- ruby/tests/container_test.sh | 2 +- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/examples/simple_script/.ruby-version b/examples/simple_script/.ruby-version index 6816713..57cf282 100644 --- a/examples/simple_script/.ruby-version +++ b/examples/simple_script/.ruby-version @@ -1 +1 @@ -2.6.5 \ No newline at end of file +2.6.5 diff --git a/examples/simple_script/WORKSPACE b/examples/simple_script/WORKSPACE index 9883f02..b57a0eb 100644 --- a/examples/simple_script/WORKSPACE +++ b/examples/simple_script/WORKSPACE @@ -21,10 +21,10 @@ load("@bazelruby_ruby_rules//ruby:defs.bzl", "ruby_bundle") ruby_bundle( name = "bundle", + bundler_version = "2.1.2", excludes = { "mini_portile": ["test/**/*"], }, gemfile = "//:Gemfile", gemfile_lock = "//:Gemfile.lock", - version = "2.0.2", ) diff --git a/ruby/private/bundle/create_bundle_build_file.rb b/ruby/private/bundle/create_bundle_build_file.rb index 9854fe8..ac95acf 100755 --- a/ruby/private/bundle/create_bundle_build_file.rb +++ b/ruby/private/bundle/create_bundle_build_file.rb @@ -47,14 +47,14 @@ ALL_GEMS = <<~ALL_GEMS ruby_library( name = "gems", - srcs = glob([{gems_lib_files}]) + glob(["bin/*"]), - includes = {gems_lib_paths}, + srcs = glob([{bundle_lib_files}]) + glob(["bin/*"]), + includes = {bundle_lib_paths}, ) ruby_library( name = "bin", srcs = glob(["bin/*"]), - deps = {gems_with_binaries} + deps = {bundle_with_binaries} ) ALL_GEMS @@ -180,20 +180,20 @@ def generate! remove_bundler_version! # Append to the end specific gem libraries and dependencies - bundle = Bundler::LockfileParser.new(Bundler.read_file(gemfile_lock)) - gem_lib_paths = [] - gems_binaries = {} # gem-name => [ gem's binaries ], ... - gems = bundle.specs.map(&:name) + bundle = Bundler::LockfileParser.new(Bundler.read_file(gemfile_lock)) + bundle_lib_paths = [] + bundle_binaries = {} # gem-name => [ gem's binaries ], ... + gems = bundle.specs.map(&:name) - bundle.specs.each { |spec| register_gem(spec, template_out, gem_lib_paths, gems_binaries) } + bundle.specs.each { |spec| register_gem(spec, template_out, bundle_lib_paths, bundle_binaries) } template_out.puts ALL_GEMS - .gsub('{gems_lib_files}', to_flat_string(gem_lib_paths.map { |p| "#{p}/**/*" })) - .gsub('{gems_binaries}', gems_binaries.values.flatten.to_s) - .gsub('{gems_lib_paths}', gem_lib_paths.to_s) + .gsub('{bundle_lib_files}', to_flat_string(bundle_lib_paths.map { |p| "#{p}/**/*" })) + .gsub('{bundle_with_binaries}', bundle_binaries.keys.map { |g| ":#{g}" }.to_s) + .gsub('{bundle_binaries}', bundle_binaries.values.flatten.to_s) + .gsub('{bundle_lib_paths}', bundle_lib_paths.to_s) .gsub('{bundler_setup}', bundler_setup_require) - .gsub('{gems_deps}', gems.map { |g| ":#{g}" }.to_s) - .gsub('{gems_with_binaries}', gems_binaries.keys.map { |g| ":#{g}" }.to_s) + .gsub('{bundle_deps}', gems.map { |g| ":#{g}" }.to_s) .gsub('{exclude}', DEFAULT_EXCLUDES.to_s) ::File.open(build_file, 'w') { |f| f.puts template_out.string } @@ -221,17 +221,17 @@ def remove_bundler_version! ::FileUtils.move(temp_gemfile_lock, gemfile_lock, force: true) end - def register_gem(spec, template_out, gem_lib_paths, gems_binaries) + def register_gem(spec, template_out, bundle_lib_paths, bundle_binaries) gem_path = GEM_PATH[ruby_version, spec.name, spec.version] - gem_lib_paths << gem_lib_path = gem_path + '/lib' + bundle_lib_paths << gem_lib_path = gem_path + '/lib' # paths to search for executables - gem_binaries = find_gems_binaries(gem_path) - gems_binaries[spec.name] = gem_binaries unless gem_binaries.nil? || gem_binaries.empty? + gem_binaries = find_bundle_binaries(gem_path) + bundle_binaries[spec.name] = gem_binaries unless gem_binaries.nil? || gem_binaries.empty? deps = spec.dependencies.map { |d| ":#{d.name}" } - warn("registering gem #{spec.name} with binaries: #{gem_binaries}") if gems_binaries.key?(spec.name) + warn("registering gem #{spec.name} with binaries: #{gem_binaries}") if bundle_binaries.key?(spec.name) template_out.puts GEM_TEMPLATE .gsub('{gem_lib_path}', gem_lib_path) @@ -246,7 +246,7 @@ def register_gem(spec, template_out, gem_lib_paths, gems_binaries) .gsub('{bundler_setup}', bundler_setup_require) end - def find_gems_binaries(gem_path) + def find_bundle_binaries(gem_path) gem_bin_paths = %W(#{gem_path}/bin #{gem_path}/exe) gem_bin_paths diff --git a/ruby/tests/container_test.sh b/ruby/tests/container_test.sh index bb827c2..c19180a 100755 --- a/ruby/tests/container_test.sh +++ b/ruby/tests/container_test.sh @@ -9,7 +9,7 @@ fi # check if we are running without access to Docker Server (eg, on CI # within its own Docker container) and if so — skip this test. -if [[ -n "$(docker info 2>/dev/null | grep 'Cannot connect')" ]]; then +if [[ -z $(command -v docker) || -n "$(docker info 2>/dev/null | grep 'Cannot connect')" ]]; then echo "No Docker runtime detected, skipping tests." exit 0 else