search

dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.65k stars 1.01k forks source link

Dependabot removing platform specific gems from Gemfile.lock #5917

Closed thomasnemer closed 1 year ago

thomasnemer commented 1 year ago

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

    - google-protobuf (3.21.7)
    + google-protobuf (3.21.7-x86_64-linux)
    - mini_portile2 (2.8.0)
    - nokogiri (1.13.8)
    -    mini_portile2 (~> 2.8.0)
    + nokogiri (1.13.8-x86_64-linux)
      sorbet (0.5.10346)
        sorbet-static (= 0.5.10346)
      sorbet-runtime (0.5.10346)
    - sorbet-static (0.5.10346-universal-darwin-14)
    - sorbet-static (0.5.10346-universal-darwin-15)
    - sorbet-static (0.5.10346-universal-darwin-16)
    - sorbet-static (0.5.10346-universal-darwin-17)
    - sorbet-static (0.5.10346-universal-darwin-18)
    - sorbet-static (0.5.10346-universal-darwin-19)
    - sorbet-static (0.5.10346-universal-darwin-20)
    - sorbet-static (0.5.10346-universal-darwin-21)
    - sorbet-static (0.5.10346-universal-darwin-22)
      sorbet-static (0.5.10346-x86_64-linux)
      sorbet-static-and-runtime (0.5.10346)
        sorbet (= 0.5.10346)
    - syntax_tree (3.6.2)
    + syntax_tree (3.6.3)

dependabot.yml content

version: 2
updates:
  - package-ecosystem: 'bundler'
    pull-request-branch-name:
      separator: '-'
    directory: '/'
    schedule:
      interval: 'daily'
    labels:
      - 'dependencies'
      - 'bundler'
      - 'auto-merge'
      - 'auto-approve'
    ignore:
      # https://github.com/dependabot/dependabot-core/issues/5420 Try again after next release of dependabot
      - dependency-name: "sorbet-static-and-runtime"
  - package-ecosystem: 'npm'
    pull-request-branch-name:
      separator: '-'
    directory: '/'
    schedule:
      interval: 'daily'
    labels:
      - 'dependencies'
      - 'npm'
      - 'auto-merge'
      - 'auto-approve'
    ignore:
      # https://github.com/axios/axios/issues/5026 Try again after next release of dependabot
      - dependency-name: "axios"

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

image

Smallest manifest that reproduces the issue

not tested yet :

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'
thomasnemer commented 1 year ago

It really feels like this issue, but I can't re-open it, so I created a new one.

deivid-rodriguez commented 1 year ago

Uugggh, this issue keeps chasing me, both upstream and here. I will have a look at this, thanks for reporting!

deivid-rodriguez commented 1 year ago

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

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)
      racc (~> 1.4)
    prettier_print (1.0.0)
    racc (1.6.0)
    sorbet (0.5.10346)
      sorbet-static (= 0.5.10346)
    sorbet-runtime (0.5.10346)
    sorbet-static (0.5.10346-universal-darwin-14)
    sorbet-static (0.5.10346-universal-darwin-15)
    sorbet-static (0.5.10346-universal-darwin-16)
    sorbet-static (0.5.10346-universal-darwin-17)
    sorbet-static (0.5.10346-universal-darwin-18)
    sorbet-static (0.5.10346-universal-darwin-19)
    sorbet-static (0.5.10346-universal-darwin-20)
    sorbet-static (0.5.10346-universal-darwin-21)
    sorbet-static (0.5.10346-universal-darwin-22)
    sorbet-static (0.5.10346-x86_64-linux)
    sorbet-static-and-runtime (0.5.10346)
      sorbet (= 0.5.10346)
      sorbet-runtime (= 0.5.10346)
    syntax_tree (3.6.2)
      prettier_print

PLATFORMS
  ruby

DEPENDENCIES
  bundler (= 2.3.14)
  google-protobuf (= 3.21.7)
  nokogiri (= 1.13.8)
  sorbet-static-and-runtime (= 0.5.10346)
  syntax_tree

RUBY VERSION
   ruby 3.1.2p20

BUNDLED WITH
   2.3.14

If you edit the Gemfile like this

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 bundle update --bundler.

Then you get a diff like yours.

So this seems like an upstream issue.

deivid-rodriguez commented 1 year ago

@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 sorbet-static, which does not have any versions that are NOT platform specific.

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.

thomasnemer commented 1 year ago

Thanks for the insight and the investigation :bow: We'll specify our platforms in the Gemfile and see how it goes with dependabot next time :)

ashkulz commented 1 year ago

@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?

deivid-rodriguez commented 1 year ago

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.

danielvdao commented 1 year ago

@deivid-rodriguez Do you happen to know what the upstream fix was regarding this:

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 sorbet-static, which does not have any versions that are NOT platform specific.

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.

Our team is investigating something similar, we're seeing that dependabot is throwing away the ruby platform and removing sorbet-static-* gems and only adding ones that match our platform (in our case, it's arm64-darwin-21 which seems to resolve to sorbet-static-0.5.10263-universal-darwin-21).

It sounds like that is the expected behavior based on the change you're mentioning because sorbet-static doesn't satisfy ruby platform as well, but I'd love to understand the full picture.

deivid-rodriguez commented 1 year ago

@danielvdao Of course. The relevant changes are https://github.com/rubygems/rubygems/pull/5495, later amended by https://github.com/rubygems/rubygems/pull/5807.

ashkulz commented 1 year ago

@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 😅

deivid-rodriguez commented 1 year ago

We just got that proposed at #6042, so we will move it forward soon!

jeffwidman commented 1 year ago

Can this be closed now that #6042 is merged/deployed?

deivid-rodriguez commented 1 year ago

Yeah, there's still another issue that I mentioned at https://github.com/dependabot/dependabot-core/issues/5917#issuecomment-1282272289, but I'll move that upstream. Thanks for the remainder!