This repository has been archived by the owner on Apr 14, 2021. It is now read-only.
Normalize file URIs in spec's lockfiles #7089
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
Get `have_lockfile` helper method actually used.
This change is simple but not easy to explain. What this test does is to ensure that if bundle install doesn't change anything, then the bundler version in the lockfile does not get rewritten. Whereas this test was passing, the implementation was wrong, but two bugs were "compesating for each other" and making it pass. The explanation is the following: Due to a regression in rubygems, in rubygems 3+ "2.0.0.0.a" is considered higher than "2.0.0.dev". This changes the test because bundler will never dowgrade the bundler version in a lockfile, so even if the lockfile gets rewritten, it will still keep "2.0.0.0.a", since it's considered higher. However, if we change the test to use "2.0.0.a" instead of "2.0.0.dev", then the test starts failing and shows the bug in the test. The bug is that the lock file in the test is written with "file://localhost" style URLs. However, the lockfile that would be generated would contain "file://" style URLs. This would trick bundler into thinking that something has been changed by `bundle install`, so it will rewrite the lock file, use "2.0.0.dev" as the locked bundler version, and cause the test to fail.
deivid-rodriguez
changed the title
|
This PR only affects tests, but it's very intricate. I rebased the bug fix in rubygems for one of the issues involved here, and exactly this spec failed when testing against master. Seeing that failure can help understanding the bug in this test. |
This was referenced Apr 1, 2019
|
👍 After merging this, I'm going to rebase #7088 |
|
@bundlerbot r+ |
ghost
pushed a commit
that referenced
this pull request
Apr 2, 2019
7089: Normalize file URIs in spec's lockfiles r=hsbt a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was that bumping bundler's version in #7088 made a lockfile spec fail. ### What was your diagnosis of the problem? My diagnosis was that there was a combination of bugs that was making this spec pass, but the spec was incorrect. ### What is your fix for the problem, implemented in this PR? My fix is to change the test version the spec uses to not hit rubygems/rubygems#2595, and thus reproduce the failure. And then fix the spec that was incorrect because the lockfile written had different URLs (`file://localhost/<stuff>`) from the lockfile we were checking against (`file://<stuff>`), thus tricking `bundle install` into thinking something had changed, and thus making it rewrite it with an incorrect bundler version. ### Why did you choose this fix out of the possible options? I chose this fix because it makes #7088 pass and it reduces surprises. Co-authored-by: David Rodríguez <[email protected]>
Build succeeded |
ghost
merged commit
colby-swandale
pushed a commit
that referenced
this pull request
Apr 8, 2019
7089: Normalize file URIs in spec's lockfiles r=hsbt a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was that bumping bundler's version in #7088 made a lockfile spec fail. ### What was your diagnosis of the problem? My diagnosis was that there was a combination of bugs that was making this spec pass, but the spec was incorrect. ### What is your fix for the problem, implemented in this PR? My fix is to change the test version the spec uses to not hit rubygems/rubygems#2595, and thus reproduce the failure. And then fix the spec that was incorrect because the lockfile written had different URLs (`file://localhost/<stuff>`) from the lockfile we were checking against (`file://<stuff>`), thus tricking `bundle install` into thinking something had changed, and thus making it rewrite it with an incorrect bundler version. ### Why did you choose this fix out of the possible options? I chose this fix because it makes #7088 pass and it reduces surprises. Co-authored-by: David Rodríguez <[email protected]> (cherry picked from commit 4cae424)
colby-swandale
added a commit
that referenced
this pull request
Jun 13, 2019
* 2-0-stable: (89 commits) fix changelog 2.0.2 typos add v2.0.2 changelog bump version to 2.0.2 Merge #7199 fix bug where bundler v3 is running a test for bundflet 2 Merge #6798 add bors configuation port GemHelper from master Merge #7080 Merge #7089 Merge #7068 Merge #7036 Merge #7067 change Bundler 3 specs in travis to use RubyGems 3.0.3 bump RubyGems v3 to the latest version on Travis Merge #6963 Merge #7078 Merge pull request #7061 from bundler/fix_circular_requires Merge #6864 remove linting step in travis (it will still run in each build) ...
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 that bumping bundler's version in #7088 made a lockfile spec fail.
What was your diagnosis of the problem?
My diagnosis was that there was a combination of bugs that was making this spec pass, but the spec was incorrect.
What is your fix for the problem, implemented in this PR?
My fix is to change the test version the spec uses to not hit rubygems/rubygems#2595, and thus reproduce the failure. And then fix the spec that was incorrect because the lockfile written had different URLs (
file://localhost/<stuff>) from the lockfile we were checking against (file://<stuff>), thus trickingbundle installinto thinking something had changed, and thus making it rewrite it with an incorrect bundler version.Why did you choose this fix out of the possible options?
I chose this fix because it makes #7088 pass and it reduces surprises.