Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Commit

Permalink
Repro option creates package with sourcefiles, debugger startup script (
Browse files Browse the repository at this point in the history
#2289)

Summary:
Release Notes:

- Created DebugReproManager to capture all sourcefiles touched by Prepack (to include minimal subset of useful sourcefiles in the debug package)
- `--repro` splits into two: `--reproUnconditionally` which will create a debug package _regardless_ of Prepack's success/failure, and `--reproOnFatal`, which will _only_ create a debug package if Prepack outputs a `FatalError`.
- The debug package now includes all relevant sourcefiles (except for node modules), a copy of the version of Prepack (lib) that was used when the package was created, and a script to `yarn install` the relevant modules for the included version of Prepack, then start the Nuclide Prepack debugger with the proper parameters for files in the debug package (including original prepack arguments). This is in addition to the original input files.
- The impact of having the `DebugReproManager` on in `--reproOnFatal` mode all the time is negligible, as show in the table below. This flag will be always be enabled on Sandcastle builds so that failures are more easily debugged.
    - The time difference between no repro flag and `--reproOnFatal` seems like it can be written off as simple variance between runs. The larger increase when actually creating the zip comes from reading and zipping the files, which takes time proportional how many files are touched.
- SourceMapManager was refactored to not use `Invariant` or `SourceFile`s. This is so that `DebugReproManager` import it without increasing the flow cycle, and allows the `DebugReproManager` to be passed from Prepack to the CLI to create the repro package.
- The repro option introduces a potential for a subtle race condition that is addressed as follows:
    - The last `if (!success && reproMode === "none") process.exit(1);` must check reproMode because `generateDebugRepro` involves an async process (directory zipping). If there is an ongoing repro and the process exits, the repro may terminate prematurely, causing no repro to be generated. Instead, this only triggers if there is no repro -- if there is, the `generateDebugRepro` function will handle process exiting if it needs to.

Usage:
```node [prepack] [files to prepack] --reproOnFatal /Absolute/path/to/repro/bundle.zip --debugBuckRoot /buck/root```
or
```node [prepack] [files to prepack] --reproUnconditionally /Absolute/path/to/repro/bundle.zip --debugBuckRoot /buck/root```

Demo: https://www.dropbox.com/s/p62ves2p55fyyl7/--repro%20annotated%20demo.mp4?dl=0
Pull Request resolved: #2289

Differential Revision: D9002841

Pulled By: caiismyname

fbshipit-source-id: 623b362f963095f1cd8163684fd6e76596e7c4fc
  • Loading branch information
caiismyname authored and facebook-github-bot committed Jul 26, 2018
1 parent 79bce3f commit bfe8cd2
Show file tree
Hide file tree
Showing 17 changed files with 342 additions and 60 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@
"seedrandom": "^2.4.2",
"source-map": "^0.5.6",
"vscode-debugadapter": "^1.24.0",
"vscode-debugprotocol": "^1.24.0"
"vscode-debugprotocol": "^1.24.0",
"zip-dir": "^1.0.2"
},
"devDependencies": {
"@babel/cli": "^7.0.0-beta.53",
Expand Down
9 changes: 6 additions & 3 deletions src/construct_realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,18 @@ import type { RealmOptions } from "./options.js";
import { RealmStatistics } from "./statistics.js";
import * as evaluators from "./evaluators/index.js";
import * as partialEvaluators from "./partial-evaluators/index.js";
import { Environment } from "./singletons.js";
import { Environment, DebugReproManager } from "./singletons.js";
import { ObjectValue } from "./values/index.js";
import { DebugServer } from "./debugger/server/Debugger.js";
import simplifyAndRefineAbstractValue from "./utils/simplifier.js";
import invariant from "./invariant.js";
import type { DebuggerConfigArguments } from "./types";
import type { DebuggerConfigArguments, DebugReproArguments } from "./types";

export default function(
opts: RealmOptions = {},
debuggerConfigArgs: void | DebuggerConfigArguments,
statistics: void | RealmStatistics = undefined
statistics: void | RealmStatistics = undefined,
debugReproArgs: void | DebugReproArguments
): Realm {
initializeSingletons();
let r = new Realm(opts, statistics || new RealmStatistics());
Expand All @@ -40,6 +41,8 @@ export default function(
}
}

if (debugReproArgs !== undefined) r.debugReproManager = DebugReproManager.construct(debugReproArgs);

let i = r.intrinsics;
initializeIntrinsics(i, r);
// TODO: Find a way to let different environments initialize their own global
Expand Down
2 changes: 1 addition & 1 deletion src/debugger/server/Debugger.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
} from "./../../environment.js";
import { CompilerDiagnostic } from "../../errors.js";
import type { Severity } from "../../errors.js";
import { SourceMapManager } from "./SourceMapManager.js";
import { SourceMapManager } from "../../utils/SourceMapManager.js";
import type { DebuggerConfigArguments } from "../../types";

export class DebugServer {
Expand Down
6 changes: 6 additions & 0 deletions src/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,12 @@ export class LexicalEnvironment {
if (this.realm.debuggerInstance) {
this.realm.debuggerInstance.checkForActions(ast);
}
if (this.realm.debugReproManager) {
if (ast.loc !== undefined && ast.loc !== null && ast.loc.source) {
this.realm.debugReproManager.addSourceFile(ast.loc.source);
}
}

let res = this.evaluateAbstract(ast, strictCode, metadata);
invariant(res instanceof Value || res instanceof Reference, ast.type);
return res;
Expand Down
11 changes: 10 additions & 1 deletion src/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,27 @@ export type ErrorCode = "PP0001";
// This is the error format used to report errors to the caller-supplied
// error-handler
export class CompilerDiagnostic extends Error {
constructor(message: string, location: ?BabelNodeSourceLocation, errorCode: string, severity: Severity) {
constructor(
message: string,
location: ?BabelNodeSourceLocation,
errorCode: string,
severity: Severity,
sourceFilePaths?: { sourceMaps: Array<string>, sourceFiles: Array<{ absolute: string, relative: string }> }
) {
super(message);

this.location = location;
this.severity = severity;
this.errorCode = errorCode;
this.sourceFilePaths = sourceFilePaths;
}

callStack: void | string;
location: ?BabelNodeSourceLocation;
severity: Severity;
errorCode: string;
// For repro bundles, we need to pass the names of all sourcefiles/sourcemaps touched by Prepack back to the CLI.
sourceFilePaths: void | { sourceMaps: Array<string>, sourceFiles: Array<{ absolute: string, relative: string }> };
}

// This error is thrown to exit Prepack when an ErrorHandler returns 'FatalError'
Expand Down
2 changes: 2 additions & 0 deletions src/initialize-singletons.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { PropertiesImplementation } from "./methods/properties.js";
import { ToImplementation } from "./methods/to.js";
import { WidenImplementation } from "./methods/widen.js";
import { concretize } from "./utils/ConcreteModelConverter.js";
import { DebugReproManagerImplementation } from "./utils/DebugReproManager.js";
import * as utils from "./utils.js";

export default function() {
Expand All @@ -34,4 +35,5 @@ export default function() {
Singletons.setWiden((new WidenImplementation(): any));
Singletons.setConcretize(concretize);
Singletons.setUtils(utils);
Singletons.setDebugReproManager((new DebugReproManagerImplementation(): any));
}
Loading

0 comments on commit bfe8cd2

Please sign in to comment.