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

Incompatible gems in optional groups is more difficult since Bundler 2.4.21 #7169

Open
Bo98 opened this issue Nov 17, 2023 · 4 comments
Open
Labels

Comments

@Bo98
Copy link
Contributor

Bo98 commented Nov 17, 2023

Describe the problem as clearly as you can

I have a Gemfile that looks like this: https://github.com/Homebrew/brew/blob/master/Library/Homebrew/Gemfile. Ignore the Dependabot-specific hack at the top. The lockfile is here: https://github.com/Homebrew/brew/blob/master/Library/Homebrew/Gemfile.lock.

The basic scenario is I have:

group :typecheck, optional: true do
  gem "sorbet-static-and-runtime", require: false
end

and in the lockfile:

...
    sorbet-static (0.5.10461-universal-darwin-14)
    sorbet-static (0.5.10461-universal-darwin-15)
    sorbet-static (0.5.10461-universal-darwin-16)
    sorbet-static (0.5.10461-universal-darwin-17)
    sorbet-static (0.5.10461-universal-darwin-18)
    sorbet-static (0.5.10461-universal-darwin-19)
    sorbet-static (0.5.10461-universal-darwin-20)
    sorbet-static (0.5.10461-universal-darwin-21)
    sorbet-static (0.5.10461-universal-darwin-22)
    sorbet-static (0.5.10461-x86_64-linux)
...

PLATFORMS
  aarch64-linux
  arm-linux
  arm64-darwin
  x86_64-darwin
  x86_64-linux

This is perhaps unconventional as technically speaking aarch64-linux and arm-linux are invalid for sorbet-static as there is no compatible gem. And indeed if you are on those platforms and try to run bundle update/bundle lock --update or try to install the typecheck group, you will indeed get resolver issues.

However, sorbet-static is in an optional group and bundle install without the group works perfectly fine on arm64 Linux. For us the group of people modifying and updating the Gemfile and the people installing from the Gemfile are two distinct groups. The latter group doesn't even need to know about the concept of a group: we pass the relevant groups as necessary. Notably, it is acceptable for us to be unable to maintain the Gemfile on arm64 Linux but still be able to install from it.

This all however worked ok until today we saw Dependabot open a PR that started removing one of the bad platforms: https://github.com/Homebrew/brew/pull/16225/files (I'll send a PR that fixes it not removing both platforms). This was because Dependabot now uses Bundler 2.4.22 and it seems 2.4.21 changed the behaviour to start force removing invalid platforms (#7035).

The behaviour change makes sense, but for specifically the use case of optional groups it doesn't make sense to me why the user should be blocked from installing everything entirely (since Bundler force removing the platform will mean the next bundle install will block unrecognised platforms) because of one optional group they are not requesting. It is also not particularly different than say the Darwin 23 scenario, which is also not technically valid with the above lockfile in a similar way, but Bundler doesn't currently force x86_64-darwin to become x86_64-darwin-14 through 22.

Any thoughts on perhaps not checking gems that are in optional groups (and maybe install_if blocks)? Or otherwise other paths to achieve the previous behaviour?

(I recognise it's a complicated issue. Longer term maybe it would make sense for new DSL similar to https://bundler.io/v2.4/man/gemfile.5.html#PLATFORMS to conditionally apply gems to certain platforms and for bundle lock --update to behave agnostically to the current running platform. That's for another issue however.)

Did you try upgrading rubygems & bundler?

The issue regressed in Bundler 2.4.21 and is present still in 2.4.22.

Post steps to reproduce the problem

Create a gemfile with sorbet-static in an optional group, resolve the lockfile and add bundle lock --add-platform aarch64-linux.
Note that it on 2.4.20 it installs correctly on arm64 Linux.
Then note that any non-frozen operation on the lockfile since 2.4.21 will remove the platform.

Which command did you run?

bundle lock --update (and whatever Dependabot uses)

What were you expecting to happen?

The lockfile is usable and left untouched when incompatible gems are kept in optional groups.

What actually happened?

The platforms are removed and the entire lockfile is now unusable on said platforms.

If not included with the output of your command, run bundle env and paste the output below

Bundler       2.4.18
  Platforms   ruby, x86_64-darwin-15
Ruby          3.1.4p223 (2023-03-30 revision 957bb7cb81995f26c671afce0ee50a5c660e540e) [x86_64-darwin-15]
  Full Path   /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/3.1.4/bin/ruby
  Config Dir  /usr/local/Cellar/portable-ruby/3.1.4/etc
RubyGems      3.3.26
  Gem Home    /usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0
  Gem Path    /usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0
  User Home   /Users/bo
  User Path   /Users/bo/.gem/ruby/3.1.0
  Bin Dir     /usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/bin
Tools         
  Git         2.37.1 (Apple Git-137.1)
  RVM         not installed
  rbenv       not installed
  chruby      not installed
Built At          2023-08-02
Git SHA           d2e3d8e3f4
Released Version  true
bin
  Set for your local app (/usr/local/Homebrew/Library/Homebrew/.bundle/config): false
clean
  Set for your local app (/usr/local/Homebrew/Library/Homebrew/.bundle/config): true
disable_shared_gems
  Set for your local app (/usr/local/Homebrew/Library/Homebrew/.bundle/config): true
force_ruby_platform
  Set for your local app (/usr/local/Homebrew/Library/Homebrew/.bundle/config): false
forget_cli_options
  Set for your local app (/usr/local/Homebrew/Library/Homebrew/.bundle/config): true
gem.ci
  Set for the current user (/Users/bo/.bundle/config): "github"
gem.linter
  Set for the current user (/Users/bo/.bundle/config): "rubocop"
gem.test
  Set for the current user (/Users/bo/.bundle/config): "rspec"
gemfile
  Set via BUNDLE_GEMFILE: "/usr/local/Homebrew/Library/Homebrew/Gemfile"
jobs
  Set for your local app (/usr/local/Homebrew/Library/Homebrew/.bundle/config): 4
path
  Set for your local app (/usr/local/Homebrew/Library/Homebrew/.bundle/config): "vendor/bundle"
retry
  Set for your local app (/usr/local/Homebrew/Library/Homebrew/.bundle/config): 3
@martinemde
Copy link
Member

Thanks for bringing this to our attention. I reviewed all the related Gemfiles and the PRs you linked. Thank you for summarizing it all.

Let me see if I understand the problem.

  1. Brew needs to be able to have certain gems that are only installed by maintainers.
  2. The Gemfile specifies these gems to make it easier for maintainers to install them.
  3. They are added in an optional group and the gems don’t support all of the intended “deployment” (user) platforms.
  4. The newest bundler is removing platforms because they don’t support the optional gem.
  5. We would like those locked platforms to stay while still being able to install maintainer gems on maintainer only platforms.

First the simple solution: Does bundle install on user machines use the frozen flag? If not, I think it would just re-add the platform implicitly. (This is not my preferred solution because it would probably re-resolve for the new platform and that's not ideal)

Another option that is top of mind for me is the PR I just pushed proposing a DSL for locked platforms (#7172). We imagined that a block form might be possible, but I didn’t have a clear use case until I saw this issue. At the very least, right now the feature would allow you to “force” your deployment platforms to stay in the lockfile. Thoughts? How would you imagine the block form working?

Lastly, I’ve seen some approaches to this that use eval_gemfile to make a separate Gemfile for gems that aren’t always installed. You could use one for maintainer gems and have them eval the main gemfile from the maintainer gemfile. I have no direct experience with this myself other than reading about it.

I’m glad it’s not completely broken right now, but only an annoyance from our good friend @dependabot.

@Bo98
Copy link
Contributor Author

Bo98 commented Nov 20, 2023

Let me see if I understand the problem.

Yeah you've summarised it correctly.

Does bundle install on user machines use the frozen flag?

Yes. We intentionally avoid making any git-committed file get marked as modified/dirty.

If not, I think it would just re-add the platform implicitly. (This is not my preferred solution because it would probably re-resolve for the new platform and that's not ideal)

The workaround we have now for Dependabot is we have a GitHub Actions workflow that runs when it finds a Dependabot-submitted PR and performs bundle lock --add-platform aarch64-linux arm-linux and pushes the result to the PR. Technically speaking I reckon bundle lock --add-platform probably shouldn't be allowing that since the next non-frozen operation to read the lockfile will immediately delete it, but it works so we're (ab)using it!

For any local updates, we already are pinning Bundler so every maintainer is running the same version and we're keeping that below 2.4.21 for now. Dependabot ignores any local pins, so the issue appeared first there.

Another option that is top of mind for me is the PR I just pushed proposing a DSL for locked platforms (#7172). We imagined that a block form might be possible, but I didn’t have a clear use case until I saw this issue. At the very least, right now the feature would allow you to “force” your deployment platforms to stay in the lockfile

If that's able to override or otherwise satisfy Bundler::Definition#remove_invalid_platforms!, then sure that will work.

I agree generally that platforms being lockfile-only information feels odd when you had to build that platform list manually (though I understand that may have improved recently). Not that this precise scenario matters to us: but in theory I reckon you should not need to do any lockfile tinkering beyond bundle update/bundle lock --update and be able to delete it and regenerate it from the Gemfile easily without worrying about lost details beyond the precise version locks. bundle lock --(add|remove)-platform would make sense to phase out in the long term.

I do also think that platform based conditionals on the Gemfile side could be improved. We've never strictly speaking needed install_if ourselves as we already apply the group selection ourselves anyway, but seeing how we have https://bundler.io/v2.4/man/gemfile.5.html#PLATFORMS then it does seem like there's room to grow there for platform based conditionals beyond the basics of MRI vs JRuby etc. A block form as you describe could be an answer to that. Or if it's any easier: a way to make locked platforms scoped to groups rather than only scoped to the entire Gemfile. In a way, the list of platforms chosen are somewhat arbitrary - if it wasn't for that one group we probably could be running just the ruby platform for everything else if we could.

To simplify this a bit: I searched around on rubygems.org, and a good example is this gem: https://rubygems.org/gems/linux-kstat. It's only built for Linux (if you ignore 0.1.0). How would one use this in a gemfile on Linux only without having to restrict the entire lockfile to *-linux? In theory we should be able to do something like:

gem "linux-kstat", "~> 0.2", platform: ["universal-linux"]
# or
group :kstat, platforms: ["universal-linux"] do
  gem "linux-kstat", "~> 0.2"
end
# or equivalent with `lock_platform` etc

Right now there's not really any good way to do this.

Lastly, I’ve seen some approaches to this that use eval_gemfile to make a separate Gemfile for gems that aren’t always installed. You could use one for maintainer gems and have them eval the main gemfile from the maintainer gemfile. I have no direct experience with this myself other than reading about it.

The complication comes a bit with shared dependencies between the two lockfiles and keeping those versions in sync. Dependabot AFAIK doesn't support syncing two lockfiles.

@hsbt
Copy link
Member

hsbt commented Nov 20, 2023

Is this same issue with #7159?

@Bo98
Copy link
Contributor Author

Bo98 commented Nov 20, 2023

#7159 seems to refer to a commit that's not on the Bundler 2.4.x branch, so no.

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

No branches or pull requests

3 participants