Only check circular deps when dependency api is available, not on full index sources #6919
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
martinemde
reviewed
Aug 25, 2023
| # Don't bother to check for circular deps when no dependency API are | ||
| # available, since it's too slow to be usable. That edge case won't work | ||
| # but resolution other than that should work fine and reasonably fast. | ||
| if source.respond_to?(:dependency_api_available?) && source.dependency_api_available? |
There was a problem hiding this comment.
Ah ok. Nice. I couldn't figure out where this check belonged. I tried putting something on RemoteSpecification that would tell it not to use the source to load dependencies, but it was a bit ugly. I'll see if I can write a test for this and we can merge.
There was a problem hiding this comment.
I'm leaving for a few weeks, so feel free to proceed as you see fit. We could close this PR, cherry-pick these two commits there, and get the issue fixed there?
martinemde
force-pushed
the
deivid-rodriguez/dont-check-circular-deps-on-full-index-sources
branch
from
19e12eb
to
7ff0f97
Compare
martinemde
approved these changes
Aug 25, 2023
martinemde
force-pushed
the
deivid-rodriguez/dont-check-circular-deps-on-full-index-sources
branch
from
7ff0f97
to
d275cdc
Compare
martinemde
changed the title
|
the ruby-core specs are failing to install ruby because of a temporary bug. I'm going to merge this anyway since the fails are not caused by this code. |
martinemde
deleted the
deivid-rodriguez/dont-check-circular-deps-on-full-index-sources
branch
4 tasks
deivid-rodriguez
pushed a commit
that referenced
this pull request
Sep 21, 2023
…rcular-deps-on-full-index-sources Only check circular deps when dependency api is available, not on full index sources (cherry picked from commit 5f9545c)
deivid-rodriguez
pushed a commit
that referenced
this pull request
Sep 21, 2023
…rcular-deps-on-full-index-sources Only check circular deps when dependency api is available, not on full index sources (cherry picked from commit 5f9545c)
deivid-rodriguez
added a commit
to dependabot/dependabot-core
that referenced
this pull request
Nov 8, 2023
* `Bundler::Fetcher#fetchers` was made private at rubygems/rubygems#6919. * `Bundler::Index#search_all` returns an enumerator since rubygems/rubygems#6962.
deivid-rodriguez
added a commit
to dependabot/dependabot-core
that referenced
this pull request
Nov 9, 2023
* `Bundler::Fetcher#fetchers` was made private at rubygems/rubygems#6919. * `Bundler::Index#search_all` returns an enumerator since rubygems/rubygems#6962.
deivid-rodriguez
added a commit
to dependabot/dependabot-core
that referenced
this pull request
Nov 15, 2023
* `Bundler::Fetcher#fetchers` was made private at rubygems/rubygems#6919. * `Bundler::Index#search_all` returns an enumerator since rubygems/rubygems#6962.
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.
What was the end-user or developer problem that led to this PR?
This check fixes some edge cases during resolution, but checking it when dependency APIs are not available is too slow.
What is your fix for the problem, implemented in this PR?
This PR builds on top of #6897 to disable this check.
Fixes #6828.
I tried this manually and it's back to fast but I'd probably add a spec before merging?
Make sure the following tasks are checked