[2.0] Remove RubyGems Aggregate & support transitive source pinning#4714
[2.0] Remove RubyGems Aggregate & support transitive source pinning#4714
Conversation
lib/bundler/definition.rb
Outdated
| ret | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Is there an extremely good reason to make this a prod instead of a method? Procs are sooo much slower than method calls. Also, procs are much harder to read and understand than methods... like how i can't even tell if this has to be a prof instead of a method. 😝
There was a problem hiding this comment.
I can actually inline it entirely -- I don't like methods because the can be used elsewhere :P
There was a problem hiding this comment.
inline is fine, but you should get used to private methods :P
|
@indirect addressed feedback |
| " gem 'rails'\n" \ | ||
| " end" | ||
| raise DeprecatedError, msg | ||
| end |
There was a problem hiding this comment.
We're sure this still works with gem "foo", path: "bar", right?
There was a problem hiding this comment.
yeah, it does what git does and uses and empty block to fetch the source when used as an option: https://github.com/bundler/bundler/blob/4327cfe160f45f1bb0f02b9ea8b7e006ec255e02/lib/bundler/dsl.rb#L343
|
@homu r+ |
|
📌 Commit 4327cfe has been approved by |
|
💔 Test failed - status |
|
Locally I get a failure at |
|
@indirect I can't repro the issue locally at all |
Unpinned back deps still pull from an arbitrary source at this point, will have to fix that later
Also the re-introduction of the guarentee of at least 1 gem source
Accounting for the removal of the rubygems aggregate
segiddins
force-pushed
the
seg-no-aggregate
branch
3 times, most recently
from
5cccf5a to
5575151
Compare
segiddins
force-pushed
the
seg-no-aggregate
branch
from
3cd25ba to
baa7112
Compare
|
@homu r=indirect |
|
📌 Commit baa7112 has been approved by |
|
⚡ Test exempted - status |
|
🎉🎉🎉🎉 |
[2.0] Remove RubyGems Aggregate & support transitive source pinning ### What was the end-user problem that led to this PR? The problem was that the resolver could resolve specs from _any_ of the sources specified in the Gemfile, even if that source had nothing to do with the spec in question. This was such a large security vulnerability that, when discovered, it warranted a CVE and its own minor release of Bundler. Closes #3671. Closes #3696. Closes #4059. ### Was was your diagnosis of the problem? My diagnosis was that we needed to get rid of the notion of a `rubygems aggregate` and enforce that specs could only come either from the source they were declared to come from (the top-level source if declared at the top-level of the Gemfile, else a scoped source), or a source that it transitively "inherited" from the gems that required it. ### What is your fix for the problem, implemented in this PR? My fix is to disable multiple top-level sources in the Gemfile, remove the RubyGems aggregate, and filter the sources gems could come from as described above. ### Why did you choose this fix out of the possible options? I chose this fix because it allows doing the filtering in a reasonably performant manner, and refactors the way we handle sources to abstract some of the grossness in such a way that the machinations to make sure that all of the necessary gem info is downloaded is encapsulated into a single method, driven from the definition, rather than being specific to rubygems sources. See #4714 and #4930 for the prior implementation.
5 participants
Closes #3671.
Closes #3696.
Closes #4059.