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

Automatically remove "ruby" from lockfile if incomplete #5807

Merged

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented Aug 2, 2022

What was the end-user or developer problem that led to this PR?

With #5495 we started being strict about lockfiles locked to "ruby" but not including exclusively gems NOT platform specific.

This was done because:

However, with this fix, bundle update or bundle outdated can no longer be run on already existing lockfiles with the previous problem, generating a missing gems error like the following:

Fetching gem metadata from https://rubygems.org/...
Resolving dependencies...
Bundler could not find compatible versions for gem "sorbet-static":
  In snapshot (Gemfile.lock):
    sorbet-static (>= 0.5.10160)

  In Gemfile:
    sorbet-static-and-runtime was resolved to 0.5.10160, which depends on
      sorbet (= 0.5.10160) was resolved to 0.5.10160, which depends on
        sorbet-static (= 0.5.10160)

Deleting your Gemfile.lock file and running `bundle install` will rebuild your snapshot from scratch, using only
the gems in your Gemfile, which may resolve the conflict.
Bundler could not find compatible versions for gem "sorbet-static-and-runtime":
  In snapshot (Gemfile.lock):
    sorbet-static-and-runtime (>= 0.5.10160)

  In Gemfile:
    sorbet-static-and-runtime

Deleting your Gemfile.lock file and running `bundle install` will rebuild your snapshot from scratch, using only
the gems in your Gemfile, which may resolve the conflict.

What is your fix for the problem, implemented in this PR?

My fix is to automatically remove the "ruby" platform from these "corrupted" lockfiles, so that they resolve properly.

Fixes #5743.

Make sure the following tasks are checked

@deivid-rodriguez deivid-rodriguez force-pushed the automatically-remove-ruby-if-not-resolvable branch from d546d3e to f1c7842 Compare August 2, 2022 18:11
@deivid-rodriguez deivid-rodriguez force-pushed the automatically-remove-ruby-if-not-resolvable branch from f1c7842 to 69d0b4e Compare August 3, 2022 16:55
@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review August 3, 2022 18:36
jurre added a commit to dependabot/dependabot-core that referenced this pull request Aug 4, 2022
The latest versions of bundler has an issue where incomplete lockfiles that are
locked to "ruby" but not including exclusively gems NOT platform specific.

This causes an issue with existing lockfiles, generating a missing gem
error.

This is explained in more detail in: rubygems/rubygems#5807

Since this is currently preventing Dependabot updates for our users,
especially around sorbet dependencies, and since we have no good way to
signal these failures to them with an explanation on how to resolve it,
I think it's best that we downgrade bundler to a version that does not
include this, until rubygems/rubygems#5807 is
merged and we can bump to the latest version again.
jurre added a commit to dependabot/dependabot-core that referenced this pull request Aug 4, 2022
The latest versions of bundler has an issue where incomplete lockfiles that are
locked to "ruby" but not including exclusively gems NOT platform specific.

This causes an issue with existing lockfiles, generating a missing gem
error.

This is explained in more detail in: rubygems/rubygems#5807

Since this is currently preventing Dependabot updates for our users,
especially around sorbet dependencies, and since we have no good way to
signal these failures to them with an explanation on how to resolve it,
I think it's best that we downgrade bundler to a version that does not
include this, until rubygems/rubygems#5807 is
merged and we can bump to the latest version again.
@deivid-rodriguez deivid-rodriguez merged commit 839441c into master Aug 5, 2022
@deivid-rodriguez deivid-rodriguez deleted the automatically-remove-ruby-if-not-resolvable branch August 5, 2022 07:36
deivid-rodriguez added a commit that referenced this pull request Aug 10, 2022
…ot-resolvable

Automatically remove "ruby" from lockfile if incomplete

(cherry picked from commit 839441c)
@casperisfine
Copy link

My fix is to automatically remove the "ruby" platform from these "corrupted" lockfiles, so that they resolve properly.

Apologies for jumping on an old PR like this, but this new behavior is causing us lots of problems.

