This repository has been archived by the owner on Apr 14, 2021. It is now read-only.
Fix using gemspec & force_ruby_platform on windows #6809
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deivid-rodriguez
approved these changes
Nov 26, 2018
segiddins
force-pushed
the
segiddins/gemspec-force-ruby-platform
branch
from
fae7715
to
7d84ff4
Compare
deivid-rodriguez
force-pushed
the
segiddins/gemspec-force-ruby-platform
branch
from
7d84ff4
to
d0fcdb9
Compare
deivid-rodriguez
force-pushed
the
segiddins/gemspec-force-ruby-platform
branch
from
d0fcdb9
to
9bf9864
Compare
deivid-rodriguez
force-pushed
the
segiddins/gemspec-force-ruby-platform
branch
3 times, most recently
from
e90c562
to
b0294c1
Compare
deivid-rodriguez
force-pushed
the
segiddins/gemspec-force-ruby-platform
branch
2 times, most recently
from
f8b0c92
to
2dbc323
Compare
|
@segiddins I reverted 2 of the commits in this PR, and that fixed the new test that you added. I also squashed the added test, with the commit that's making it pass. 493ff58 is not needed for this PR, but I left it there because I think it's a good refactoring :) |
|
By the way, this PR as it is now seems to reduce the number of Windows spec failures from 356 to 349 😃 |
…::Platform::RUBY This allows us to always say we're ruby? when force_ruby_platform is set, and fixes using gemspec & force_ruby_platform on windows.
deivid-rodriguez
force-pushed
the
segiddins/gemspec-force-ruby-platform
branch
from
2dbc323
to
3cb89b7
Compare
|
I did try this PR, and it fixed the original problem. It also includes a regression spec, so I plan to merge it tomorrow unless there's feedback. |
|
@bundlerbot r+ |
ghost
pushed a commit
that referenced
this pull request
Jul 26, 2019
6809: Fix using gemspec & force_ruby_platform on windows r=deivid-rodriguez a=segiddins ### What was the end-user problem that led to this PR? The problem was using `gemspec` and `force_ruby_platform` on Windows would lead to gems not being requirable. Fixes #6801. ### What was your diagnosis of the problem? My diagnosis was there was a place where `force_ruby_platform` wasn't being taken into account, namely the query methods on `Bundler.current_ruby` ### What is your fix for the problem, implemented in this PR? My fix was to add a check for the local platform in `current_ruby`, so the `force_ruby_platform` override would be taken into account. ### Why did you choose this fix out of the possible options? I chose this fix because it avoids hard-coding knowledge of the setting in more places. Co-authored-by: Samuel Giddins <[email protected]> Co-authored-by: David Rodríguez <[email protected]>
Build succeeded |
ghost
merged commit
This pull request was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was the end-user problem that led to this PR?
The problem was using
gemspecandforce_ruby_platformon Windows would lead to gems not being requirable.Fixes #6801.
What was your diagnosis of the problem?
My diagnosis was there was a place where
force_ruby_platformwasn't being taken into account, namely the query methods onBundler.current_rubyWhat is your fix for the problem, implemented in this PR?
My fix was to add a check for the local platform in
current_ruby, so theforce_ruby_platformoverride would be taken into account.Why did you choose this fix out of the possible options?
I chose this fix because it avoids hard-coding knowledge of the setting in more places.