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.

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

lock_platform directive in Gemfile #7172

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions bundler/lib/bundler/definition.rb
Expand Up @@ -55,7 +55,7 @@ def self.build(gemfile, lockfile, unlock)
# to be updated or true if all gems should be updated
# @param ruby_version [Bundler::RubyVersion, nil] Requested Ruby Version
# @param optional_groups [Array(String)] A list of optional groups
def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, optional_groups = [], gemfiles = [])
def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, optional_groups = [], gemfiles = [], platforms = [])
if [true, false].include?(unlock)
@unlocking_bundler = false
@unlocking = unlock
Expand Down Expand Up @@ -89,6 +89,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
@locked_gems = LockfileParser.new(@lockfile_contents)
@locked_platforms = @locked_gems.platforms
@platforms = @locked_platforms.dup
platforms.each {|p| add_platform(p) }
Copy link
Member

Choose a reason for hiding this comment

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

does it work to call platforms.each {|p| add_platform(p) } unconditionally, at line 118?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably. This was a premature optimization when we know that @locked_platforms is empty when there's no lockfile.

@locked_bundler_version = @locked_gems.bundler_version
@locked_ruby_version = @locked_gems.ruby_version
@originally_locked_specs = SpecSet.new(@locked_gems.specs)
Expand All @@ -105,13 +106,14 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
end
else
@unlock = {}
@platforms = []
@locked_gems = nil
@locked_deps = {}
@locked_specs = SpecSet.new([])
@originally_locked_specs = @locked_specs
@locked_sources = []
@locked_platforms = []
@platforms = platforms
@new_platform = true unless @platforms.empty?
end

locked_gem_sources = @locked_sources.select {|s| s.is_a?(Source::Rubygems) }
Expand Down Expand Up @@ -143,7 +145,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
@unlock[:gems] ||= @dependencies.map(&:name)
else
eager_unlock = (@unlock[:gems] || []).map {|name| Dependency.new(name, ">= 0") }
@unlock[:gems] = @locked_specs.for(eager_unlock, false, platforms).map(&:name).uniq
@unlock[:gems] = @locked_specs.for(eager_unlock, false, @platforms).map(&:name).uniq
end

@dependency_changes = converge_dependencies
Expand Down Expand Up @@ -975,6 +977,7 @@ def remove_invalid_platforms!(dependencies)
platforms.reverse_each do |platform|
next if local_platform == platform ||
(@new_platform && platforms.last == platform) ||
@locked_platforms.include?(platform) ||
@path_changes ||
@dependency_changes ||
!@originally_locked_specs.incomplete_for_platform?(dependencies, platform)
Expand Down
14 changes: 13 additions & 1 deletion bundler/lib/bundler/dsl.rb
Expand Up @@ -32,6 +32,7 @@ def initialize
@install_conditionals = []
@optional_groups = []
@platforms = []
@lock_platforms = []
@env = nil
@ruby_version = nil
@gemspecs = []
Expand Down Expand Up @@ -220,7 +221,7 @@ def github(repo, options = {})

def to_definition(lockfile, unlock)
check_primary_source_safety
Definition.new(lockfile, @dependencies, @sources, unlock, @ruby_version, @optional_groups, @gemfiles)
Definition.new(lockfile, @dependencies, @sources, unlock, @ruby_version, @optional_groups, @gemfiles, @lock_platforms)
end

def group(*args, &blk)
Expand Down Expand Up @@ -254,6 +255,17 @@ def platforms(*platforms)
end
alias_method :platform, :platforms

# Name collision, working around it for a proof
def lock_platform(*platforms)
platforms.each do |p|
gem_platform = Gem::Platform.new(p)
if gem_platform.to_s == "unknown"
raise GemfileError, "Unknown lock_platform #{platform_string.inspect} in Gemfile"
end
@lock_platforms << gem_platform
end
end

def env(name)
old = @env
@env = name
Expand Down
38 changes: 38 additions & 0 deletions bundler/spec/install/gemfile/specific_platform_spec.rb
Expand Up @@ -459,6 +459,44 @@
expect(err).to include(error_message).once
end

it "does not resolve if a locked platform does not match any of available platform specific variants for a top level dependency" do
build_repo4 do
build_gem("sorbet-static", "0.5.6433") {|s| s.platform = "x86_64-linux" }
build_gem("sorbet-static", "0.5.6433") {|s| s.platform = "universal-darwin-20" }
end

gemfile <<~G
source "#{file_uri_for(gem_repo4)}"

lock_platform "arm64-darwin-21"

gem "sorbet-static", "0.5.6433"
G

error_message = <<~ERROR.strip
Could not find gems matching 'sorbet-static (= 0.5.6433)' valid for all resolution platforms (arm64-darwin-21, arm64-darwin-20) in rubygems repository #{file_uri_for(gem_repo4)}/ or installed locally.

The source contains the following gems matching 'sorbet-static (= 0.5.6433)':
* sorbet-static-0.5.6433-universal-darwin-20
* sorbet-static-0.5.6433-x86_64-linux
ERROR

# simulate a platform that is available and is not the locked platform
simulate_platform "arm64-darwin-20" do
bundle "lock", :raise_on_error => false
end

expect(err).to include(error_message).once

# Make sure it doesn't print error twice in verbose mode

simulate_platform "arm64-darwin-20" do
bundle "lock --verbose", :raise_on_error => false
end

expect(err).to include(error_message).once
end

it "does not generate a lockfile if RUBY platform is forced and some gem has no RUBY variant available" do
build_repo4 do
build_gem("sorbet-static", "0.5.9889") {|s| s.platform = Gem::Platform.local }
Expand Down
146 changes: 146 additions & 0 deletions bundler/spec/runtime/platform_spec.rb
Expand Up @@ -467,4 +467,150 @@
end
end
end

it "will add platform directly specified by a gemfile" do
simulate_platform "x86-darwin-100"

build_repo4 do
build_gem "nokogiri", "1.13.8"
build_gem "nokogiri", "1.13.8" do |s|
s.platform = "aarch64-linux"
end
end

lockfile <<-L
GEM
remote: #{file_uri_for(gem_repo4)}/
specs:
nokogiri (1.13.8)

PLATFORMS
ruby

DEPENDENCIES
nokogiri

CHECKSUMS

BUNDLED WITH
#{Bundler::VERSION}
L


install_gemfile <<-G
source "#{file_uri_for(gem_repo4)}"

lock_platform "aarch64-linux"

gem "nokogiri"
G

checksums = checksum_section do |c|
c.repo_gem gem_repo4, "nokogiri", "1.13.8"
# TODO: This is wrong, the point is to get the checksum
# Figure out why it's not fetching them for other platforms
c.no_checksum "nokogiri", "1.13.8", "aarch64-linux"
# c.repo_gem gem_repo4, "nokogiri", "1.13.8", "aarch64-linux"
end

expected_lockfile = <<~L
GEM
remote: #{file_uri_for(gem_repo4)}/
specs:
nokogiri (1.13.8)
nokogiri (1.13.8-aarch64-linux)

PLATFORMS
aarch64-linux
ruby

DEPENDENCIES
nokogiri

CHECKSUMS
#{checksums}

BUNDLED WITH
#{Bundler::VERSION}
L

# TODO: "nokogiri was expected to be of platform ruby but was ruby"
# expect(the_bundle).to include_gems "nokogiri 1.13.8 ruby"
expect(lockfile).to eq(expected_lockfile)
end

it "adds platforms specified by a gemfile" do
simulate_platform "x86-darwin-100"

build_repo4 do
build_gem "nokogiri", "1.13.8"
build_gem "nokogiri", "1.13.8" do |s|
s.platform = "aarch64-linux"
end
build_gem "nokogiri", "1.13.8" do |s|
s.platform = "x86_64-linux"
end
end

lockfile <<-L
GEM
remote: #{file_uri_for(gem_repo4)}/
specs:
nokogiri (1.13.8)

PLATFORMS
ruby

DEPENDENCIES
nokogiri

CHECKSUMS

BUNDLED WITH
#{Bundler::VERSION}
L

install_gemfile <<-G
source "#{file_uri_for(gem_repo4)}"

lock_platform "aarch64-linux", "x86_64-linux"

gem "nokogiri"
G

checksums = checksum_section do |c|
c.repo_gem gem_repo4, "nokogiri", "1.13.8"
# TODO: This is wrong, the point is to get the checksum
# Figure out why it's not fetching them for other platforms
c.no_checksum "nokogiri", "1.13.8", "aarch64-linux"
c.no_checksum "nokogiri", "1.13.8", "x86_64-linux"
end

expected_lockfile = <<~L
GEM
remote: #{file_uri_for(gem_repo4)}/
specs:
nokogiri (1.13.8)
nokogiri (1.13.8-aarch64-linux)
nokogiri (1.13.8-x86_64-linux)

PLATFORMS
aarch64-linux
ruby
x86_64-linux

DEPENDENCIES
nokogiri

CHECKSUMS
#{checksums}

BUNDLED WITH
#{Bundler::VERSION}
L

# TODO: "nokogiri was expected to be of platform ruby but was ruby"
# expect(the_bundle).to include_gems "nokogiri 1.13.8 ruby"
expect(lockfile).to eq(expected_lockfile)
end
end