Resolve for specific platforms #4836
Conversation
| @@ -52,7 +52,7 @@ class Dependency < Gem::Dependency | |||
| :x64_mingw_20 => Gem::Platform::X64_MINGW, | |||
| :x64_mingw_21 => Gem::Platform::X64_MINGW, | |||
| :x64_mingw_22 => Gem::Platform::X64_MINGW, | |||
| :x64_mingw_23 => Gem::Platform::X64_MINGW | |||
| :x64_mingw_23 => Gem::Platform::X64_MINGW, | |||
There was a problem hiding this comment.
really? we're doing trailing commas now? 😝
There was a problem hiding this comment.
we have been since introducing rubocop
There was a problem hiding this comment.
this way we don't have to touch that line when adding a new element to the array
There was a problem hiding this comment.
what good is it to be in charge of a project if we adopt code style that clashes with my personal coding aesthetic 😝
|
omg this is the most hopeful I have felt about platforms in literally five years, no exaggeration. hooray 🎉 |
segiddins
force-pushed
the
seg-resolve-for-specific-platforms
branch
4 times, most recently
from
87e310f
to
25d0a8e
Compare
|
@homu r+ |
|
📌 Commit 25d0a8e has been approved by |
…ndirect Resolve for specific platforms Closes #4295. This will require adding a bunch of tests, as well as figuring out how to put this new behavior behind a feature flag (thus fixing all of the existing tests).
|
@homu r- |
|
💔 Test failed - status |
|
@homu r+ |
|
📌 Commit 46f69e5 has been approved by |
|
@homu r- |
|
(Waiting to merge this until after we stable-branch 1.13) |
segiddins
force-pushed
the
seg-resolve-for-specific-platforms
branch
from
46f69e5
to
18450d5
Compare
|
This fixes #4896 |
| current_platform = Bundler.rubygems.platforms.map {|p| generic(p) }.compact.last | ||
| add_platform(current_platform) | ||
| current_platform = Bundler.rubygems.platforms.last | ||
| add_platform(current_platform) if Bundler.settings[:specific_platform] |
There was a problem hiding this comment.
we'll probably need to special-case around when in deployment mode and not add any platforms and count on the platforms check in #4895
|
@homu r+ |
|
📌 Commit 18450d5 has been approved by |
|
@indirect this should finally be mergable |
|
hooray 🏅 @homu r+ |
|
📌 Commit 42a36b0 has been approved by |
|
🔒 Merge conflict |
|
☔ The latest upstream changes (presumably #4654) made this pull request unmergeable. Please resolve the merge conflicts. |
:specific_platform is enabled
# Conflicts: # lib/bundler/cli/doctor.rb
segiddins
force-pushed
the
seg-resolve-for-specific-platforms
branch
from
42a36b0
to
0ac668d
Compare
|
Rebase'd |
|
📌 Commit 0ac668d has been approved by |
…ndirect Resolve for specific platforms Closes #4295. This will require adding a bunch of tests, as well as figuring out how to put this new behavior behind a feature flag (thus fixing all of the existing tests).
|
☀️ Test successful - status |
…rect [2.0] Enable specific_platform by default on 2.0 ### What was the end-user problem that led to this PR? The problem was that Bundler has somewhat suspect handling of multi-platform gems. We'd assume that different platform versions of gems were generally interchangeable, so if Bundler resolved to the "ruby" platform gem we'd just blindly try to swap in the gem for the local platform, which could lead to issues (say if the sets of dependencies were different). ### Was was your diagnosis of the problem? My diagnosis was that we needed to stop only working with the notion of "generic" platforms, which mapped everything to (basically) either java, pure ruby, and windows, and instead keep track of the actual platforms a bundle was being used on, and resolve for those specific platforms. ### What is your fix for the problem, implemented in this PR? My fix enables the changes made in #4836 by default on Bundler 2. ### Why did you choose this fix out of the possible options? I chose this fix because it means Bundler will default to more correct platforms semantics out of the box.
Closes #4295.
This will require adding a bunch of tests, as well as figuring out how to put this new behavior behind a feature flag (thus fixing all of the existing tests).