Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some bad PRs getting created when compact index API is having issues #7269

Merged

Conversation

deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented May 9, 2023

If Bundler needs to fallback to fetching dependency information from the old API, we need to double check that the target version Bundler said it's compatible, it's actually compatible with the locked Ruby version.

This is because this API ignores requirements on the Ruby version.

This was already checked when figuring out the latest resolvable version, but it was not checked when falling back to trying to unlock all dependencies.

This commit should fix that.

Fixes #7257.

@github-actions github-actions bot added the L: ruby:bundler RubyGems via bundler label May 9, 2023
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/prevent-updates-incompatible-with-ruby branch from 4f8f046 to 580bc8e Compare May 9, 2023 19:40
Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this API ignores requirements on the Ruby version

To clarify, are you referring here to the old API or the compact API?

It is surprising that we have to handle this edge case in our custom code, and not in bundler nor our custom native helper for bundler, but if anyone would understand the intricacies here, it'd be you...

Assuming this is the best design, then the code implementation looks fine to me.

@deivid-rodriguez
Copy link
Contributor Author

deivid-rodriguez commented May 9, 2023

To clarify, are you referring here to the old API or the compact API?

To the old API. The CompactIndex API should be always correct, however, it's been having issues for the last few days, so Bundler falls back to the old API, which mostly works, but not in this edge case.

It is surprising that we have to handle this edge case in our custom code, and not in bundler nor our custom native helper for bundler, but if anyone would understand the intricacies here, it'd be you...

I first tried to monkeypatch Bundler to not use the dependency API, but then if the CompactIndex API is having issues like now... nothing works. Then I found that we were already explicitly handling this case, just not everywhere where needed, so I just fixed the existing approach. By the way, I think CI is failing because.... the CompactIndex API is having issues 😞, I believe we're missing some VCR cassettes in there.

@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/prevent-updates-incompatible-with-ruby branch from 580bc8e to 460bc0f Compare May 9, 2023 20:21
@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review May 10, 2023 07:49
@deivid-rodriguez deivid-rodriguez requested a review from a team as a code owner May 10, 2023 07:49
If Bundler needs to fallback to fetching dependency information from the
old API, we need to double that the target version Bundler said it's
compatible, it's actually compatible with the locked Ruby version.

This is because this API ignores requirements on the Ruby version.

This was already checked when figuring out the latest resolvable
version, but it was not checked when falling back to trying to unlock
all dependencies.

This commit should fix that.
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/prevent-updates-incompatible-with-ruby branch from 460bc0f to c0008f1 Compare May 10, 2023 07:49
@deivid-rodriguez deivid-rodriguez merged commit 2389cc7 into main May 10, 2023
@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/prevent-updates-incompatible-with-ruby branch May 10, 2023 10:16
brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
…ues (dependabot#7269)

If Bundler needs to fallback to fetching dependency information from the
old API, we need to double that the target version Bundler said it's
compatible, it's actually compatible with the locked Ruby version.

This is because this API ignores requirements on the Ruby version.

This was already checked when figuring out the latest resolvable
version, but it was not checked when falling back to trying to unlock
all dependencies.

This commit should fix that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: ruby:bundler RubyGems via bundler
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Gems that require Ruby 3.0+ are giving updates when on Ruby 2.7.8 that don't run or work
2 participants