diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ec9bc4d..dfc25be2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ ## 2.1.2 +- #649: fix: further improve isolation of warbled jars/wars from host bundler configuration - #648: fix: further improve isolation of warbled jars/wars from user/host gem paths +- #465: fix: remove duplication of bundler path gems in warbled jars/wars +- #331: fix: correct support for `gem_excludes` inside bundler git specs - chore: relax jruby-rack requirement to allow compatibility with upcoming 2.0.x ## 2.1.1 diff --git a/lib/warbler/config.rb b/lib/warbler/config.rb index e3a9f2a1..bf59074f 100644 --- a/lib/warbler/config.rb +++ b/lib/warbler/config.rb @@ -60,8 +60,10 @@ class Config # Whether to include dependent gems (default true) attr_accessor :gem_dependencies - # Array of regular expressions matching relative paths in gems to - # be excluded from the war. Default contains no exclusions. + # Array of regular expressions matching paths *inside* packed gems to be + # excluded from the archive. Paths are matched relative to each gem's root + # (for git-sourced gems: the repository checkout root). Default contains no + # exclusions. attr_accessor :gem_excludes # Whether to exclude **/*.log files (default is true) diff --git a/lib/warbler/jar.rb b/lib/warbler/jar.rb index f0649637..f7d171c8 100644 --- a/lib/warbler/jar.rb +++ b/lib/warbler/jar.rb @@ -219,7 +219,7 @@ def find_single_gem_files(config, spec) # OK if the gem does not exists as its un-packed on the "shared" path # ... at least gem spec.spec_file should exists although not crucial if JRUBY_VERSION != JRubyJars::VERSION - warn "skipping #{spec.name} default gem (assuming its part of jruby-jars #{JRubyJars::VERSION})" unless silent? + warn "skipping #{spec.name} default gem (assuming its part of jruby-jars #{JRubyJars::VERSION})" end else warn "skipping #{spec.name} gem (#{full_gem_path.to_s} does not exist)" diff --git a/lib/warbler/templates/bundler.erb b/lib/warbler/templates/bundler.erb index 2f49dcc0..35c5a1ec 100644 --- a/lib/warbler/templates/bundler.erb +++ b/lib/warbler/templates/bundler.erb @@ -1,4 +1 @@ ENV['BUNDLE_WITHOUT'] = '<%= config.bundle_without.join(':') %>' -<% if config.bundler[:frozen] -%> -ENV['BUNDLE_FROZEN'] = '1' -<% end -%> diff --git a/lib/warbler/traits/bundler.rb b/lib/warbler/traits/bundler.rb index 22e482a5..e3f077ca 100644 --- a/lib/warbler/traits/bundler.rb +++ b/lib/warbler/traits/bundler.rb @@ -14,6 +14,23 @@ class Bundler include PathmapHelper include BundlerHelper + # Bundler settings enforced in the packed application via a generated + # *.bundle/config*, the highest-precedence documented configuration level: + BUNDLE_CONFIG_DEFAULTS = { + # BUNDLE_VERSION: disable bundler's "auto-switch" to Gemfile.lock's BUNDLED WITH bundler version + # (bundler restarts the process to do so, which cannot work inside a servlet container or self-contained executable jar) + 'BUNDLE_VERSION' => 'system', + # BUNDLE_FROZEN: fail fast with a descriptive error on Gemfile vs Gemfile.lock drift instead of attempting + # a runtime re-resolution + 'BUNDLE_FROZEN' => 'true', + # BUNDLE_PATH__SYSTEM (path.system): pin bundler to the "system" gems - which are the packed gems, via + # GEM_HOME/GEM_PATH from *init.rb* - immune to a BUNDLE_PATH/BUNDLE_DEPLOYMENT leaking in from the host's + # environment or ~/.bundle/config + 'BUNDLE_PATH__SYSTEM' => 'true', + # BUNDLE_AUTO_INSTALL: never install gems at boot time, even if the host's environment or global config enables it + 'BUNDLE_AUTO_INSTALL' => 'false' + }.freeze + def self.detect? File.exist?(ENV['BUNDLE_GEMFILE'] || 'Gemfile') end @@ -36,6 +53,8 @@ def add_bundler_gems; require 'bundler' config.gem_dependencies = false # Bundler takes care of these config.bundler = {} if config.bundler == true + warn_on_bundler_version_mismatch + bundler_specs.each do |spec| spec = to_spec(spec) @@ -45,11 +64,12 @@ def add_bundler_gems; require 'bundler' config.bundler[:git_specs] << spec when ::Bundler::Source::Path unless bundler_source_is_warbled_gem_itself?(spec.source) - if spec.source.path && spec.source.path.relative? - # include (assuming) relative *[APP_ROOT]/gem/path* - # NOTE: might be tuned to only add gemspec.files ... + if spec.source.path&.relative? + # pack the gem at its relative *[APP_ROOT]/gem/path* location - at runtime bundler resolves the path + # source relative to the packed Gemfile. + # Deliberately NOT also added to config.gems: bundler never materializes path gems from the gem + # repository; doing so would cause duplication. config.includes += FileList[File.join(spec.source.path, '**/*')] - config.gems << spec # probably not really needed else warn("Bundler `path' components are not fully supported.\n" + "The `#{spec.full_name}' component was not bundled.\n" + @@ -57,13 +77,12 @@ def add_bundler_gems; require 'bundler' end end else - config.gems << spec + config.gems << spec unless spec.respond_to?(:default_gem?) && spec.default_gem? end end config.bundler[:gemfile] = ::Bundler.default_gemfile config.bundler[:gemfile_path] = apply_pathmaps(config, relative_from_pwd(::Bundler.default_gemfile), :application) config.bundler[:lockfile] = ::Bundler.default_lockfile - config.bundler[:frozen] = ::Bundler.settings[:frozen] path = ::Bundler.settings[:path] config.excludes += [path, "#{path}/**/*"] if path config.init_contents << "#{config.warbler_templates}/bundler.erb" @@ -73,42 +92,41 @@ def update_archive(jar) add_bundler_files(jar) if config.bundler end - # Add Bundler Gemfiles and git repositories to the archive. + # Add Bundler Gemfiles, .bundle/config and git repositories to the archive. def add_bundler_files(jar) gemfile = relative_from_pwd(config.bundler[:gemfile]) lockfile = relative_from_pwd(config.bundler[:lockfile]) + bundle_config = File.join('.bundle', 'config') + jar.files[apply_pathmaps(config, gemfile, :application)] = config.bundler[:gemfile].to_s - if File.exist?(lockfile) - jar.files[apply_pathmaps(config, lockfile, :application)] = config.bundler[:lockfile].to_s - end - if config.bundler[:git_specs] - pathmap = "#{config.relative_gem_path}/bundler/gems/%p" - pathmap.sub!(%r{^/+}, '') - config.pathmaps.git = [pathmap] - config.bundler[:git_specs].each do |spec| - full_gem_path = Pathname.new(spec.full_gem_path) - - gem_relative_path = full_gem_path.relative_path_from(::Bundler.install_path) - filenames = [] - gem_relative_path.each_filename { |f| filenames << f } - - exclude_gems = true - unless filenames.empty? - full_gem_path = Pathname.new(::Bundler.install_path) + filenames.first - exclude_gems = false - end + jar.files[apply_pathmaps(config, lockfile, :application)] = config.bundler[:lockfile].to_s if File.exist?(lockfile) + # NOTE: in-memory content must be an IO - jar creation treats plain Strings as source file paths + jar.files[apply_pathmaps(config, bundle_config, :application)] = StringIO.new(bundle_config_contents) - if spec.groups.include?(:warbler_excluded) - pattern = "#{full_gem_path.to_s}/**/#{spec.name}.gemspec" # #42: gemspec only to avert Bundler error - else - pattern = "#{full_gem_path.to_s}/**/*" - end + add_bundler_git_specs(config.bundler[:git_specs], jar) if config.bundler[:git_specs] + end - FileList[pattern].each do |src| - f = Pathname.new(src).relative_path_from(full_gem_path).to_s - next if exclude_gems && config.gem_excludes && config.gem_excludes.any? {|rx| f =~ rx } - jar.files[apply_pathmaps(config, File.join(full_gem_path.basename, f), :git)] = src - end + private + + def add_bundler_git_specs(git_specs, jar) + config.pathmaps.git = ["#{config.relative_gem_path}/bundler/gems/%p".sub(%r{^/+}, '')] + + # a git source checkout may contain multiple gems (spec.full_gem_path being a sub-directory) - bundler expects + # the complete repository checkout under bundler/gems/-, so pack from its root (once per repository, + # even when several specs share the checkout) + checkout_paths = git_specs.map do |spec| + full_gem_path = Pathname.new(spec.full_gem_path) + filenames = full_gem_path.relative_path_from(::Bundler.install_path).each_filename.to_a + filenames.empty? ? full_gem_path : Pathname.new(::Bundler.install_path) + filenames.first + end.uniq + + checkout_paths.each do |checkout_path| + FileList["#{checkout_path.to_s}/**/*"].each do |src| + f = Pathname.new(src).relative_path_from(checkout_path).to_s + # NOTE: for git sources the excludes match relative to the packed repository checkout root (the gem root, + # except multi-gem repos) + next if config.gem_excludes && config.gem_excludes.any? { |rx| f =~ rx } + jar.files[apply_pathmaps(config, File.join(checkout_path.basename, f), :git)] = src end end end @@ -121,17 +139,32 @@ def relative_from_pwd(path) end end - private + def warn_on_bundler_version_mismatch + lockfile = ::Bundler.default_lockfile + return unless lockfile && File.exist?(lockfile) + locked = ::Bundler::LockfileParser.new(File.read(lockfile)).bundler_version rescue nil + if locked && locked.to_s != ::Bundler::VERSION + warn("Gemfile.lock BUNDLED WITH (#{locked}) does not match the bundler running warbler (#{::Bundler::VERSION}).\n" + + "The packed application will boot with the default bundler of the packed JRuby (jruby-jars),\n" + + "consider re-generating Gemfile.lock with a matching bundler version.") + end + end + + # Contents for the packed *.bundle/config*: only warbler's deployment settings, deliberately independent of the + # application's build-time bundler configuration. + def bundle_config_contents + require 'yaml' + settings = BUNDLE_CONFIG_DEFAULTS.dup + # frozen mode errors without a lockfile - do not force it upon applications packed without a Gemfile.lock + settings.delete('BUNDLE_FROZEN') if config.bundler[:frozen] == false || !File.exist?(config.bundler[:lockfile].to_s) + settings.to_yaml + end def bundler_specs bundle_without = config.bundle_without.map { |s| s.to_sym } definition = ::Bundler.definition - all = definition.specs.to_a requested_groups = definition.groups - bundle_without - requested = requested_groups.empty? ? [] : definition.specs_for(requested_groups).to_a - excluded_git_specs = (all - requested).select { |spec| ::Bundler::Source::Git === spec.source } - excluded_git_specs.each { |spec| spec.groups << :warbler_excluded } - requested + excluded_git_specs + requested_groups.empty? ? [] : definition.specs_for(requested_groups).to_a end def bundler_source_is_warbled_gem_itself?(source) diff --git a/spec/warbler/bundler_spec.rb b/spec/warbler/bundler_spec.rb index 58ef5a0c..f978d927 100644 --- a/spec/warbler/bundler_spec.rb +++ b/spec/warbler/bundler_spec.rb @@ -8,6 +8,7 @@ require File.expand_path('../../spec_helper', __FILE__) require 'open3' require 'bundler' +require 'yaml' describe Warbler::Jar, "with Bundler" do use_fresh_rake_application @@ -66,6 +67,67 @@ def apply_silently expect(file_list(%r{WEB-INF/Gemfile.lock})).to_not be_empty end + it "generates an opinionated .bundle/config into the war" do + File.open("Gemfile.lock", "w") {|f| f << "GEM"} + apply_silently + expect(file_list(%r{WEB-INF/\.bundle/config})).to_not be_empty + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings['BUNDLE_VERSION']).to eq 'system' + expect(settings['BUNDLE_FROZEN']).to eq 'true' + expect(settings['BUNDLE_PATH__SYSTEM']).to eq 'true' + expect(settings['BUNDLE_AUTO_INSTALL']).to eq 'false' + end + + it "does not force BUNDLE_FROZEN without a Gemfile.lock" do + apply_silently + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings).to_not have_key 'BUNDLE_FROZEN' + expect(settings['BUNDLE_VERSION']).to eq 'system' + end + + it "does not force BUNDLE_FROZEN when opted out via config.bundler[:frozen]" do + File.open("Gemfile.lock", "w") {|f| f << "GEM"} + use_config do |config| + config.bundler = { :frozen => false } + end + apply_silently + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings).to_not have_key 'BUNDLE_FROZEN' + expect(settings['BUNDLE_VERSION']).to eq 'system' # other deployment settings unaffected + end + + context "with an application-provided .bundle/config" do + before :each do + FileUtils.mkdir_p('.bundle') + File.write File.join('.bundle', 'config'), <<-CONFIG.gsub(/^ {10}/, '') + --- + BUNDLE_RETRY: "5" + BUNDLE_VERSION: "lockfile" + BUNDLE_GITHUB__COM: "user:secret" + CONFIG + end + + after(:each) { FileUtils.rm_rf('.bundle') } + + it "does not carry any of it into the archive (only warbler's deployment settings)" do + File.open("Gemfile.lock", "w") {|f| f << "GEM"} + apply_silently + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings).to_not have_key 'BUNDLE_RETRY' + expect(settings).to_not have_key 'BUNDLE_GITHUB__COM' # credentials never packed + expect(settings['BUNDLE_VERSION']).to eq 'system' + expect(settings.keys).to match_array %w(BUNDLE_VERSION BUNDLE_FROZEN BUNDLE_PATH__SYSTEM BUNDLE_AUTO_INSTALL) + end + end + + it "does not package default gems (provided by the JRuby runtime)" do + File.open("Gemfile", "w") {|f| f << "source 'https://rubygems.org'\ngem 'rspec'\ngem 'stringio'"} + apply_silently + expect(file_list(%r{WEB-INF/gems/specifications/rspec})).to_not be_empty + expect(file_list(%r{WEB-INF/gems/specifications/stringio})).to be_empty + expect(file_list(%r{WEB-INF/gems/gems/stringio})).to be_empty + end + it "allows overriding of the gem path when using Bundler" do use_config do |config| config.gem_path = '/WEB-INF/jewels' @@ -85,14 +147,25 @@ def apply_silently expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/tester.gemspec})).to_not be_empty end - it "bundles only the gemspec for :git entries that are excluded" do - File.open("Gemfile", "w") {|f| f << "source 'https://rubygems.org'\ngem 'rake'\ngroup :test do\ngem 'tester', :git => '#{@gem_dir}'\nend\n"} + it "respects config.gem_excludes for :git entries" do + File.open("Gemfile", "w") {|f| f << "source 'file://#{@gem_dir}'\ngem 'tester', :git => '#{@gem_dir}'\n"} bundle_install '--local' + use_config do |config| + config.gem_excludes += [%r{^lib/tester/version}] + end apply_silently expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/lib/tester/version\.rb})).to be_empty expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/tester.gemspec})).to_not be_empty end + it "does not bundle :git entries that are excluded" do + File.open("Gemfile", "w") {|f| f << "source 'https://rubygems.org'\ngem 'rake'\ngroup :test do\ngem 'tester', :git => '#{@gem_dir}'\nend\n"} + bundle_install '--local' + apply_silently + # bundler (>= 2.6) does not need excluded git sources present at all + expect(file_list(%r{WEB-INF/gems/bundler/gems/tester})).to be_empty + end + end context 'with :path entries in the Gemfile' do @@ -107,7 +180,7 @@ def apply_silently expect(file_list(%r{tester})).to be_empty end - it "does work with relative :path" do + it "does work with relative :path, packed once at its relative location" do gem_dir = File.join(Dir.pwd, 'gems/tester') #begin Dir.mkdir(gem_dir) @@ -115,9 +188,13 @@ def apply_silently File.open("Gemfile", "w") {|f| f << "source 'https://rubygems.org'\ngem 'rake'\ngem 'tester', :path => 'gems/tester'\n"} bundle_install '--local' apply_silently - expect(file_list(%r{tester})).to_not be_empty # included from :path as is - expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/lib/tester/version\.rb})).to be_empty - expect(file_list(%r{WEB-INF/gems/bundler/gems/tester[^/]*/tester.gemspec})).to be_empty + # packed at the Gemfile-relative location bundler resolves at runtime + expect(file_list(%r{WEB-INF/gems/tester/tester\.gemspec})).to_not be_empty + expect(file_list(%r{WEB-INF/gems/tester/lib/tester/version\.rb})).to_not be_empty + # and nowhere else (#465): not in the gem repository nor as a git checkout + expect(file_list(%r{WEB-INF/gems/gems/tester})).to be_empty + expect(file_list(%r{WEB-INF/gems/specifications/tester})).to be_empty + expect(file_list(%r{WEB-INF/gems/bundler/gems/tester})).to be_empty #ensure #FileUtils.rm_r(gem_dir) rescue nil #end @@ -184,11 +261,12 @@ def apply_silently context "when frozen" do run_in_directory "spec/sample_bundler" - it "includes the bundler gem" do + it "does not vendor the (default gem) bundler - the packed JRuby provides it" do bundle_install apply_silently - expect(config.gems.detect{|k,v| k.name == 'bundler'}).to_not be nil - expect(file_list(/bundler-/)).to_not be_empty + expect(config.gems.detect{|k,v| k.name == 'bundler'}).to be nil + expect(file_list(%r{gems/bundler-})).to be_empty + expect(file_list(%r{specifications/bundler-})).to be_empty end it "does not include the bundler cache directory" do @@ -196,10 +274,13 @@ def apply_silently expect(file_list(%r{vendor/bundle})).to be_empty end - it "includes ENV['BUNDLE_FROZEN'] in init.rb" do + it "carries the frozen setting into the packed .bundle/config" do apply_silently + settings = YAML.load(jar.files['WEB-INF/.bundle/config'].string) + expect(settings['BUNDLE_FROZEN']).to eq 'true' + # no longer exported via init.rb - the packed .bundle/config is authoritative contents = jar.contents('META-INF/init.rb') - expect(contents.split("\n").grep(/ENV\['BUNDLE_FROZEN'\] = '1'/)).to_not be_empty + expect(contents.split("\n").grep(/BUNDLE_FROZEN/)).to be_empty end context "with the runnable feature" do @@ -249,12 +330,11 @@ def apply_silently bundle 'config', 'set', 'deployment', 'true' end - it "includes the bundler gem" do + it "does not vendor the (default gem) bundler - the packed JRuby provides it" do bundle_install apply_silently expect(file_list(%r{gems/rake-13.4.2/lib})).to_not be_empty - expect(file_list(%r{gems/bundler-})).to_not be_empty - expect(file_list(%r{gems/bundler-.*/exe})).to_not be_empty + expect(file_list(%r{gems/bundler-})).to be_empty end after do @@ -273,3 +353,45 @@ def apply_silently end end end + +describe Warbler::Traits::Bundler, "#warn_on_bundler_version_mismatch" do + require 'tmpdir' + + let(:trait) { Warbler::Traits::Bundler.allocate } + + def lockfile_with_bundled_with(version) + dir = Dir.mktmpdir 'bundled-with' + path = File.join(dir, 'Gemfile.lock') + File.write path, <<-LOCKFILE.gsub(/^ {6}/, '') + GEM + specs: + + PLATFORMS + ruby + + DEPENDENCIES + + BUNDLED WITH + #{version} + LOCKFILE + Pathname.new(path) + end + + it "warns when the lockfile BUNDLED WITH differs from the running bundler" do + allow(::Bundler).to receive(:default_lockfile).and_return lockfile_with_bundled_with('9.9.9') + expect(trait).to receive(:warn).with(/BUNDLED WITH \(9\.9\.9\) does not match the bundler running warbler \(#{Regexp.escape ::Bundler::VERSION}\)/) + trait.send :warn_on_bundler_version_mismatch + end + + it "does not warn when the lockfile BUNDLED WITH matches the running bundler" do + allow(::Bundler).to receive(:default_lockfile).and_return lockfile_with_bundled_with(::Bundler::VERSION) + expect(trait).to_not receive(:warn) + trait.send :warn_on_bundler_version_mismatch + end + + it "does not warn without a lockfile" do + allow(::Bundler).to receive(:default_lockfile).and_return Pathname.new('/no/such/Gemfile.lock') + expect(trait).to_not receive(:warn) + trait.send :warn_on_bundler_version_mismatch + end +end