From 40864c9b25033602be96eab87053bb486e28e46d Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Mon, 30 Oct 2023 12:48:50 -0700 Subject: [PATCH 1/2] fix: add back bin/node-gyp-bin/node-gyp files This is a continuation of #6932. This makes a few more changes identified in also porting these changes to v10. --- .gitattributes | 2 +- bin/node-gyp-bin/node-gyp.cmd | 8 ++++---- test/bin/windows-shims.js | 28 ++++++++++++++++++++++------ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/.gitattributes b/.gitattributes index 5bfd3199c0181..5d3dbc3b3ac65 100644 --- a/.gitattributes +++ b/.gitattributes @@ -7,7 +7,7 @@ /configure text eol=lf # our cmd scripts always need to be CRLF -/bin/*.cmd text eol=crlf +/bin/**/*.cmd text eol=crlf # ignore all line endings in node_modules since we dont control that /node_modules/** -text diff --git a/bin/node-gyp-bin/node-gyp.cmd b/bin/node-gyp-bin/node-gyp.cmd index 1ef2ae0c68fc4..083c9c58a502a 100755 --- a/bin/node-gyp-bin/node-gyp.cmd +++ b/bin/node-gyp-bin/node-gyp.cmd @@ -1,5 +1,5 @@ -if not defined npm_config_node_gyp ( - node "%~dp0\..\..\node_modules\node-gyp\bin\node-gyp.js" %* -) else ( +if not defined npm_config_node_gyp ( + node "%~dp0\..\..\node_modules\node-gyp\bin\node-gyp.js" %* +) else ( node "%npm_config_node_gyp%" %* -) +) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 8769f2a905458..9ee8d9a0854c6 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -1,22 +1,23 @@ const t = require('tap') const { spawnSync } = require('child_process') const { resolve, join, extname, basename, sep } = require('path') -const { readFileSync, chmodSync, readdirSync, statSync } = require('fs') +const { readFileSync, chmodSync, readdirSync, rmSync, statSync } = require('fs') const Diff = require('diff') const { sync: which } = require('which') const { version } = require('../../package.json') -const ROOT = resolve(__dirname, '../..') -const BIN = join(ROOT, 'bin') -const NODE = readFileSync(process.execPath) -const SHIMS = readdirSync(BIN).reduce((acc, shim) => { - const p = join(BIN, shim) +const readNonJsFiles = (dir) => readdirSync(dir).reduce((acc, shim) => { + const p = join(dir, shim) if (extname(p) !== '.js' && !statSync(p).isDirectory()) { acc[shim] = readFileSync(p, 'utf-8') } return acc }, {}) +const ROOT = resolve(__dirname, '../..') +const BIN = join(ROOT, 'bin') +const SHIMS = readNonJsFiles(BIN) +const NODE_GYP = readNonJsFiles(join(BIN, 'node-gyp-bin')) const SHIM_EXTS = [...new Set(Object.keys(SHIMS).map(p => extname(p)))] // windows requires each segment of a command path to be quoted when using shell: true @@ -65,6 +66,21 @@ t.test('shim contents', t => { }) }) +t.test('node-gyp', t => { + // these files need to exist to avoid breaking yarn 1.x + + for (const [key, file] of Object.entries(NODE_GYP)) { + t.match(file, /npm_config_node_gyp/, `${key} contains env var`) + t.match( + file, + /[\\/]\.\.[\\/]\.\.[\\/]node_modules[\\/]node-gyp[\\/]bin[\\/]node-gyp\.js/, + `${key} contains path` + ) + } + + t.end() +}) + t.test('run shims', t => { const path = t.testdir({ ...SHIMS, From e33edad735c59b6a8c9382357cf9bf194c322a2a Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Mon, 30 Oct 2023 12:57:16 -0700 Subject: [PATCH 2/2] chore: add test flake improvements to windows shim tests --- test/bin/windows-shims.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 9ee8d9a0854c6..5fa6ff142b737 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -1,7 +1,7 @@ const t = require('tap') const { spawnSync } = require('child_process') const { resolve, join, extname, basename, sep } = require('path') -const { readFileSync, chmodSync, readdirSync, rmSync, statSync } = require('fs') +const { copyFileSync, readFileSync, chmodSync, readdirSync, rmSync, statSync } = require('fs') const Diff = require('diff') const { sync: which } = require('which') const { version } = require('../../package.json') @@ -84,7 +84,6 @@ t.test('node-gyp', t => { t.test('run shims', t => { const path = t.testdir({ ...SHIMS, - 'node.exe': NODE, // simulate the state where one version of npm is installed // with node, but we should load the globally installed one 'global-prefix': { @@ -109,6 +108,16 @@ t.test('run shims', t => { }, }) + // hacky fix to decrease flakes of this test from `NOTEMPTY: directory not empty, rmdir` + // this should get better in tap@18 and we can try removing it then + copyFileSync(process.execPath, join(path, 'node.exe')) + t.teardown(async () => { + rmSync(join(path, 'node.exe')) + await new Promise(res => setTimeout(res, 100)) + // this is superstition + rmSync(join(path, 'node.exe'), { force: true }) + }) + const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { if (cmd.endsWith('bash.exe')) { // only cygwin *requires* the -l, but the others are ok with it