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

Improved CI package manager detection #1313

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

colincasey
Copy link
Contributor

The previous method for detecting package managers works well when yarn, pnpm, or npm binaries are installed in the traditional way but fails when corepack shims are present. This PR changes the detection logic to use package metadata when determining which package manager to use.

Fixes #1312

The previous method for detecting package managers works well when `yarn`, `pnpm`, or `npm` binaries are installed in the traditional way but fails when `corepack` shims are present. This PR changes the detection logic to use package metadata when determining which package manager to use.
@colincasey colincasey self-assigned this Aug 29, 2024
@colincasey colincasey requested a review from a team as a code owner August 29, 2024 15:46
The previous method for detecting package managers works well when `yarn`, `pnpm`, or `npm` binaries are installed in the traditional way but fails when `corepack` shims are present. This PR changes the detection logic to use package metadata when determining which package manager to use.
bin/test Show resolved Hide resolved
Copy link
Member

@joshwlewis joshwlewis left a comment

Choose a reason for hiding this comment

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

Looks like this will fix the issue, so leaving a 👍🏻. I think we probably could be more robust here by detecting conflicts, but I think we've already got that handled in bin/compile which will absolutely run before bin/test anyway.

colincasey added a commit that referenced this pull request Sep 4, 2024
Along with lockfile detection, we also detect what intended package manager (and version) should be used based on metadata found in package.json. When multiple package managers are  declared we should fail the build early and ask the user to correct any conflicts found.

Related to #1313
colincasey added a commit that referenced this pull request Sep 4, 2024
Along with lockfile detection, we also detect what intended package manager (and version) should be used based on metadata found in package.json. When multiple package managers are  declared we should fail the build early and ask the user to correct any conflicts found.

Related to #1313
@colincasey colincasey enabled auto-merge (squash) September 4, 2024 14:42
@colincasey colincasey merged commit c40573d into main Sep 4, 2024
11 checks passed
@colincasey colincasey deleted the fix_ci_package_manager_detection branch September 4, 2024 15:08
@heroku-linguist heroku-linguist bot mentioned this pull request Sep 5, 2024
colincasey added a commit that referenced this pull request Sep 11, 2024
* Fail on conflicting package manager metadata in package.json

Along with lockfile detection, we also detect what intended package manager (and version) should be used based on metadata found in package.json. When multiple package managers are  declared we should fail the build early and ask the user to correct any conflicts found.

Related to #1313

* Fail on conflicting package manager metadata in package.json

Along with lockfile detection, we also detect what intended package manager (and version) should be used based on metadata found in package.json. When multiple package managers are  declared we should fail the build early and ask the user to correct any conflicts found.

Related to #1313
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.

Recent Buildpack change incorrectly uses pnpm despite Yarn configuration
2 participants