Use BUNDLE_LOCKFILE when detecting the lockfile#919
Conversation
Honor `BUNDLE_LOCKFILE` (added in Bundler 4) when selecting the lockfile used for Bundler version detection, deployment mode, and bundler-cache keys. Without this, workflows using an alternate lockfile can read the wrong `BUNDLED WITH` version and generate cache keys from the wrong lockfile.
Thomascountz
force-pushed
the
thomascountz/support-bundle-lockfile
branch
from
ae3fce3 to
a34034a
Compare
|
|
||
| if (fs.existsSync(gemfilePath)) { | ||
| return [gemfilePath, `${gemfilePath}.lock`] | ||
| return [gemfilePath, lockfilePath || `${gemfilePath}.lock`] |
There was a problem hiding this comment.
Given that we have a fs.existsSync check for BUNDLE_GEMFILE or its default value Gemfile that would thrown an exception if missing, we should probably have fs.existsSync check for BUNDLE_LOCKFILE if it's explicitly set. In this case, unlike the check for gemfilePath, we should not check existence of Gemfile.lock because it's optional.
There was a problem hiding this comment.
I'm not following, the code is fine as-is, no? Because the lockfile is always optional.
There was a problem hiding this comment.
If you explicit define an environment variable for it, most likely it means you want to explicitly use that file. In that case if the file specified in the variable does not exist and we treat it as optional, wouldn’t that be confusing?
There was a problem hiding this comment.
Since it's a variable recognized by Bundler people might use it independently of setup-ruby so I think it's fine and not something we should check.
But, that makes me think probably this needs to be part of the cache key.
There was a problem hiding this comment.
lockFile is already in computeBaseKey, so all good
3 participants
If a workflow sets
BUNDLE_LOCKFILE, setup-ruby still assumes${BUNDLE_GEMFILE}.lockorgems.locked. That means it can read the wrongBUNDLED WITHversion, fail to enable deployment mode because it checks the wrong lockfile path, and/or generate a cache key from the wrong lockfile.Bundler added
BUNDLE_LOCKFILEsupport in PR #9059