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

Only use npm_package_json when we know it's correct #962

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@
"pnpm": "^8.7.1",
"prettier": "^3.0.3",
"selfsigned": "^2.0.1",
"ts-dedent": "^2.2.0",
"typescript": "^5.2.2",
"uvu": "^0.5.3",
"vscode-languageclient": "^9.0.1",
Expand Down
26 changes: 19 additions & 7 deletions src/cli-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,26 @@ import {QuietCiLogger, QuietLogger} from './logging/quiet-logger.js';
import {DefaultLogger} from './logging/default-logger.js';

export const packageDir = await (async (): Promise<string | undefined> => {
// Recent versions of npm set this environment variable that tells us the
// package.
const packageJsonPath = process.env.npm_package_json;
if (packageJsonPath) {
return pathlib.dirname(packageJsonPath);
// Recent versions of npm and yarn set the `npm_package_json` environment
// variable.
//
// `yarn` will only find `wireit` with the `-T` flag, which also sets
// `npm_package_json` to the package.json at the project root, even if we're
// in another package.
//
// Therefore, trust `npm_package_json` when in npm, but introspect the
// filesystem otherwise.

const agent = getNpmUserAgent();

if (agent === 'npm') {
const packageJsonPath = process.env.npm_package_json;
if (packageJsonPath) {
return pathlib.dirname(packageJsonPath);
}
}
// Older versions of npm, as well as yarn and pnpm, don't set this variable,
// so we have to find the nearest package.json by walking up the filesystem.

// Find the nearest package.json by walking up the filesystem.
let maybePackageDir = process.cwd();
while (true) {
try {
Expand Down
105 changes: 105 additions & 0 deletions src/test/basic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

import {suite} from 'uvu';
import * as assert from 'uvu/assert';
import {dedent} from 'ts-dedent';
import {rigTest} from './util/rig-test.js';
import {IS_WINDOWS} from '../util/windows.js';
import {injectYarnBerryToRig} from './util/yarn-berry.js';
import {NODE_MAJOR_VERSION} from './util/node-version.js';
import {checkScriptOutput} from './util/check-script-output.js';

Expand Down Expand Up @@ -880,6 +882,109 @@ test(
}),
);

test(
'commands can be run locally in yarn berry',
rigTest(async ({rig}) => {
await injectYarnBerryToRig(rig);

const rootCmd = await rig.newCommand();
const innerCmd = await rig.newCommand();

const originalLock = await rig.read('yarn.lock');

await rig.write({
'package.json': {
private: true,
workspaces: ['packages/*'],
scripts: {
cmd: 'yarn run -B wireit',
},
wireit: {
cmd: {
command: rootCmd.command,
},
},
devDependencies: {
wireit: '*',
},
resolutions: {
wireit: `portal:${process.cwd()}`,
},
},

'packages/inner/package.json': {
scripts: {
cmd: 'yarn run -TB wireit',
},
wireit: {
cmd: {
command: innerCmd.command,
},
},
},

// On a real system, `yarn` should do this automatically; however, in this
// test rig, a stale copy (`originalLock`) remains, even if we manually
// run `yarn`. Therefore, we manually update the lockfile here to include
// the inner package.
'yarn.lock':
originalLock +
dedent`

"inner-d81e84@workspace:packages/inner":
version: 0.0.0-use.local
resolution: "inner-d81e84@workspace:packages/inner"
languageName: unknown
linkType: soft

"wireit@portal:${process.cwd()}::locator=root-workspace-0b6124%40workspace%3A.":
version: 0.0.0-use.local
resolution: "wireit@portal:${process.cwd()}::locator=root-workspace-0b6124%40workspace%3A."
dependencies:
braces: "npm:^3.0.2"
chokidar: "npm:^3.5.3"
dedent: "npm:^1.5.1"
fast-glob: "npm:^3.2.11"
jsonc-parser: "npm:^3.0.0"
proper-lockfile: "npm:^4.1.2"
ts-dedent: "npm:^2.2.0"
bin:
wireit: bin/wireit.js
languageName: node
linkType: soft
`,
});

// this is what should modify the lockfile, so we don't have to do it
// manually
rig.exec('yarn');

// logging: can be deleted once this works
//
// since the `.bin` is already set up by the test rig, IDK if we should need to
// modify the lockfile to include wireit. however, it's saying
//
// Usage Error: Couldn't find a script name "wireit" in the top-level (used by inner-d81e84@workspace:packages/inner).
//
// so i'm trying to make it work
rig.exec('ls -la node_modules/.bin');
rig.read('yarn.lock').then(console.log);

{
const exec = rig.exec('yarn run cmd', {cwd: 'packages/inner'});
(
await Promise.race([
innerCmd.nextInvocation(),
rootCmd.nextInvocation(),
])
).exit(0);
assert.equal((await exec.exit).code, 0);
assert.equal(innerCmd.numInvocations, 1);
assert.equal(rootCmd.numInvocations, 0);
}
}),
);

test(
'commands run under pnpm workspaces',
rigTest(async ({rig}) => {
Expand Down
48 changes: 40 additions & 8 deletions src/test/cli-options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
* SPDX-License-Identifier: Apache-2.0
*/

import * as pathlib from 'path';
import * as assert from 'uvu/assert';
import {suite} from 'uvu';
import {rigTest} from './util/rig-test.js';
import {WireitTestRig} from './util/test-rig.js';
import {Options} from '../cli-options.js';
import * as pathlib from 'path';
import {Agent, Options} from '../cli-options.js';
import {IS_WINDOWS} from '../util/windows.js';
import {injectYarnBerryToRig} from './util/yarn-berry.js';
import {Result} from '../error.js';
import {WireitTestRig} from './util/test-rig.js';
import {rigTest} from './util/rig-test.js';
import {suite} from 'uvu';

const test = suite<object>();

Expand All @@ -29,6 +31,25 @@ async function getOptionsResult(
extraScripts?: Record<string, string>,
): Promise<Result<Options>> {
rig.env.WIREIT_DEBUG_LOG_FILE = '';

if (command.startsWith('yarnBerry')) {
// `yarn` chooses whether to use "classic" or "berry" based on the presence
// of `yarnPath` in `.yarnrc.yml`.
//
// To closest simulate a user's environment, set `yarnPath` and let the
// global `yarn` command pick it when testing `yarnBerry`.
command = command.replace(/yarnBerry/g, 'yarn');

extraScripts = Object.fromEntries(
Object.entries(extraScripts || {}).map(([scriptName, scriptCommand]) => [
scriptName,
scriptCommand.replace(/yarnBerry/g, 'yarn'),
]),
);

await injectYarnBerryToRig(rig);
}

await rig.write({
'package.json': {
scripts: {
Expand All @@ -39,7 +60,12 @@ async function getOptionsResult(
},
},
});
env = {...env, WIREIT_DEBUG_LOG_FILE: ''};

env = {
...env,
WIREIT_DEBUG_LOG_FILE: '',
};

assert.equal((await rig.exec(command, {env}).exit).code, 0);
return JSON.parse(await rig.read('options.json')) as Result<Options>;
}
Expand Down Expand Up @@ -67,8 +93,14 @@ async function assertOptions(
});
}

for (const command of ['npm', 'yarn', 'pnpm'] as const) {
const agent = command === 'yarn' ? 'yarnClassic' : command;
// yarnBerry tests work everywhere but Windows (#1014). Until Windows testing
// is fixed, skip yarnBerry tests on Windows to prevent CI failures.
const commands = (['npm', 'yarn', 'pnpm', 'yarnBerry'] as const).filter(
(command) => !IS_WINDOWS || command !== 'yarnBerry',
);

for (const command of commands) {
const agent: Agent = command === 'yarn' ? 'yarnClassic' : command;
// eslint-disable-next-line @typescript-eslint/unbound-method
const skipIfYarn = command === 'yarn' ? test.skip : test;
// eslint-disable-next-line @typescript-eslint/unbound-method
Expand Down
38 changes: 23 additions & 15 deletions src/test/util/test-rig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,22 +64,30 @@ export class WireitTestRig
*/
override async setup() {
await super.setup();
const absWireitBinaryPath = pathlib.resolve(repoRoot, 'bin', 'wireit.js');
const absWireitTempInstallPath = pathlib.resolve(
this.temp,
'node_modules',
'.bin',
'wireit',

await Promise.all(
[['wireit', ['bin', 'wireit.js']] as const].map(
async ([name, pathParts]) => {
const binaryPath = pathlib.resolve(repoRoot, ...pathParts);
const tempInstallPath = pathlib.resolve(
this.temp,
'node_modules',
'.bin',
name,
);

if (IS_WINDOWS) {
// Npm install works differently on Windows, since it won't recognize a
// shebang like "#!/usr/bin/env node". Npm instead uses the cmd-shim
// package to generate Windows shell wrappers for each binary, so we do
// that here too.
await cmdShim(binaryPath, tempInstallPath);
} else {
await this.symlink(binaryPath, tempInstallPath, 'file');
}
},
),
);
if (IS_WINDOWS) {
// Npm install works differently on Windows, since it won't recognize a
// shebang like "#!/usr/bin/env node". Npm instead uses the cmd-shim
// package to generate Windows shell wrappers for each binary, so we do
// that here too.
await cmdShim(absWireitBinaryPath, absWireitTempInstallPath);
} else {
await this.symlink(absWireitBinaryPath, absWireitTempInstallPath, 'file');
}
}

/**
Expand Down
52 changes: 52 additions & 0 deletions src/test/util/yarn-berry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* @license
* Copyright 2024 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import * as pathlib from 'path';
import {dedent} from 'ts-dedent';
import {WireitTestRig} from './test-rig.js';

type TestEnviron = {
rig: WireitTestRig;
command: string;
extraScripts?: Record<string, string>;
};

export const injectYarnBerryToRig = async (rig: WireitTestRig) => {
await rig.write({
'.yarnrc.yml': `
nodeLinker: node-modules
yarnPath: ${pathlib.join(
process.cwd(),
'third_party',
'.yarn',
'releases',
'yarn-4.0.1.cjs',
)}
`,
});

// `yarn.lock` tells `yarn` that this test is meant to be its own workspace
// root, even though there are higher `package.json` files in the file tree.
//
// This is the `yarn.lock` you get if you initialize `yarn` in an empty
// folder.
await rig.write({
'yarn.lock': dedent`
# This file is generated by running "yarn install" inside your project.
# Manual changes might be lost - proceed with caution!

__metadata:
version: 8
cacheKey: 10c0

"root-workspace-0b6124@workspace:.":
version: 0.0.0-use.local
resolution: "root-workspace-0b6124@workspace:."
languageName: unknown
linkType: soft
`,
});
};
Loading