Skip to content

Use BUNDLE_LOCKFILE when detecting the lockfile#919

Open
Thomascountz wants to merge 1 commit into
ruby:masterfrom
zendesk:thomascountz/support-bundle-lockfile
Open

Use BUNDLE_LOCKFILE when detecting the lockfile#919
Thomascountz wants to merge 1 commit into
ruby:masterfrom
zendesk:thomascountz/support-bundle-lockfile

Conversation

@Thomascountz
Copy link
Copy Markdown

@Thomascountz Thomascountz commented Jun 5, 2026

If a workflow sets BUNDLE_LOCKFILE, setup-ruby still assumes ${BUNDLE_GEMFILE}.lock or gems.locked. That means it can read the wrong BUNDLED WITH version, 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_LOCKFILE support in PR #9059

Copy link
Copy Markdown
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you

Comment thread README.md Outdated
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 Thomascountz force-pushed the thomascountz/support-bundle-lockfile branch from ae3fce3 to a34034a Compare June 8, 2026 07:16
@Thomascountz Thomascountz requested a review from eregon June 8, 2026 13:46
Comment thread bundler.js

if (fs.existsSync(gemfilePath)) {
return [gemfilePath, `${gemfilePath}.lock`]
return [gemfilePath, lockfilePath || `${gemfilePath}.lock`]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not following, the code is fine as-is, no? Because the lockfile is always optional.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants