Skip to content

Commit

Permalink
Allow for multiple root directories when scanning for fence files (sm…
Browse files Browse the repository at this point in the history
…ikula#91)

## Motivation

In owa, we currently rely on excluding the node_modules directory.

This works fine under current usage, but BuildXL by convention stores state in the Out directory, which stores unique logs across multiple runs -- this was causing good-fences to take upwards of 20 minutes on subsequent runs.

By specifying the rootDirs as `packages` and `shared` instead of relying on traversing from the common directory `.`, we speed up good-fences (and avoid transient read errors caused by files being deleted out from under good-fences as it traverses BuildXL's internal state).

## Changes
- changes rootDir to an array of strings
- supports falling back to rootDir: string as an argument for backwords compatibility
- upgrades commander to use variadic arguments to support rootDir from the cli
  • Loading branch information
Adjective-Object authored May 6, 2021
1 parent a8e86be commit d1c2711
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 21 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
},
"author": "Scott Mikula <[email protected]>",
"dependencies": {
"commander": "^2.11.0",
"commander": "^7.2.0",
"minimatch": "^3.0.4",
"typescript": "^4.0.3"
},
"devDependencies": {
"@types/commander": "^2.11.0",
"@types/commander": "^2.12.2",
"@types/jest": "^26.0.15",
"@types/node": "^12.7.8",
"jest": "^26.6.0"
Expand Down
2 changes: 1 addition & 1 deletion src/core/ImportRecord.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export default class ImportRecord {
// Is this import an external dependency (i.e. is it under node_modules or outside the rootDir)?
get isExternal() {
let isInNodeModules = this.filePath.split(path.sep).indexOf('node_modules') != -1;
let isUnderRootFolder = this.filePath.startsWith(getOptions().rootDir);
let isUnderRootFolder = getOptions().rootDir.some(rootDir => this.filePath.startsWith(rootDir));
let isLocalRelativePath = this.filePath.startsWith('./');
let isExternalPath = !isUnderRootFolder && !isLocalRelativePath;
return isInNodeModules || isExternalPath;
Expand Down
9 changes: 5 additions & 4 deletions src/core/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import { run } from './runner';
const packageVersion = require('../../package.json').version;

// Parse command line options
const options = commander
const program = commander
.version(packageVersion)
.option('-p, --project <string>', 'tsconfig.json file')
.option('-r, --rootDir <string>', 'root directory of the project')
.parse(process.argv) as RawOptions;
.option('-p, --project <string> ', 'tsconfig.json file')
.option('-r, --rootDir <string...>', 'root directories of the project');
program.parse(process.argv);
const options = program.opts() as RawOptions;

// Run good-fences
const result = run(options);
Expand Down
2 changes: 1 addition & 1 deletion src/types/Options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ import NormalizedPath from './NormalizedPath';

export default interface Options {
project: NormalizedPath;
rootDir: NormalizedPath;
rootDir: NormalizedPath[];
ignoreExternalFences: boolean;
}
2 changes: 1 addition & 1 deletion src/types/RawOptions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export default interface RawOptions {
project?: string;
rootDir?: string;
rootDir?: string | string[];
ignoreExternalFences?: boolean;
}
4 changes: 3 additions & 1 deletion src/utils/getAllConfigs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ export default function getAllConfigs(): ConfigSet {
configSet = {};

let files: string[] = [];
accumulateFences(getOptions().rootDir, files, getOptions().ignoreExternalFences);
for (let rootDir of getOptions().rootDir) {
accumulateFences(rootDir, files, getOptions().ignoreExternalFences);
}

files.forEach(file => {
loadConfig(file, configSet);
Expand Down
10 changes: 8 additions & 2 deletions src/utils/getOptions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import RawOptions from '../types/RawOptions';
import Options from '../types/Options';
import normalizePath from './normalizePath';
import NormalizedPath from '../types/NormalizedPath';

let options: Options;

Expand All @@ -10,10 +11,15 @@ export default function getOptions() {

export function setOptions(rawOptions: RawOptions) {
// Normalize and apply defaults
const rootDir = normalizePath(rawOptions.rootDir || process.cwd());
const nonNormalizedRoots: string[] = Array.isArray(rawOptions.rootDir)
? rawOptions.rootDir
: [rawOptions.rootDir || process.cwd()];
const rootDir: NormalizedPath[] = nonNormalizedRoots.map(
(individualRootDirPath: string) => normalizePath(individualRootDirPath)
);
const project = rawOptions.project
? normalizePath(rawOptions.project)
: normalizePath(rootDir, 'tsconfig.json');
: normalizePath(rootDir[0], "tsconfig.json");

options = {
project,
Expand Down
18 changes: 9 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -635,12 +635,12 @@
dependencies:
"@babel/types" "^7.3.0"

"@types/commander@^2.11.0":
version "2.11.0"
resolved "https://registry.yarnpkg.com/@types/commander/-/commander-2.11.0.tgz#7fc765ccad14827e2babd6a99583359ff3e40563"
integrity sha512-G4+ewSX5/TPqpUjltkuyJ4QhX8oTy94WEyJMvC+R8cg/qKGjq+/n+b/TRVe/+/278jV9Iot5dS186r7NCcUrtg==
"@types/commander@^2.12.2":
version "2.12.2"
resolved "https://registry.yarnpkg.com/@types/commander/-/commander-2.12.2.tgz#183041a23842d4281478fa5d23c5ca78e6fd08ae"
integrity sha512-0QEFiR8ljcHp9bAbWxecjVRuAMr16ivPiGOw6KFQBVrVd0RQIcM3xKdRisH2EDWgVWujiYtHwhSkSUoAAGzH7Q==
dependencies:
"@types/node" "*"
commander "*"

"@types/graceful-fs@^4.1.2":
version "4.1.3"
Expand Down Expand Up @@ -1125,10 +1125,10 @@ combined-stream@^1.0.6, combined-stream@~1.0.6:
dependencies:
delayed-stream "~1.0.0"

commander@^2.11.0:
version "2.11.0"
resolved "https://registry.yarnpkg.com/commander/-/commander-2.11.0.tgz#157152fd1e7a6c8d98a5b715cf376df928004563"
integrity sha512-b0553uYA5YAEGgyYIGYROzKQ7X5RAqedkfjiZxwi0kL1g3bOaBNNZfYkzt/CL0umgD5wc9Jec2FbB98CjkMRvQ==
commander@*, commander@^7.2.0:
version "7.2.0"
resolved "https://registry.yarnpkg.com/commander/-/commander-7.2.0.tgz#a36cb57d0b501ce108e4d20559a150a391d97ab7"
integrity sha512-QrWXB+ZQSVPmIWIhtEO9H+gwHaMGYiF5ChvoJ+K9ZGHG/sVsa6yiesAD1GC/x46sET00Xlwo1u49RVVVzvcSkw==

component-emitter@^1.2.1:
version "1.3.0"
Expand Down

0 comments on commit d1c2711

Please sign in to comment.