Fix Bundler --local option

Resolves: rhbz#2311898
This commit is contained in:
Vít Ondruch 2024-09-13 16:26:26 +02:00
commit 8daf7ccb72
2 changed files with 382 additions and 1 deletions

View file

@ -0,0 +1,372 @@
From cd2ad8c881b68b0c76da865c316c07dba28d2285 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Thu, 12 Sep 2024 16:41:34 +0200
Subject: [PATCH 1/7] Simplify handling default gem caching
By the time `cached_gem` is called, default gem cache has already been
handled. So no need to try redownload it again, it's enough to check the
cache location directly.
---
bundler/lib/bundler/source/rubygems.rb | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/bundler/lib/bundler/source/rubygems.rb b/bundler/lib/bundler/source/rubygems.rb
index 1085fdc2d89f..31815ea63a37 100644
--- a/bundler/lib/bundler/source/rubygems.rb
+++ b/bundler/lib/bundler/source/rubygems.rb
@@ -223,11 +223,12 @@ def cache(spec, custom_path = nil)
end
def cached_built_in_gem(spec)
- cached_path = cached_path(spec)
+ cached_path = cached_gem(spec)
if cached_path.nil?
remote_spec = remote_specs.search(spec).first
if remote_spec
cached_path = fetch_gem(remote_spec)
+ spec.remote = remote_spec.remote
else
Bundler.ui.warn "#{spec.full_name} is built in to Ruby, and can't be cached because your Gemfile doesn't have any sources that contain it."
end
@@ -324,14 +325,6 @@ def remotes_for_spec(spec)
end
def cached_gem(spec)
- if spec.default_gem?
- cached_built_in_gem(spec)
- else
- cached_path(spec)
- end
- end
-
- def cached_path(spec)
global_cache_path = download_cache_path(spec)
caches << global_cache_path if global_cache_path
From 67f3dbdc2b8a8d67db4e4d7ccd01e92f9f30b317 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Thu, 12 Sep 2024 17:09:20 +0200
Subject: [PATCH 2/7] Complete spec
---
bundler/spec/bundler/cache/gems_spec.rb | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bundler/spec/bundler/cache/gems_spec.rb b/bundler/spec/cache/gems_spec.rb
index 8f81d2d45e9e..1f3b22b4f823 100644
--- a/bundler/spec/bundler/cache/gems_spec.rb
+++ b/bundler/spec/bundler/cache/gems_spec.rb
@@ -149,9 +149,10 @@
end
context "when a remote gem is not available for caching" do
- it "uses builtin gems when installing to system gems" do
+ it "warsn, but uses builtin gems when installing to system gems" do
bundle "config set path.system true"
install_gemfile %(source "https://gem.repo1"; gem 'json', '#{default_json_version}'), verbose: true
+ expect(err).to include("json-#{default_json_version} is built in to Ruby, and can't be cached")
expect(out).to include("Using json #{default_json_version}")
end
From c683f189a5de65e3aee787020e0692e7fa23e1ef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Thu, 12 Sep 2024 18:50:09 +0200
Subject: [PATCH 3/7] Consistently access install options through symbol keys
---
bundler/lib/bundler/installer.rb | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/bundler/lib/bundler/installer.rb b/bundler/lib/bundler/installer.rb
index 485782d1b410..8868fc772f22 100644
--- a/bundler/lib/bundler/installer.rb
+++ b/bundler/lib/bundler/installer.rb
@@ -194,7 +194,7 @@ def generate_standalone_bundler_executable_stubs(spec, options = {})
# that said, it's a rare situation (other than rake), and parallel
# installation is SO MUCH FASTER. so we let people opt in.
def install(options)
- force = options["force"]
+ force = options[:force]
jobs = installation_parallelization(options)
install_in_parallel jobs, options[:standalone], force
end
@@ -247,9 +247,9 @@ def install_in_parallel(size, standalone, force = false)
# returns whether or not a re-resolve was needed
def resolve_if_needed(options)
- @definition.prefer_local! if options["prefer-local"]
+ @definition.prefer_local! if options[:"prefer-local"]
- if options["local"] || (@definition.no_resolve_needed? && !@definition.missing_specs?)
+ if options[:local] || (@definition.no_resolve_needed? && !@definition.missing_specs?)
@definition.resolve_with_cache!
false
else
From 088ea59a084e25eefdf3bc05a2fcc41593ee0271 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Thu, 12 Sep 2024 18:58:01 +0200
Subject: [PATCH 4/7] Inline a private method
Removes an (in my opinion) excessive indirection and handles options
more consistently.
---
bundler/lib/bundler/installer.rb | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/bundler/lib/bundler/installer.rb b/bundler/lib/bundler/installer.rb
index 8868fc772f22..6c07d2328b00 100644
--- a/bundler/lib/bundler/installer.rb
+++ b/bundler/lib/bundler/installer.rb
@@ -194,9 +194,13 @@ def generate_standalone_bundler_executable_stubs(spec, options = {})
# that said, it's a rare situation (other than rake), and parallel
# installation is SO MUCH FASTER. so we let people opt in.
def install(options)
+ standalone = options[:standalone]
force = options[:force]
jobs = installation_parallelization(options)
- install_in_parallel jobs, options[:standalone], force
+ spec_installations = ParallelInstaller.call(self, @definition.specs, jobs, standalone, force)
+ spec_installations.each do |installation|
+ post_install_messages[installation.name] = installation.post_install_message if installation.has_post_install_message?
+ end
end
def installation_parallelization(options)
@@ -238,13 +242,6 @@ def ensure_specs_are_compatible!
end
end
- def install_in_parallel(size, standalone, force = false)
- spec_installations = ParallelInstaller.call(self, @definition.specs, size, standalone, force)
- spec_installations.each do |installation|
- post_install_messages[installation.name] = installation.post_install_message if installation.has_post_install_message?
- end
- end
-
# returns whether or not a re-resolve was needed
def resolve_if_needed(options)
@definition.prefer_local! if options[:"prefer-local"]
From 0c047262cf14806016f60969796840388dd678dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Thu, 12 Sep 2024 19:07:31 +0200
Subject: [PATCH 5/7] Fix spec to setup a default gem correctly
Looking at the description is something that used to be done a long time
ago.
---
bundler/spec/bundler/cache/gems_spec.rb | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/bundler/spec/bundler/cache/gems_spec.rb b/bundler/spec/cache/gems_spec.rb
index 1f3b22b4f823..23e4cc7083fa 100644
--- a/bundler/spec/bundler/cache/gems_spec.rb
+++ b/bundler/spec/bundler/cache/gems_spec.rb
@@ -134,9 +134,7 @@
end
it "doesn't make remote request after caching the gem" do
- build_gem "builtin_gem_2", "1.0.2", path: bundled_app("vendor/cache") do |s|
- s.summary = "This builtin_gem is bundled with Ruby"
- end
+ build_gem "builtin_gem_2", "1.0.2", path: bundled_app("vendor/cache"), default: true
install_gemfile <<-G
source "https://gem.repo2"
From 32d46da6e77cd5f49b83ddf538bc26471c363dbe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Thu, 12 Sep 2024 19:14:15 +0200
Subject: [PATCH 6/7] Improve spec
The behavior it's testing is independent from the bundle path being
used.
---
bundler/spec/bundler/cache/gems_spec.rb | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/bundler/spec/bundler/cache/gems_spec.rb b/bundler/spec/cache/gems_spec.rb
index 23e4cc7083fa..44de86fb66e3 100644
--- a/bundler/spec/bundler/cache/gems_spec.rb
+++ b/bundler/spec/bundler/cache/gems_spec.rb
@@ -103,8 +103,7 @@
end
end
- it "uses remote gems when installing to system gems" do
- bundle "config set path.system true"
+ it "uses remote gems when installing" do
install_gemfile %(source "https://gem.repo2"; gem 'json', '#{default_json_version}'), verbose: true
expect(out).to include("Installing json #{default_json_version}")
end
From 4d4c49d8368997a274b8c8069ef21072792d0350 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Thu, 12 Sep 2024 19:24:46 +0200
Subject: [PATCH 7/7] Fix `--local` hitting the network when default gems are
included
---
bundler/lib/bundler/installer.rb | 3 ++-
bundler/lib/bundler/installer/gem_installer.rb | 6 ++++--
bundler/lib/bundler/installer/parallel_installer.rb | 5 +++--
bundler/lib/bundler/source/rubygems.rb | 6 +++---
bundler/spec/bundler/bundler/installer/gem_installer_spec.rb | 7 ++++---
bundler/spec/bundler/cache/gems_spec.rb | 5 +++++
6 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/bundler/lib/bundler/installer.rb b/bundler/lib/bundler/installer.rb
index 6c07d2328b00..b65546a10a91 100644
--- a/bundler/lib/bundler/installer.rb
+++ b/bundler/lib/bundler/installer.rb
@@ -196,8 +196,9 @@ def generate_standalone_bundler_executable_stubs(spec, options = {})
def install(options)
standalone = options[:standalone]
force = options[:force]
+ local = options[:local]
jobs = installation_parallelization(options)
- spec_installations = ParallelInstaller.call(self, @definition.specs, jobs, standalone, force)
+ spec_installations = ParallelInstaller.call(self, @definition.specs, jobs, standalone, force, local: local)
spec_installations.each do |installation|
post_install_messages[installation.name] = installation.post_install_message if installation.has_post_install_message?
end
diff --git a/bundler/lib/bundler/installer/gem_installer.rb b/bundler/lib/bundler/installer/gem_installer.rb
index a7770eb7e0b6..1da91857bd57 100644
--- a/bundler/lib/bundler/installer/gem_installer.rb
+++ b/bundler/lib/bundler/installer/gem_installer.rb
@@ -2,14 +2,15 @@
module Bundler
class GemInstaller
- attr_reader :spec, :standalone, :worker, :force, :installer
+ attr_reader :spec, :standalone, :worker, :force, :local, :installer
- def initialize(spec, installer, standalone = false, worker = 0, force = false)
+ def initialize(spec, installer, standalone = false, worker = 0, force = false, local = false)
@spec = spec
@installer = installer
@standalone = standalone
@worker = worker
@force = force
+ @local = local
end
def install_from_spec
@@ -54,6 +55,7 @@ def install
spec.source.install(
spec,
force: force,
+ local: local,
build_args: Array(spec_settings),
previous_spec: previous_spec,
)
diff --git a/bundler/lib/bundler/installer/parallel_installer.rb b/bundler/lib/bundler/installer/parallel_installer.rb
index e745088f81ad..d10e5ec92403 100644
--- a/bundler/lib/bundler/installer/parallel_installer.rb
+++ b/bundler/lib/bundler/installer/parallel_installer.rb
@@ -68,11 +68,12 @@ def self.call(*args, **kwargs)
attr_reader :size
- def initialize(installer, all_specs, size, standalone, force, skip: nil)
+ def initialize(installer, all_specs, size, standalone, force, local: false, skip: nil)
@installer = installer
@size = size
@standalone = standalone
@force = force
+ @local = local
@specs = all_specs.map {|s| SpecInstallation.new(s) }
@specs.each do |spec_install|
spec_install.state = :installed if skip.include?(spec_install.name)
@@ -127,7 +128,7 @@ def worker_pool
def do_install(spec_install, worker_num)
Plugin.hook(Plugin::Events::GEM_BEFORE_INSTALL, spec_install)
gem_installer = Bundler::GemInstaller.new(
- spec_install.spec, @installer, @standalone, worker_num, @force
+ spec_install.spec, @installer, @standalone, worker_num, @force, @local
)
success, message = gem_installer.install_from_spec
if success
diff --git a/bundler/lib/bundler/source/rubygems.rb b/bundler/lib/bundler/source/rubygems.rb
index 31815ea63a37..3b6ef8bd580d 100644
--- a/bundler/lib/bundler/source/rubygems.rb
+++ b/bundler/lib/bundler/source/rubygems.rb
@@ -148,7 +148,7 @@ def specs
end
def install(spec, options = {})
- if (spec.default_gem? && !cached_built_in_gem(spec)) || (installed?(spec) && !options[:force])
+ if (spec.default_gem? && !cached_built_in_gem(spec, local: options[:local])) || (installed?(spec) && !options[:force])
print_using_message "Using #{version_message(spec, options[:previous_spec])}"
return nil # no post-install message
end
@@ -222,9 +222,9 @@ def cache(spec, custom_path = nil)
raise InstallError, e.message
end
- def cached_built_in_gem(spec)
+ def cached_built_in_gem(spec, local: false)
cached_path = cached_gem(spec)
- if cached_path.nil?
+ if cached_path.nil? && !local
remote_spec = remote_specs.search(spec).first
if remote_spec
cached_path = fetch_gem(remote_spec)
diff --git a/bundler/spec/bundler/bundler/installer/gem_installer_spec.rb b/bundler/spec/bundler/installer/gem_installer_spec.rb
index ea506c36c8bf..6583bd8181b1 100644
--- a/bundler/spec/bundler/bundler/installer/gem_installer_spec.rb
+++ b/bundler/spec/bundler/bundler/installer/gem_installer_spec.rb
@@ -7,6 +7,7 @@
let(:installer) { instance_double("Installer", definition: definition) }
let(:spec_source) { instance_double("SpecSource") }
let(:spec) { instance_double("Specification", name: "dummy", version: "0.0.1", loaded_from: "dummy", source: spec_source) }
+ let(:base_options) { { force: false, local: false, previous_spec: nil } }
subject { described_class.new(spec, installer) }
@@ -14,7 +15,7 @@
it "invokes install method with empty build_args" do
allow(spec_source).to receive(:install).with(
spec,
- { force: false, build_args: [], previous_spec: nil }
+ base_options.merge(build_args: [])
)
subject.install_from_spec
end
@@ -28,7 +29,7 @@
allow(Bundler.settings).to receive(:[]).with("build.dummy").and_return("--with-dummy-config=dummy")
expect(spec_source).to receive(:install).with(
spec,
- { force: false, build_args: ["--with-dummy-config=dummy"], previous_spec: nil }
+ base_options.merge(build_args: ["--with-dummy-config=dummy"])
)
subject.install_from_spec
end
@@ -42,7 +43,7 @@
allow(Bundler.settings).to receive(:[]).with("build.dummy").and_return("--with-dummy-config=dummy --with-another-dummy-config")
expect(spec_source).to receive(:install).with(
spec,
- { force: false, build_args: ["--with-dummy-config=dummy", "--with-another-dummy-config"], previous_spec: nil }
+ base_options.merge(build_args: ["--with-dummy-config=dummy", "--with-another-dummy-config"])
)
subject.install_from_spec
end
diff --git a/bundler/spec/bundler/cache/gems_spec.rb b/bundler/spec/cache/gems_spec.rb
index 44de86fb66e3..0177a4ef0628 100644
--- a/bundler/spec/bundler/cache/gems_spec.rb
+++ b/bundler/spec/bundler/cache/gems_spec.rb
@@ -108,6 +108,11 @@
expect(out).to include("Installing json #{default_json_version}")
end
+ it "does not use remote gems when installing with --local flag" do
+ install_gemfile %(source "https://gem.repo2"; gem 'json', '#{default_json_version}'), verbose: true, local: true
+ expect(out).to include("Using json #{default_json_version}")
+ end
+
it "caches remote and builtin gems" do
install_gemfile <<-G
source "https://gem.repo2"

View file

@ -171,7 +171,7 @@
Summary: An interpreter of object-oriented scripting language
Name: ruby
Version: %{ruby_version}%{?development_release}
Release: 14%{?dist}
Release: 15%{?dist}
# Licenses, which are likely not included in binary RPMs:
# Apache-2.0:
# benchmark/gc/redblack.rb
@ -289,6 +289,10 @@ Patch12: ruby-3.4.0-Extract-hardening-CFLAGS-to-a-special-hardenflags-variable.p
Patch13: rubygems-3.5.17-Avoid-another-race-condition-of-open-mode.patch
# https://github.com/rubygems/rubygems/pull/7939
Patch14: rubygems-3.5.17-Remove-the-lock-file-for-binstubs.patch
# Fix Bundler reaching to the interned with `--local` option.
# https://github.com/rubygems/rubygems/issues/8025
# https://github.com/rubygems/rubygems/pull/8027
Patch15: bundler-2.5.19-Fix--local-hitting-the-network-when-default-gems-are.patch
Requires: %{name}-libs%{?_isa} = %{version}-%{release}
%{?with_rubypick:Suggests: rubypick}
@ -768,6 +772,7 @@ analysis result in RBS format, a standard type description format for Ruby
%patch 12 -p1
%patch 13 -p1
%patch 14 -p1
%patch 15 -p2
# Provide an example of usage of the tapset:
cp -a %{SOURCE3} .
@ -1744,6 +1749,10 @@ make -C %{_vpath_builddir} runruby TESTRUN_SCRIPT=" \
%changelog
* Fri Sep 13 2024 Vít Ondruch <vondruch@redhat.com> - 3.3.5-15
- Fix Bundler `--local` option
Resolves: rhbz#2311898
* Tue Sep 03 2024 Vít Ondruch <vondruch@redhat.com> - 3.3.5-14
- Upgrade to Ruby 3.3.5.
Resolves: rhbz#2309364