-
Notifications
You must be signed in to change notification settings - Fork 916
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
Dependabot removing platform specific gems from Gemfile.lock #5917
Comments
It really feels like this issue, but I can't re-open it, so I created a new one. |
Uugggh, this issue keeps chasing me, both upstream and here. I will have a look at this, thanks for reporting! |
I confirmed this behavior using Bundler only. With source "https://rubygems.org"
ruby '3.1.2'
gem 'bundler', '2.3.14'
gem 'sorbet-static-and-runtime', '0.5.10346'
gem 'syntax_tree'
gem 'google-protobuf', '3.21.7'
gem 'nokogiri', '1.13.8' and
If you edit the diff --git a/Gemfile b/Gemfile
index 1307105..d52785c 100644
--- a/Gemfile
+++ b/Gemfile
@@ -2,7 +2,7 @@ source "https://rubygems.org"
ruby '3.1.2'
-gem 'bundler', '2.3.14'
+gem 'bundler', '2.3.24'
gem 'sorbet-static-and-runtime', '0.5.10346'
gem 'syntax_tree'
gem 'google-protobuf', '3.21.7' and run Then you get a diff like yours. So this seems like an upstream issue. |
@thomasnemer Ok now I remember. So this is due to a recent upstream change (note that Dependabot is already at Bundler 2.3.22). Basically, your lock file is not resolvable using the "RUBY" platform only (by the "RUBY" platform we mean, using only gems that are not platform specific). The reason is that the lock file includes This was previously tolerated, but in order to fix other platform issues, we had to start being strict about it. But instead of failing, we decided to automatically fix the problematic lock files, locking them to a proper specific platform where the lock file is valid. I think there's an issue here though, and that is that Bundler should not remove your existing platform specific gems. So the expectation here would be a diff like this: diff --git a/Gemfile.lock b/Gemfile.lock
index caf5c86..c70d427 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -2,9 +2,11 @@ GEM
remote: https://rubygems.org/
specs:
- google-protobuf (3.21.7)
- mini_portile2 (2.8.0)
- nokogiri (1.13.8)
- mini_portile2 (~> 2.8.0)
+ google-protobuf (3.21.7-x86_64-darwin)
+ google-protobuf (3.21.7-x86_64-linux)
+ nokogiri (1.13.8-arm64-darwin)
+ racc (~> 1.4)
+ nokogiri (1.13.8-x86_64-darwin)
+ racc (~> 1.4)
+ nokogiri (1.13.8-x86_64-linux)
racc (~> 1.4)
prettier_print (1.0.0)
racc (1.6.0)
@@ -28,10 +30,18 @@ GEM
prettier_print
PLATFORMS
- ruby
+ universal-darwin-14
+ universal-darwin-15
+ universal-darwin-16
+ universal-darwin-17
+ universal-darwin-18
+ universal-darwin-19
+ universal-darwin-20
+ universal-darwin-22
+ x86_64-linux
DEPENDENCIES
It would be nice if Dependabot showed some message in the PR when it replaces the Ruby platform with something else, and why, but it's not something it can currently do. |
Thanks for the insight and the investigation 🙇 |
@deivid-rodriguez I think this is a known issue, and were going to solve it in a bundler update. Did that not happen with 2.3.22? |
Which problem are you having, exactly the same? Here Dependabot is behaving as expected, and Bundler is having an issue where it's removing too many platforms. Yet, removing the RUBY one in particular is correct. I plan to move this issue to the Bundler repo. |
@deivid-rodriguez Do you happen to know what the upstream fix was regarding this:
Our team is investigating something similar, we're seeing that dependabot is throwing away the It sounds like that is the expected behavior based on the change you're mentioning because |
@danielvdao Of course. The relevant changes are rubygems/rubygems#5495, later amended by rubygems/rubygems#5807. |
@deivid-rodriguez are there any plans to update dependabot to 2.3.24 to solve the bundler taking too long to resolve issue? We keep our bundler version in sync with Dependabot via GH Actions, as otherwise we tend to forget to update 😅 |
We just got that proposed at #6042, so we will move it forward soon! |
Can this be closed now that #6042 is merged/deployed? |
Yeah, there's still another issue that I mentioned at #5917 (comment), but I'll move that upstream. Thanks for the remainder! |
We're seeing similar issues described in this issue: dependabot/dependabot-core#5917 And it looks like most of the issues came from bundler, not dependabot itself. Given that - ruby-lsp is using a relatively old version of bundler (2.3.7 was released in Feb 2022) - dependabot now uses 2.3.25 by default I think it's worth trying out a new bundler to see if it helps resolving the problem.
We're seeing similar issues described in this issue: dependabot/dependabot-core#5917 And it looks like most of the issues came from bundler, not dependabot itself. Given that - ruby-lsp is using a relatively old version of bundler (2.3.7 was released in Feb 2022) - dependabot now uses 2.3.25 by default I think it's worth trying out a new bundler to see if it helps resolving the problem. And if we compare this commit's change with #410, we can see that the new bundler already generates the entries we'd like to see. So I think the result is promising.
Is there an existing issue for this?
Package ecosystem
Bundler
Package manager version
2.3.14
Language version
Ruby 3.1.2
Manifest location and content before the Dependabot update
/Gemfile.lock
dependabot.yml content
Updated dependency
syntax_tree from 3.6.2 to 3.6.3
What you expected to see, versus what you actually saw
I would have expected dependabot not to introduce platform-specific dependencies where there was none (google-protobuf and nokogiri), and expected to keep the existing ones (sorbet-static)
Native package manager behavior
On a linux workstation everything is fine as expected since it's the same architecture as dependabot. On a macos workstation, the bundler modifies the manifest to add again those missing platforms.
Images of the diff or a link to the PR, issue, or logs
Smallest manifest that reproduces the issue
not tested yet :
The text was updated successfully, but these errors were encountered: