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

Modernize Jakefile + fix project references breaks #24938

Merged
merged 36 commits into from
Jun 15, 2018

Conversation

RyanCavanaugh
Copy link
Member

Merges parser, core, and compiler back into compiler per request

Rewrites the Jakefile and fixes a variety of tsbuild bugs.

Includes LKG update; see me offline for an analysis of what's i nthere

@RyanCavanaugh
Copy link
Member Author

CI failures are just node6 OOMing as usual

@weswigham
Copy link
Member

weswigham commented Jun 14, 2018

@RyanCavanaugh node6 doesn't usually OOM; it usually just times out, and usually only on travis (as we've never had the issue on VSTS), AFAIK (and usually not repeatable if you rerun the build). I'd find consistent OOMs (even if confined to one version) fairly concerning; it probably means that on that version we OOM but on the others we're close to being OOM.

@weswigham
Copy link
Member

weswigham commented Jun 14, 2018

For reference, in node8 the build step that runs OOM seems to take 85s on travis, while on node6, it runs OOM after just 97s (and it'll take longer when nearing OOM as it frantically GCs). The similarity would lead me to believe that, barring running into some memory blowup issue confined to node6 late in the build, that that build is on the knife's edge of even being possible (and the slight difference in runtime overhead between 6 and 8 is enough to throw a wrench into the build).

@rbuckton can probably relate, considering he's already had to acknowledge that running all the builds in one processes like that isn't possible when running under gulp, so it's not too surprising tsbuild would hit the same limitations...

@RyanCavanaugh
Copy link
Member Author

K, investigating the memory thing

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions, since I don't understand a lot of the infrastructure that's changing here. Besides those, let's go ahead and merge this if it will help our build.

Jakefile.js Outdated
exec(`${host} ${Paths.scripts.produceLKG}`, () => {
const sizeAfter = getDirSize(Paths.lkg);
if (sizeAfter > (sizeBefore * 1.10)) {
// throw new Error("The lib folder increased by 10% or more. This likely indicates a bug.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the Error should go back in and the console.log should go away.

Jakefile.js Outdated
ex.run();
}
/*
file(Paths.tsserverLibraryDefinitionFile, [TaskNames.coreBuild, Paths.copyright, ...libraryTargets], function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed?

@@ -45,7 +45,7 @@
"@types/minimatch": "latest",
"@types/minimist": "latest",
"@types/mkdirp": "latest",
"@types/mocha": "latest",
"@types/mocha": "^5.2.2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the changes in package.json intentional?

@@ -1095,16 +1109,18 @@ namespace ts {
return resolvedNames;
}

function buildAllProjects() {
function buildAllProjects(): number {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why isn't this return type ExitStatus?

@@ -1,19 +0,0 @@
{
"extends": "../tsconfig-base",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are these files replaced by (if anything)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing

Jakefile.js Outdated
var nodeServerInFile = "tests/webTestServer.ts";
compileFile(nodeServerOutFile, [nodeServerInFile], [builtLocalDirectory, tscFile], [], /*useBuiltCompiler:*/ true, { noOutFile: true, lib: "es6" });
jake.mkdirP(Paths.baselines.local);
jake.mkdirP(Paths.baselines.localRwc);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to make an internal folder for people without internal checked out. We probably don't want that.

# Conflicts:
#	tests/baselines/reference/api/typescript.d.ts
@RyanCavanaugh RyanCavanaugh merged commit 94ee765 into microsoft:master Jun 15, 2018
@rbuckton
Copy link
Member

Apparently we are no longer writing out typescript.js in the jake builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants