Allow explicitly setting platforms in Gemfile

[segiddins]

Closes #4813.
See #4853.
Builds upon #4836.

[indirect]

Can you explain what's going on here? This PR doesn't explain why adding this Gemfile method is a good idea, or what underlying problem is being solved. It's also somewhat problematic that this adds a new Gemfile method, but no documentation for it.

[segiddins]

This addresses the linked issue -- it's a way for the user to specify an explicit list of platforms to resolve for

[indirect]

The linked ticket and the todo list item don't contain any mention of a Gemfile DSL method. In general, DSL methods are best for options that need to effect every user with the Gemfile, but the user in the linked ticket has a need that only applies to his personal machine running Windows. Why is a Gemfile method the appropriate solution rather than a flag (or a global setting) that allows users to force the platform ruby?

The underlying problem in the linked ticket is a need to force the platform ruby, and the item in the todo list also says that users need a way to force the platform ruby (and only that platform). Why is the best solution to that very narrow problem a completely general solution that allows anyone to force any set of platforms in any Gemfile, even though Bundler already provides --add-platform and then respects the existing list of platforms?

Not really related, but I think documentation for Gemfile methods needs to be a blocker from now on—we have way too much stuff that no one knows about unless they read the commits.

[segiddins]

I'm not saying it's ready for merge, I'd add documentation before it made it into master. I suggested adding a DSL method and got no response, so I went and implemented the best solution to the general problem that I could think of. I'll write up why the current add platform is insufficient for this usecase next week

[homu]

☔ The latest upstream changes (presumably 42a36b0) made this pull request unmergeable. Please resolve the merge conflicts.

[indirect]

@segiddins ping for a discussion of DSL vs flags?

[segiddins]

My concern with the flags approach is that it can lead to platforms sneaking into the definition and potentially reducing the number of versions that can be resolved. I went with the DSL approach because it can allow the owner of a bundle to, for instance, choose to only resolve for universal-darwin instead of specific OS X versions. I wanted to avoid a flag that was specifically for 'only ruby platform' for a few reasons: 1) everyone installing would have to use that flag 2) what if you wanted only jruby or only rbx? 3) putting this in the gemfile means it's clear the exact set of platforms the bundle is meant to be used on 4) this allows users who know exactly what they want (I'm thinking specifically of large apps where there are ops people) to opt-out of bundler's platform "magic"/implicit behavior

[indirect]

I have two conces with using the Gemfile dsl to set allowed platforms:

1. it locks out users on other platforms (windows dev checks out project and then what happens?)
2. it directly conflicts with the existing lock flags that add and remove platforms

Can you elaborate on how this fits in with those things? So far through all of Bundler’s history, platforms have been listed in the lock file, allowing end users flexibility in their own platform without needing permission or intervention from the owner of the Gemfile.
The way the ruby DSL method works has led us to move it out of the Gemfile and into the lock, so this makes me worry we are doing the same thing again.

[indirect]

(Relatedly, the need to install with only the ruby platform is specific to a single machine, so I think it makes much more sense as a local or global setting rather than a flag or Gemfile method.)

[segiddins]

it locks out users on other platforms (windows dev checks out project and then what happens?)

They have to update the Gemfile, just as their lockfile would have to be updated

it directly conflicts with the existing lock flags that add and remove platforms

I view it more as an alternate way of managing platforms

(Relatedly, the need to install with only the ruby platform is specific to a single machine, so I think it makes much more sense as a local or global setting rather than a flag or Gemfile method.)

Except it would change the platforms listed in the lockfile, so in that sense it isn't limited to a specific machine

[indirect]

They have to update the Gemfile, just as their lockfile would have to be updated

In cases like gems under development (where the lock is not checked in), this forces any new platform to fork. That's not okay with me.

I view it more as an alternate way of managing platforms

In my experience, two conflicting ways to do the same thing is... not good. I'm not comfortable with adding this as long as we also have lock-based platform management.

Except it would change the platforms listed in the lockfile, so in that sense it isn't limited to a specific machine

Sorry, my bad at not communicating this clearly. Let me give you an example based on #4813:

Two developers working on the same project both use the same (maybe work-issued) machine. They're both on Windows. Both of their machines report the platform x64-mingw32. One of the developers has installed a full toolchain, and needs to install the gem from source. One of the developers has not installed a full toolchain, but has installed precompiled external dependencies.

This means that on one machine that reports itself as x64-mingw32, we need to install as if the machine only had the platform ruby. On the other machine, which also reports itself as x64-mingw32, we still need to install x64-mingw32 gems as usual.

It's impossible to do this by declaring platforms in the Gemfile.

[segiddins]

In cases like gems under development (where the lock is not checked in), this forces any new platform to fork. That's not okay with me.

Only if they use the gemfile DSL method, which there should be absolutely 0 reason to do if you're not committing the lockfile

This means that on one machine that reports itself as x64-mingw32, we need to install as if the machine only had the platform ruby. On the other machine, which also reports itself as x64-mingw32, we still need to install x64-mingw32 gems as usual.

That completely breaks the notion that Bundler lists every gem in the lockfile -- the ruby platform gem and the mingw one could be different

[segiddins]

@indirect would you maybe prefer the following approach? Is it more like what you had in mind?

diff --git a/lib/bundler.rb b/lib/bundler.rb
index f5bbd61..019b1ef 100644
--- a/lib/bundler.rb
+++ b/lib/bundler.rb
@@ -258,6 +258,11 @@ EOF
       with_clean_env { Kernel.exec(*args) }
     end

+    def local_platform
+      return Gem::Platform::RUBY if settings[:only_ruby_platform]
+      Gem::Platform.local
+    end
+
     def default_gemfile
       SharedHelpers.default_gemfile
     end
diff --git a/lib/bundler/current_ruby.rb b/lib/bundler/current_ruby.rb
index 6180285..7b3d87e 100644
--- a/lib/bundler/current_ruby.rb
+++ b/lib/bundler/current_ruby.rb
@@ -57,15 +57,15 @@ module Bundler
     end

     def mswin64?
-      Bundler::WINDOWS && Gem::Platform.local.os == "mswin64" && Gem::Platform.local.cpu == "x64"
+      Bundler::WINDOWS && Bundler.local_platform.os == "mswin64" && Bundler.local_platform.cpu == "x64"
     end

     def mingw?
-      Bundler::WINDOWS && Gem::Platform.local.os == "mingw32" && Gem::Platform.local.cpu != "x64"
+      Bundler::WINDOWS && Bundler.local_platform.os == "mingw32" && Bundler.local_platform.cpu != "x64"
     end

     def x64_mingw?
-      Bundler::WINDOWS && Gem::Platform.local.os == "mingw32" && Gem::Platform.local.cpu == "x64"
+      Bundler::WINDOWS && Bundler.local_platform.os == "mingw32" && Bundler.local_platform.cpu == "x64"
     end

     (KNOWN_MINOR_VERSIONS + KNOWN_MAJOR_VERSIONS).each do |version|
diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index 12951cd..5a168e3 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -523,7 +523,7 @@ module Bundler
     end

     def add_current_platform
-      current_platform = Bundler.rubygems.platforms.last
+      current_platform = Bundler.local_platform
       add_platform(current_platform) if Bundler.settings[:specific_platform]
       add_platform(generic(current_platform))
     end
diff --git a/lib/bundler/dsl.rb b/lib/bundler/dsl.rb
index 428ccd4..cdbae07 100644
--- a/lib/bundler/dsl.rb
+++ b/lib/bundler/dsl.rb
@@ -65,7 +65,7 @@ module Bundler
       case specs_by_name_and_version.size
       when 1
         specs = specs_by_name_and_version.values.first
-        spec = specs.find {|s| s.match_platform(Gem::Platform.local) } || specs.first
+        spec = specs.find {|s| s.match_platform(Bundler.local_platform) } || specs.first

         @gemspecs << spec

diff --git a/lib/bundler/gem_helpers.rb b/lib/bundler/gem_helpers.rb
index 6d926ce..955834f 100644
--- a/lib/bundler/gem_helpers.rb
+++ b/lib/bundler/gem_helpers.rb
@@ -25,7 +25,7 @@ module Bundler
     module_function :generic

     def generic_local_platform
-      generic(Gem::Platform.local)
+      generic(Bundler.local_platform)
     end
     module_function :generic_local_platform

diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb
index bed8754..18e87b4 100644
--- a/lib/bundler/settings.rb
+++ b/lib/bundler/settings.rb
@@ -17,6 +17,7 @@ module Bundler
       major_deprecations
       no_install
       no_prune
+      only_ruby_platform
       only_update_to_newer_versions
       plugins
       silence_root_warning
diff --git a/spec/cache/gems_spec.rb b/spec/cache/gems_spec.rb
index 474233a..19dd16e 100644
--- a/spec/cache/gems_spec.rb
+++ b/spec/cache/gems_spec.rb
@@ -238,7 +238,7 @@ describe "bundle cache" do
         gem "platform_specific"
       G

-      expect(cached_gem("platform_specific-1.0-#{Gem::Platform.local}")).to exist
+      expect(cached_gem("platform_specific-1.0-#{Bundler.local_platform}")).to exist
       expect(cached_gem("platform_specific-1.0-java")).to exist
     end

diff --git a/spec/commands/install_spec.rb b/spec/commands/install_spec.rb
index eb78ced..457fec2 100644
--- a/spec/commands/install_spec.rb
+++ b/spec/commands/install_spec.rb
@@ -216,7 +216,7 @@ describe "bundle install with gem sources" do
         G

         run "require 'platform_specific' ; puts PLATFORM_SPECIFIC"
-        expect(out).to eq("1.0.0 #{Gem::Platform.local}")
+        expect(out).to eq("1.0.0 #{Bundler.local_platform}")
       end

       it "falls back on plain ruby" do
diff --git a/spec/support/builders.rb b/spec/support/builders.rb
index 7436779..16ced2b 100644
--- a/spec/support/builders.rb
+++ b/spec/support/builders.rb
@@ -79,8 +79,8 @@ module Spec
         end

         build_gem "platform_specific" do |s|
-          s.platform = Gem::Platform.local
-          s.write "lib/platform_specific.rb", "PLATFORM_SPECIFIC = '1.0.0 #{Gem::Platform.local}'"
+          s.platform = Bundler.local_platform
+          s.write "lib/platform_specific.rb", "PLATFORM_SPECIFIC = '1.0.0 #{Bundler.local_platform}'"
         end

         build_gem "platform_specific" do |s|

[indirect]

@segiddins nice! let's do that as the immediate solution for this problem, and hold the rest of this PR in reserve while we see how things develop.

[segiddins]

Closing in favor of #5028.