setup-ruby icon indicating copy to clipboard operation
setup-ruby copied to clipboard

Consider bundle groups in cache keys

Open artygus opened this issue 4 years ago • 3 comments

It's possible to add or skip certain gem groups with with: and without: config options (via corresponding BUNDLE_WITH and BUNDLE_WITHOUT env variables) which might set an incomplete cache. It's because currently cache key includes the whole Gemfile.lock contents. I wonder if it's a good idea to add values of aforementioned env variables to the cache key?

For example my current use case is two github actions: one for specs that requires all gems and another is linter action that only requires pronto specific stuff. Example Gemfile:

source "https://rubygems.org"

gem "rails", "~> 6.1"
# ... other gems ...

group :linter, optional: true do
  gem "pronto"
  gem "pronto-rubocop"
end

group :development, :linter do
  gem "rubocop", require: false
  gem "rubocop-rails", require: false
end

Action for linter run involves two env vars:

env:
  BUNDLE_WITH: linter
  BUNDLE_WITHOUT: default:development:test

If Gemfile.lock changes and linter action runs quicker it would set cache that only includes pronto gems, and specs action would install rails and bunch of other gems all over again on every run.

With a recent introduction of cache-version config in https://github.com/ruby/setup-ruby/pull/175 it's not such a big problem, I could just set a custom cache-version prefix to linter action though 🤔

If this proposal makes sense I'd be happy to submit a PR.

artygus avatar May 16 '21 10:05 artygus

Right, so the Gemfile.lock is the same no matter which with/without groups are specified.

One concern here is that it might be useful to install all gems in a first job, and then other jobs (with needs: [first_job]) might only install a subset and reusing the cache would then not only work it would also be more efficient.

Including their values in the key automatically also makes sense, is probably more intuitive, and would avoid having any extra gems in vendor/bundler, which might be unexpected when using without. So I think it makes sense and a PR is welcome.

eregon avatar May 16 '21 16:05 eregon

@artygus Are you still interested to make a PR for this?

eregon avatar Sep 25 '21 11:09 eregon

@eregon whoops! I totally forgot about this one, I'll try to come up with a PR next week. I'll keep you posted.

artygus avatar Sep 25 '21 13:09 artygus