Context:

  • We have sorbet-static in our Gemfile.
  • We force the ruby platform in our Gemfile because we run our CI nightly against ruby-head, so we need to be able to install from source in that case.

We recently updated bundler, and now it keeps trying to remove the ruby platform. I understand that it wasn't quite working before, but it seems harsh to me to silently remove a platform like this. I'd totally understand bundle exiting with an error given that sorbet-static has no source version, but I feel a bit "betrayed" by it doing something like this behind my back.

@deivid-rodriguez
Copy link
Member Author

Hi!

Sorry, I just wanted to make things easier for users, because a Gemfile/Gemfile.lock files that were previously working, would suddenly would no longer work, and that seemed pretty bad.

Bundler does this kind of sanitization in other places, for example if it finds redundant lockfile dependencies, or redundant platform specific gems in there, so this didn't seem bad.

@Deterville
Copy link

Bundles stacks record locking reset Thanks for the recovery deivid=rodriguez

@casperisfine
Copy link

@deivid-rodriguez please don't apologize :)

I dropped this message a bit quickly without much actionable content because I had to run, but I'd like to try to find a way forward.

a Gemfile/Gemfile.lock files that were previously working, would suddenly would no longer work, and that seemed pretty bad.

What is interesting is that our current Gemfile/Gemfile.lock work perfectly on install, but whenever we update we lose the ruby platform, so it's effectively broken for us.

One of the root cause is that sorbet-static is somewhat abusing the Gem platform attribute to ship static binaries that don't link to Ruby. It's clever, but cause the problem described above.

I searched a lot, but I couldn't find a proper workaround for this aside from manually editing the Gemfile.lock or downgrading bundler. Would you by any chance have some pointers to some potential features I could use to workaround this issue?

I wish I could scope the sorbet-static gem to tell bundler it's fine if it's not installed, but I don't see how to do that.

@deivid-rodriguez
Copy link
Member Author

I don't understand the issue very well. What is it broken without the ruby platform, which error do you get?

@casperisfine
Copy link

which error do you get?

On 3.1.2 it works fine. However when trying to bundle that same Gemfile/Gemfile.lock on 3.2.0-dev with bundler 2.3.23, the bundle install get stuck on dependency resolution (our CI job timeout after 1h):

Resolving dependencies...............................................................................................................................................................................................
--
  | Resolving dependencies................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

But before we upgraded bundler, it would simply fail quickly complaining that some of our native gems require ruby < 3.2.0-dev, which is expected.

I know you've told me in the past that you don't support using the same Gemfile.lock for distinct Ruby version, but that's really key to being able to test ruby nightly and report bugs upstream, which we do routinely and is really important for ruby-core.

I hope this description makes sense?

I can try to generate a small repro, I think just nokogiri + sorbet in the same Gemfile should allow to repro.

@casperisfine
Copy link

source "https://rubygems.org"

gem "sorbet"
gem "nokogiri"
GEM
  remote: https://rubygems.org/
  specs:
    mini_portile2 (2.8.0)
    nokogiri (1.13.9)
      mini_portile2 (~> 2.8.0)
      racc (~> 1.4)
    nokogiri (1.13.9-arm64-darwin)
      racc (~> 1.4)
    racc (1.6.0)
    sorbet (0.5.10501)
      sorbet-static (= 0.5.10501)
    sorbet-static (0.5.10501-universal-darwin-14)
    sorbet-static (0.5.10501-universal-darwin-15)
    sorbet-static (0.5.10501-universal-darwin-16)
    sorbet-static (0.5.10501-universal-darwin-17)
    sorbet-static (0.5.10501-universal-darwin-18)
    sorbet-static (0.5.10501-universal-darwin-19)
    sorbet-static (0.5.10501-universal-darwin-20)
    sorbet-static (0.5.10501-universal-darwin-21)
    sorbet-static (0.5.10501-universal-darwin-22)
    sorbet-static (0.5.10501-x86_64-linux)

PLATFORMS
  arm64-darwin-21
  ruby

DEPENDENCIES
  nokogiri
  sorbet

BUNDLED WITH
   2.3.23

So the pair above allows you to bundle install successfully with both Ruby 3.1.2 and 3.2.0-dev.

But if you run bundle lock --update, the ruby platform is removed and you can't bundle install on 3.2.0-dev anymore.

@deivid-rodriguez
Copy link
Member Author

Ok, I think it should work without Ruby. I will have a look at what's going on.

@casperisfine
Copy link

Hum, actually it works on my machine with that minimal example, there might be more to it in our big gemfile that cause bundler to spend over an hour resolving dependencies.

I'll try to run it in debug mode see if there's more to it.

@deivid-rodriguez
Copy link
Member Author

Also 2.3.24 actually fixed some "bundler resolving forever" issues. Maybe you can try that too.

@casperisfine
Copy link

Oh! Missed that there was a 2.3.24, trying that now.

As for the debug build, no real smoking guns in the debug output I'm afraid, only:

Running `bundle install --jobs 4 --path "/tmp/bundle" --retry 3` with bundler 2.3.23
Found changes from the lockfile, re-resolving dependencies because you added a new platform to your gemfile

But that's essentially what I was describing.

@casperisfine
Copy link

Seems like 2.3.24 does indeed fix my issue. Thank you @deivid-rodriguez !

@casperisfine
Copy link

Hum, I'll dig more tomorrow, but while this solved it on "CI", it fails when we build our production images. The only difference is that we pass --without development:test, which somehow seems to trip bundler:

Your bundle only supports platforms ["arm64-darwin", "arm64-darwin-21",
"x86_64-linux"] but your local platform is ruby. Add the current platform to the
lockfile with
`bundle lock --add-platform ruby` and try again.

And I can't add ruby platform back because of sorbet-static 😫

@deivid-rodriguez
Copy link
Member Author

deivid-rodriguez commented Oct 20, 2022

That sounds like you're using force_ruby_platform, is that intended?

@casperisfine
Copy link

It is (or was) yes. That's how we managed to keep bundling on ruby-head a few months back, so now we have BUNDLE_FORCE_RUBY_PLATFORM=true automatically set for ruby-head.

We might no need it anymore with more recent bundlers? I'll try removing it.

@casperisfine
Copy link

Yeah, I think we don't need it now that #5715 was fixed.

@deivid-rodriguez
Copy link
Member Author

deivid-rodriguez commented Oct 20, 2022

Yes, force_ruby_platform does not play well with sorbet-static, so hopefully you can remove that 🤞.

@casperisfine
Copy link

Hum, no dice I'm afraid, if I remove the force ruby platform, I can't bundle anymore because I only have native versions of nokogiri in my gemfile:



nokogiri-1.13.8-x86_64-linux requires ruby version < 3.2.dev, >= 2.6, which is
--
  | incompatible with the current version, 3.2.0.dev

@flavorjones
Copy link
Contributor

flavorjones commented Oct 20, 2022

