From 99a2b5f3e59fa5e3fc1f7ca19060106d6b06b595 Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Wed, 4 Sep 2024 11:35:52 -0300 Subject: [PATCH 1/2] 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 https://github.com/heroku/heroku-buildpack-nodejs/pull/1313 --- bin/compile | 1 + lib/failure.sh | 49 +++++++++++++++++++ .../package.json | 11 +++++ test/run | 10 ++++ 4 files changed, 71 insertions(+) create mode 100644 test/fixtures/conflicting-package-manager-metadata/package.json diff --git a/bin/compile b/bin/compile index cfdd74471..d0aa51ca6 100755 --- a/bin/compile +++ b/bin/compile @@ -118,6 +118,7 @@ fail_dot_heroku "$BUILD_DIR" fail_dot_heroku_node "$BUILD_DIR" fail_invalid_package_json "$BUILD_DIR" fail_multiple_lockfiles "$BUILD_DIR" +fail_conflicting_package_manager_metadata "$BUILD_DIR" fail_iojs_unsupported "$BUILD_DIR" warn_prebuilt_modules "$BUILD_DIR" warn_missing_package_json "$BUILD_DIR" diff --git a/lib/failure.sh b/lib/failure.sh index f9f933745..3cb80cacc 100644 --- a/lib/failure.sh +++ b/lib/failure.sh @@ -1007,3 +1007,52 @@ warn_about_node_version_22_5_0() { " "https://github.com/nodejs/node/pull/53934" mcount 'warnings.node.22-5-0' } + +fail_conflicting_package_manager_metadata() { + # track the package managers we see in an associative array + declare -A package_managers + declare -a fields_detected + + npm_engine=$(read_json "$BUILD_DIR/package.json" ".engines.npm") + yarn_engine=$(read_json "$BUILD_DIR/package.json" ".engines.yarn") + pnpm_engine=$(read_json "$BUILD_DIR/package.json" ".engines.pnpm") + package_manager=$(read_json "$BUILD_DIR/package.json" ".packageManager") + + if [ -n "$npm_engine" ]; then + package_managers["npm"]=0 + fields_detected+=("- npm version detected in engines.npm ($npm_engine)") + fi + + if [ -n "$yarn_engine" ]; then + package_managers["yarn"]=0 + fields_detected+=("- yarn version declared in engines.yarn ($yarn_engine)") + fi + + if [ -n "$pnpm_engine" ]; then + package_managers["pnpm"]=0 + fields_detected+=("- pnpm version declared in engines.pnpm ($pnpm_engine)") + fi + + if [[ "$package_manager" == yarn* ]]; then + package_managers["yarn"]=0 + fields_detected+=("- yarn version declared in packageManager ($package_manager)") + elif [[ "$package_manager" == pnpm* ]]; then + package_managers["pnpm"]=0 + fields_detected+=("- pnpm version declared in packageManager ($package_manager)") + fi + + # was there more than one package manager found? + if (( "${#package_managers[@]}" > 1 )); then + mcount "failures.multiple-package-managers" + meta_set "failure" "multiple-package-managers" + header "Build failed" + warn "Multiple package managers declared in package.json + + Installing dependencies using the wrong package manager can result in missing packages or subtle bugs + in production. Only one of the following fields should be used, all others should be removed: + +$(for item in "${fields_detected[@]}"; do echo " $item"; done) + " + fail + fi +} diff --git a/test/fixtures/conflicting-package-manager-metadata/package.json b/test/fixtures/conflicting-package-manager-metadata/package.json new file mode 100644 index 000000000..3c83bd92a --- /dev/null +++ b/test/fixtures/conflicting-package-manager-metadata/package.json @@ -0,0 +1,11 @@ +{ + "name": "conflicting-package-manager-metadata", + "version": "1.0.0", + "license": "ISC", + "packageManager": "pnpm@9.9.0", + "engines": { + "npm": "10.x", + "yarn": "4.x", + "pnpm": "9.x" + } +} diff --git a/test/run b/test/run index c72de1431..d2f072ff4 100755 --- a/test/run +++ b/test/run @@ -2018,6 +2018,16 @@ testWarningNode_22_5_0() { assertCapturedSuccess } +testConflictingPackageManagerMetadata() { + compile "conflicting-package-manager-metadata" + assertCaptured "Multiple package managers declared in package.json" + assertCaptured "- npm version detected in engines.npm (10.x)" + assertCaptured "- yarn version declared in engines.yarn (4.x)" + assertCaptured "- pnpm version declared in engines.pnpm (9.x)" + assertCaptured "- pnpm version declared in packageManager (pnpm@9.9.0)" + assertCapturedError +} + # Utils pushd "$(dirname 0)" >/dev/null From 13a99b61c1073e2855798b00ea1056dde3331f0a Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Wed, 4 Sep 2024 11:37:40 -0300 Subject: [PATCH 2/2] 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 https://github.com/heroku/heroku-buildpack-nodejs/pull/1313 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2b95e626..7227fdb13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] - Added Node.js version 22.8.0. +- Fail on conflicting package manager metadata in package.json. ([#1317](https://github.com/heroku/heroku-buildpack-nodejs/pull/1317)) ## [v263] - 2024-08-27