diff --git a/CHANGELOG.md b/CHANGELOG.md index 421c6108d..b8a254bea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,14 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### Changed -- Starting to improve error messages by drawing squiggles underneith the - specific part of the package.json file that's in error. +- [**Breaking**] A leading `/` on a `files` or `output` glob pattern is now + interpreted relative to the current package directory. Previously it was + interpreted relative to the root of the filesystem. In the case of `files` + (but not `output`), it is still possible to reference files outside of the + current package with a pattern like `../foo`. + +- Starting to improve error messages by drawing squiggles underneath the + specific part of the `package.json` file that's in error. ## [0.3.1] - 2022-04-30 diff --git a/README.md b/README.md index 71f9ee784..9b1bac25c 100644 --- a/README.md +++ b/README.md @@ -526,10 +526,17 @@ The following glob syntaxes are supported in the `files` and `output` arrays: Also note these details: +- Paths should always use `/` (forward-slash) delimiters, even on Windows. +- Paths are interpreted relative to the current package even if there is a + leading `/` (e.g. `/foo` is the same as `foo`). - Whenever a directory is matched, all recursive children of that directory are included. +- `files` are allowed to reach outside of the current package using e.g. + `../foo`. `output` files cannot reference files outside of the current + package. - Symlinks in input `files` are followed, so that they are identified by their content. -- Symlinks in `output` files are cached as symlinks, so that restoring from cache doesn't create unnecessary copies. +- Symlinks in `output` files are cached as symlinks, so that restoring from + cache doesn't create unnecessary copies. - The order of `!exclude` patterns is significant. - Hidden/dot files are matched by `*` and `**`. - Patterns are case-sensitive (if supported by the filesystem). diff --git a/package-lock.json b/package-lock.json index bcd7cdfd5..4ba2238a8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ ], "dependencies": { "@actions/cache": "=2.0.2", + "braces": "^3.0.2", "chokidar": "^3.5.3", "fast-glob": "^3.2.11", "jsonc-parser": "^3.0.0" @@ -21,6 +22,7 @@ "wireit": "bin/wireit.js" }, "devDependencies": { + "@types/braces": "^3.0.1", "@types/node": "^14.0.0", "@typescript-eslint/eslint-plugin": "^5.17.0", "@typescript-eslint/parser": "^5.17.0", @@ -402,6 +404,12 @@ "node": ">= 6" } }, + "node_modules/@types/braces": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/@types/braces/-/braces-3.0.1.tgz", + "integrity": "sha512-+euflG6ygo4bn0JHtn4pYqcXwRtLvElQ7/nnjDu7iYG56H0+OhCd7d6Ug0IE3WcFpZozBKW2+80FUbv5QGk5AQ==", + "dev": true + }, "node_modules/@types/json-schema": { "version": "7.0.11", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.11.tgz", @@ -4526,6 +4534,12 @@ "integrity": "sha512-RbzJvlNzmRq5c3O09UipeuXno4tA1FE6ikOjxZK0tuxVv3412l64l5t1W5pj4+rJq9vpkm/kwiR07aZXnsKPxw==", "dev": true }, + "@types/braces": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/@types/braces/-/braces-3.0.1.tgz", + "integrity": "sha512-+euflG6ygo4bn0JHtn4pYqcXwRtLvElQ7/nnjDu7iYG56H0+OhCd7d6Ug0IE3WcFpZozBKW2+80FUbv5QGk5AQ==", + "dev": true + }, "@types/json-schema": { "version": "7.0.11", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.11.tgz", diff --git a/package.json b/package.json index 27e76a649..4dad2ee46 100644 --- a/package.json +++ b/package.json @@ -236,6 +236,7 @@ } }, "devDependencies": { + "@types/braces": "^3.0.1", "@types/node": "^14.0.0", "@typescript-eslint/eslint-plugin": "^5.17.0", "@typescript-eslint/parser": "^5.17.0", @@ -255,6 +256,7 @@ }, "dependencies": { "@actions/cache": "=2.0.2", + "braces": "^3.0.2", "chokidar": "^3.5.3", "fast-glob": "^3.2.11", "jsonc-parser": "^3.0.0" diff --git a/src/test/cache-common.ts b/src/test/cache-common.ts index f16e786b6..46ef5c5ad 100644 --- a/src/test/cache-common.ts +++ b/src/test/cache-common.ts @@ -1007,4 +1007,71 @@ export const registerCommonCacheTests = ( } }) ); + + test( + 'leading slash on output glob is package relative', + timeout(async ({rig}) => { + const cmdA = await rig.newCommand(); + await rig.write({ + 'package.json': { + scripts: { + a: 'wireit', + }, + wireit: { + a: { + command: cmdA.command, + files: ['input'], + output: ['/output'], + }, + }, + }, + input: 'v0', + }); + + // Initial run with input v0. + { + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + await rig.write({output: 'v0'}); + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + assert.equal(cmdA.numInvocations, 1); + assert.equal(await rig.read('output'), 'v0'); + } + + // Input changed to v1. Run again. + { + await rig.write({input: 'v1'}); + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + await rig.write({output: 'v1'}); + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + assert.equal(cmdA.numInvocations, 2); + assert.equal(await rig.read('output'), 'v1'); + } + + // Input changed back to v0. Output should be cached. + { + await rig.write({input: 'v0'}); + const exec = rig.exec('npm run a'); + const res = await exec.exit; + assert.equal(res.code, 0); + assert.equal(cmdA.numInvocations, 2); + assert.equal(await rig.read('output'), 'v0'); + } + + // Input changed back to v1. Output should be cached. + { + await rig.write({input: 'v1'}); + const exec = rig.exec('npm run a'); + const res = await exec.exit; + assert.equal(res.code, 0); + assert.equal(cmdA.numInvocations, 2); + assert.equal(await rig.read('output'), 'v1'); + } + }) + ); }; diff --git a/src/test/clean.test.ts b/src/test/clean.test.ts index 886c0c64e..d03337684 100644 --- a/src/test/clean.test.ts +++ b/src/test/clean.test.ts @@ -531,4 +531,39 @@ test( }) ); +test( + 'leading slash on output glob is package relative', + timeout(async ({rig}) => { + const cmdA = await rig.newCommand(); + await rig.write({ + 'package.json': { + scripts: { + a: 'wireit', + }, + wireit: { + a: { + command: cmdA.command, + output: ['/output'], + }, + }, + }, + output: 'foo', + }); + + // Output should exist before we run the script. + assert.ok(await rig.exists('output')); + + // Output should be deleted between running the script and executing the + // command. + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + assert.not(await rig.exists('output')); + + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + assert.equal(cmdA.numInvocations, 1); + }) +); + test.run(); diff --git a/src/test/freshness.test.ts b/src/test/freshness.test.ts index 5163d6291..e37d10e21 100644 --- a/src/test/freshness.test.ts +++ b/src/test/freshness.test.ts @@ -1654,4 +1654,48 @@ test( }) ); +test( + 'leading slash on files glob is package relative', + timeout(async ({rig}) => { + const cmdA = await rig.newCommand(); + await rig.write({ + 'package.json': { + scripts: { + a: 'wireit', + }, + wireit: { + a: { + command: cmdA.command, + files: ['/input.txt'], + }, + }, + }, + 'input.txt': 'v0', + }); + + // Initially stale, so command is invoked. + { + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + assert.equal(cmdA.numInvocations, 1); + } + + // Input file changed, so script is stale, and command is invoked. + { + await rig.write({ + 'input.txt': 'v1', + }); + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + inv.exit(0); + const res = await exec.exit; + assert.equal(res.code, 0); + assert.equal(cmdA.numInvocations, 2); + } + }) +); + test.run(); diff --git a/src/test/glob.test.ts b/src/test/glob.test.ts index 92df7307e..b5e3b0ea3 100644 --- a/src/test/glob.test.ts +++ b/src/test/glob.test.ts @@ -44,7 +44,7 @@ test.before.each(async (ctx) => { files, patterns, expected, - cwd = rig.temp, + cwd = '.', absolute = false, followSymlinks = true, includeDirectories = false, @@ -73,7 +73,7 @@ test.before.each(async (ctx) => { let actual, error; try { actual = await glob(patterns, { - cwd, + cwd: rig.resolve(cwd), absolute, followSymlinks, includeDirectories, @@ -486,4 +486,57 @@ test('dirent tags symlinks to directories as directories when followSymlinks=tru assert.not(actual[0].dirent.isSymbolicLink()); }); +test('re-roots to cwd', ({check}) => + check({ + files: ['foo'], + patterns: ['/foo'], + expected: ['foo'], + })); + +test('re-roots to cwd with exclusion', ({check}) => + check({ + files: ['foo', 'bar', 'baz'], + patterns: ['/*', '!/bar'], + expected: ['foo', 'baz'], + })); + +test('re-rooting allows ../', ({check}) => + check({ + cwd: 'subdir', + files: ['foo', 'subdir/'], + patterns: ['../foo'], + expected: ['../foo'], + })); + +// TODO(aomarks) This should be normalized to "foo" consistently. It currently +// differs on Windows between Node 14 and 16. +test.skip('re-rooting handles /./foo', ({check}) => + check({ + files: ['foo'], + patterns: ['/./foo'], + expected: [`.${pathlib.sep}foo`], + })); + +test('re-rooting handles /../foo', ({check}) => + check({ + cwd: 'subdir', + files: ['foo', 'subdir/'], + patterns: ['/../foo'], + expected: ['../foo'], + })); + +test('re-roots to cwd with braces', ({check}) => + check({ + files: ['foo', 'bar'], + patterns: ['{/foo,/bar}'], + expected: ['foo', 'bar'], + })); + +test('braces can be escaped', ({check}) => + check({ + files: ['{foo,bar}'], + patterns: ['\\{foo,bar\\}'], + expected: ['{foo,bar}'], + })); + test.run(); diff --git a/src/util/glob.ts b/src/util/glob.ts index 4e3d58da6..4a12a09f4 100644 --- a/src/util/glob.ts +++ b/src/util/glob.ts @@ -5,6 +5,7 @@ */ import fastGlob from 'fast-glob'; +import braces from 'braces'; import * as pathlib from 'path'; import type {Entry} from 'fast-glob'; @@ -56,6 +57,8 @@ interface GlobGroup { * * - Input patterns must be / separated. * - Matches are returned with the OS-specific separator. + * - Leading `/`s are interpreted as relative to the `cwd`, instead of the root + * of the filesystem. * - Dot (aka hidden) files are always matched. * - Empty or blank patterns throw. * - The order of "!exclusion" patterns matter (i.e. files can be "re-included" @@ -85,17 +88,22 @@ export async function glob( return []; } - let expandedPatterns = patterns; - if (opts.expandDirectories) { - expandedPatterns = []; // New array so we don't mutate input patterns array. - for (const pattern of patterns) { - expandedPatterns.push(pattern); - // Also include a recursive-children version of every pattern, in case the - // pattern refers to a directory. This gives us behavior similar to the - // npm package.json "files" array, where matching a directory implicitly - // includes all transitive children. - if (!isRecursive(pattern)) { - expandedPatterns.push(pattern + '/**'); + const expandedPatterns = []; // New array so we don't mutate input patterns array. + for (const pattern of patterns) { + // We need to expand `{foo,bar}` style brace patterns ourselves so that we + // can reliably interpret the syntax of the pattern. For example, for + // re-rooting we need to check for a leading `/`, but we can't do that + // directly on `{/foo,/bar}`. + for (const expanded of braces(pattern, {expand: true})) { + expandedPatterns.push(expanded); + if (opts.expandDirectories) { + // Also include a recursive-children version of every pattern, in case + // the pattern refers to a directory. This gives us behavior similar to + // the npm package.json "files" array, where matching a directory + // implicitly includes all transitive children. + if (!isRecursive(expanded)) { + expandedPatterns.push(expanded + '/**'); + } } } } @@ -131,8 +139,15 @@ export async function glob( // We want each group to include all subsequent negated patterns. The simplest // way to do that is to build the groups backwards. for (let i = expandedPatterns.length - 1; i >= 0; i--) { - const pattern = expandedPatterns[i]; + let pattern = expandedPatterns[i]; const isExclusive = pattern[0] === '!'; + if (isExclusive) { + pattern = pattern.slice(1); // Remove the "!" + } + // Ignore leading `/`s so that e.g. "/foo" is interpreted relative to the + // cwd, instead of relative to the root of the filesystem. We want to include + // >1 leading slashes, since those are technically valid paths too. + pattern = pattern.replace(/^\/+/, ''); if (isExclusive) { if (prevWasInclusive) { // A new group is needed because this exclusion comes before an @@ -151,8 +166,7 @@ export async function glob( } groups.push(currentGroup); } - const inverted = pattern.slice(1); // Remove the "!" - currentGroup.exclude.push(inverted); + currentGroup.exclude.push(pattern); } else if (pattern.match(/^\s*$/)) { // fast-glob already throws on empty strings, but we also throw on // only-whitespace patterns. @@ -189,6 +203,9 @@ export async function glob( // ENOTDIR errors when the path we appended to was not a directory. We // can't know in advance which patterns refer to directories. suppressErrors: true, + // We already do brace expansion ourselves. Doing it again would be + // inefficient and would also break brace escaping. + braceExpansion: false, }); for (const match of matches) { combinedMap.set(match.path, match); diff --git a/tsconfig.json b/tsconfig.json index 2ad1a91f9..e5602d090 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -3,6 +3,7 @@ "target": "es2020", "module": "es2022", "moduleResolution": "node", + "esModuleInterop": true, "useDefineForClassFields": true, "lib": ["es2020"], "rootDir": "src",