select a pre-release if it's specified in the Gemfile #6570
Conversation
|
Thanks for opening a pull request and helping make Bundler better! Someone from the Bundler team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality. We use Travis CI to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of Travis CI in the PR status window below. If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #bundler channel on Slack. For more information about contributing to the Bundler project feel free to review our CONTRIBUTING guide |
8344558 to
87f8d64
Compare
|
Thanks! @greysteil any objections to me merging this? |
|
@akihiro17 - nice work on this! It's a thorny part of the resolution process to be digging into. I've confirmed locally that this fixes the issue and the test is 👌. As I understand it, the logic here is "allow a pre-release version for a non-pre-release requirement if any of the sub-dependency requirements of that pre-releases version specify pre-releases". I don't think that logic makes sense, and think this might instead be working because actually we can remove line 186-189 entirely. It was added before #6014 was, and I think #6014 should do everything required to ensure pre-releases aren't selected when another option is possible. What do you reckon? Am I missing something in the logic here? |
|
Thank you for your review. I think we can remove pre-release logic(line 186-19) from |
|
Awesome - do you want to update this PR with that change? I'm then 👍. @segiddins - sound good to you? |
I'll update the PR later. Thanks. |
because rubygems#6014 avoids pre-releases when a non-pre-release version is possible
87f8d64 to
01a2200
Compare
|
This seems fine? But with this sort of changes, I've learned it's hard to be 100% sure ahead of time @bundlerbot r+ |
|
📌 Commit 01a2200 has been approved by |
select a pre-release if it's specified in the Gemfile ### What was the end-user problem that led to this PR? The problem was #6449 Closes #6449 ### What was your diagnosis of the problem? My diagnosis was that Bundler didn't select a pre-release even if one of the dependencies of a gem was specified with a pre-release version in the Gemfile. ### What is your fix for the problem, implemented in this PR? My fix is to change `Bundler::Resolver#requirement_satisfied_by?` so that it does not reject a pre-release if at least one of the dependencies is pre-released. ### Why did you choose this fix out of the possible options?
|
☀️ Test successful - status-travis |
select a pre-release if it's specified in the Gemfile ### What was the end-user problem that led to this PR? The problem was #6449 Closes #6449 ### What was your diagnosis of the problem? My diagnosis was that Bundler didn't select a pre-release even if one of the dependencies of a gem was specified with a pre-release version in the Gemfile. ### What is your fix for the problem, implemented in this PR? My fix is to change `Bundler::Resolver#requirement_satisfied_by?` so that it does not reject a pre-release if at least one of the dependencies is pre-released. ### Why did you choose this fix out of the possible options? (cherry picked from commit a97917c)
* 1-16-stable: Version 1.16.4 with changelog Auto merge of #6668 - eregon:fix-encoding-spec-from-6661, r=deivid-rodriguez Add encoding magic comment to gemfile spec Auto merge of #6662 - bundler:indirect/update-authors, r=colby-swandale Auto merge of #6650 - greysteil:dont-mutate-original-trees, r=segiddins Auto merge of #6661 - eregon:consistent-encoding-for-reading-files, r=deivid-rodriguez Auto merge of #6652 - bundler:seg-molinillo-0.6.6, r=segiddins Auto merge of #6645 - bundler:colby/require-etc, r=colby-swandale Auto merge of #6636 - ojab:1-16-stable, r=indirect Auto merge of #6624 - bundler:no-document, r=colby-swandale Auto merge of #6621 - ralphbolo:patch-1, r=segiddins Auto merge of #6613 - kemitchell:mention-show-sorts-in-doc, r=colby-swandale Auto merge of #6570 - akihiro17:prerelease-dependency, r=segiddins Auto merge of #6568 - greysteil:conservative-groups, r=segiddins
What was the end-user problem that led to this PR?
The problem was #6449
Closes #6449
What was your diagnosis of the problem?
My diagnosis was that Bundler didn't select a pre-release even if one of the dependencies of a gem was specified with a pre-release version in the Gemfile.
What is your fix for the problem, implemented in this PR?
My fix is to change
Bundler::Resolver#requirement_satisfied_by?so that it does not reject a pre-release if at least one of the dependencies is pre-released.Why did you choose this fix out of the possible options?