Index gems that come from a git or path source - #1011
Open
paracycle wants to merge 1 commit into
Open
Conversation
`Gem::Specification.find_by_name` only knows about gems that RubyGems installed. A gem the Gemfile takes from a `git` or `path` source is never registered with RubyGems, so the lookup raises, the rescue swallows it, and the gem is left out of the workspace silently. That is not a rare corner. On a large workspace that pins its framework to a branch, 204 of 882 locked specs were dropped this way: 154 from path sources, 36 from git sources. Every Rails gem was among them, so nothing defined `ActiveRecord::Base`. The cost is far larger than the missing gems, because Ruby resolves a constant by searching the ancestors of the enclosing namespace. A class that inherits from a missing superclass never linearizes its ancestor chain, and then no constant inside that class resolves either, however well defined it is. Measured on that workspace, indexing the git sources took the unresolved work queue from 153,583 items to 34,601, and incomplete ancestor chains from 75,028 to 4,009. Bundler keeps an index per source that knows both where it checked the source out and what the gemspec declares, so ask the source when RubyGems cannot help. The index is built once per source: a multi-gem repository such as rails shares one source across every gem it provides, and building it reads gemspecs off disk. Take the directories from the gemspec rather than assuming `lib`. Of the gems resolved this way, `grpc` alone declares `src/ruby/lib`, `src/ruby/bin` and `src/ruby/pb`, and none of them would have been found. A source that is not checked out yet still leaves its gems unindexed, rather than failing the whole workspace.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
Graph#add_workspace_dependency_pathsfinds a dependency's code throughGem::Specification.find_by_name. That method only knows about gems RubyGems installed. A gem the Gemfile takes from agitorpathsource is never registered with RubyGems, so the lookup raisesGem::MissingSpecError, the rescue swallows it, and the gem is left out of the workspace without any signal.This is not a rare corner. On a large workspace that pins its framework to a branch, 204 of 882 locked specs were dropped: 154 from path sources, 36 from git sources. Every Rails gem was among them, so nothing in the graph defined
ActiveRecord::Base.The damage is much wider than the missing gems. Ruby resolves a constant by searching the ancestors of the enclosing namespace. A class that inherits from a missing superclass never linearizes its ancestor chain, and then no constant inside that class resolves either, however well defined it is. Constants as ordinary as
ArrayandHashwere reported unresolved even with the RBS core definitions indexed.Change
When RubyGems cannot find the gem, ask Bundler. Bundler keeps an index per source that knows both where it checked the source out and what the gemspec declares.
Two details matter:
libis wrong often enough to matter. Of the gems resolved this way,grpcalone declaressrc/ruby/lib,src/ruby/binandsrc/ruby/pb. Across the workspace, 77 gems declare something other than["lib"], includinglib/concurrent-ruby,rubyandclient.A source that is not checked out yet still leaves its gems unindexed rather than failing the whole workspace, which is the behaviour today.
Effect
Measured on a workspace of about 110,000 files whose Gemfile takes Rails from a branch.
Constants such as
TestCase,Arel,RecordInvalidandRecordNotFoundresolve after the change and did not before.Notes for the reviewer
Bundler.load.specswould be the tidier API, but it materializes the whole bundle and raises on a workspace whose lockfile and checkout disagree. The current code deliberately avoids requiring a working bundle, and this change keeps that property.