I believe we discovered in an earlier issue (#4012) that a Gemfile.lock generated by one version of Ruby is not guaranteed to work with another version of Ruby. So if you generate with Ruby 3.1 it's not guaranteed to work with Ruby 3.2. Maybe @deivid-rodriguez can correct me if I'm misunderstanding the intended behavior.

@deivid-rodriguez
Copy link
Member Author

That's right, although if you do it the other way around it might work: generate it with Ruby 3.2, so that the version of nokogiri that's not platform specific is picked up. Alternatively, you can use gem "nokogiri", force_ruby_platform: true to explicitly force it.

@casperisfine
Copy link

I believe we discovered in an earlier issue (#4012) that a Gemfile.lock generated by one version of Ruby is not guaranteed to work with another version of Ruby.

Yes I remember. But until now we've always managed to find work arounds, and this "non-feature" is extremely valuable for us, so I'm really determined to find a way to make it work, even if that mean I have to monkey patch bundler.

if you do it the other way around it might work: generate it with Ruby 3.2

Unfortunately that's not an option. We have hundreds of dev that may potentially run bundle update, plus dependabot, we can't get them all to use ruby-head.

gem "nokogiri", force_ruby_platform: true

Thanks, I'll try to hack around with this.

But ultimately I think Bundler could behave differently here. If it wasn't for sorbet-static preventing us to have a ruby platform we'd be fine. But I wonder why ruby is even a platform? Why isn't it implictly always present?

@deivid-rodriguez
Copy link
Member Author

Unfortunately that's not an option. We have hundreds of dev that may potentially run bundle update, plus dependabot, we can't get them all to use ruby-head.

Yeah, I didn't really mean that as a serious suggestion, just wanted to point it out to show the currently brittleness of this.

Thanks, I'll try to hack around with this.

Yeah, I really think the force_ruby_platform per-gem option would really fix your problem, because the global FORCE_RUBY_PLATFORM cannot, since sorbet-static cannot be forced into a non platform specific variant. So what you want is to force the not platform specific variants, just for the specific gems that are resolving for different platforms depending on whether you are on Ruby 3.1 or 3.2.

But ultimately I think Bundler could behave differently here. If it wasn't for sorbet-static preventing us to have a ruby platform we'd be fine. But I wonder why ruby is even a platform? Why isn't it implictly always present?

I guess the answer of the question "why ruby is a platform" is... "legacy stuff", "historical decisions that most likely made total sense at the time", and those kind of answers. With all the recent changes though, "ruby" as a platform should be getting less and less used and be gradually removed from lock files, because it's not really necessary most of the times.

@casperisfine
Copy link

I really think the force_ruby_platform per-gem option would really fix your problem

So, it fix the problem I have, but has the very negative effect of forcing all our users to compile these huge gems from source when they could use the precompiled version.

With all the recent changes though, "ruby" as a platform should be getting less and less used and be gradually removed from lock files, because it's not really necessary most of the times.

Right, PLATFORM ruby isn't what I want it's just a proxy.

Most precompiled gems have ruby < 3.2.dev as a requirement, so by asking for platform=ruby I'm basically telling bundler: please also provide source version of precompiled gems because I know you'll need them.

But ultimately, what I really want to say is: "Include all the versions you need so that it is compatible with both 3.1.2 and 3.2.0.dev", so in a way I guess what I'm asking for is:

# Gemfile
ruby "3.1.2", "3.2.0dev"

Is this something you think would be doable? As said previously I know multi-ruby isn't something Bundler want to support, but I respectfully think it is a mistake because it prevent people like us from testing ruby-head and report bugs in advance of releases.

@casperisfine
Copy link

Ok, I think I may have found a dirty workaround:

# if we're on 3.2.0dev
bundle lock --add-plaform=dummy
bundle lock --remove-plaform=dummy
# endif
bundle install .....

@deivid-rodriguez
Copy link
Member Author

So, it fix the problem I have, but has the very negative effect of forcing all our users to compile these huge gems from source when they could use the precompiled version.

Right, while before you would set BUNDLE_FORCE_RUBY_PLATFORM in your 3.2.0-dev environment and not change Gemfiles otherwise.

Is this something you think would be doable? As said previously I know multi-ruby isn't something Bundler want to support, but I respectfully think it is a mistake because it prevent people like us from testing ruby-head and report bugs in advance of releases.

I personally didn't say I don't want to support something like that (or if I said that, I don't have such an opinion anymore). I just said it was never a goal and I'm not surprised it has issues.

I think the problem with ruby "3.1.2", "3.2.0.dev" would be that Bundler as it works now would still prefer a version that works everywhere, so it would go with using the not platform specific version of nokogiri. I will think about your use case, and see if I come up with something.

@casperisfine
Copy link

I will think about your use case, and see if I come up with something.

❤️

@ianks
Copy link
Collaborator

ianks commented Oct 21, 2022

Stumbled up this issue as well, and wrote a spec of what I think the expected behavior should be in an ideal world: 3c29a15

  context "when a gem has a source gem and a precompiled gem" do
    context "when another platform-only gem is present" do
      context "the Ruby version is not locked" do
        it "does not remove the source gem nor RUBY platform from lockfile" do
          build_repo2 do
            # Source gem with no Ruby version requirement
            build_gem("my-precompiled-gem", "3.0.0")

            # Optional precompiled gem for the current platform, but wont work with
            # future ruby versions
            build_gem("my-precompiled-gem", "3.0.0") do |s|
              s.platform = Bundler.local_platform # e.g. x86_64-linux
              s.required_ruby_version = "< #{future_ruby_minor_version}"
            end

            # Platform-only
            build_gem("no-ruby-platform-gem", "1.0.0") do |s|
              s.platform = Bundler.local_platform # e.g. x86_64-linux
            end
          end

          source = file_uri_for(gem_repo2)

          # IMPORTANT: We cannot rule out the possibility that gems will be
          # bundled on future ruby version.
          gemfile <<~G
            source "#{source}"
            raise "This should not be loaded" unless RUBY_VERSION < "#{future_ruby_minor_version}"
            gem "my-precompiled-gem"
            gem "no-ruby-platform-gem"
          G

          bundle :install

          # If the Ruby version is not locked, we shouldn't disqualify the
          # source gem which will work on a more recent Ruby version.
          expect(lockfile).to eq <<~L
            GEM
              remote: #{source}/
              specs:
                my-precompiled-gem (3.0.0)
                my-precompiled-gem (3.0.0-#{Bundler.local_platform})
                no-ruby-platform-gem (1.0.0-#{Bundler.local_platform})
            PLATFORMS
              ruby
              #{Bundler.local_platform}
            DEPENDENCIES
              my-precompiled-gem
              no-ruby-platform-gem
            BUNDLED WITH
              #{Bundler::VERSION}
          L
        end
      end
    end
  end

The key point being: Bundler shouldn’t remove the source gem because it’s feasible that future_ruby_version would bundle this gemfile and be left without a compatible my-precompiled-gem. In essence, Bundler is assuming that the Ruby version the user invoked bundle install with should be implicitly assumed for all future invocations. It’s assuming that the Gemfile.lock can be locked to the current Ruby version.

Now, if the above example had specified ruby “x.x.x”, that changes the expectation. In this case, I think removing the source gem would be correct since there’s no possible way it can be used.

I think the locking logic needs to take into account whether or not the user specified a ruby “…” in the Gemfile.

@ianks
Copy link
Collaborator

ianks commented Oct 21, 2022

Yeah, I really think the force_ruby_platform per-gem option would really fix your problem, because the global FORCE_RUBY_PLATFORM cannot

This is a sledgehammer for a nail. We don’t want to force bundler to compile a gem when it doesnt need to. In many cases, we know that there are binaries for one Ruby version, but not another. It’s best to use the binary gem when possible and only fallback to compilation as an absolute last resort.

The force_ruby_platform option, whether globally or locally, should be an absolute last resort — an escape hatch.

Bundler as it works now would still prefer a version that works everywhere

I think this is the crux of the issue. Ideally, bundler could favor versions that have higher constraint specificity, but that seems bug-prone to implement and likely backwards incompatible. Another way would be to add an optional spec.metadata[:resolution_priority] so bundler could <=> the multiple matching versions. That would allow gem maintainers to specify exactly what bundler should do when multiple matches are found.

@deivid-rodriguez
Copy link
Member Author

My naive idea to approach this, not sure how it will work in practice, is to handle RUBY VERSION in the lockfile similarly to PLATFORMS. If you have a ruby constraint in your Gemfile (which would be compulsory for this optional feature), then a RUBY VERSION section is already added to the lockfile. The idea would be to allow keeping a list of rubies in there, not just one, and keep the necessary variants of each gem that are optimal for those recorded rubies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Platform specific gems not being found by bundler
5 participants