From 0c65334b8f48ce20db1f96dca19a7ccd08b879ea Mon Sep 17 00:00:00 2001 From: Aleksandar Milicevic Date: Mon, 21 Oct 2019 15:29:05 -0700 Subject: [PATCH] Use checked-in package-lock.json file for npm install (#1078) This avoids generating package-lock.json upon every build; given that npm install without a lock file is not necessarily deterministic, every build can in theory generate a new package-lock.json. Instead, when the lock file is explicitly provided, the behavior of npm install is fixed and deterministic. --- Public/Sdk/Experimental/NodeJs/node.dsc | 2 +- Public/Sdk/Experimental/NodeJs/npm.dsc | 14 +++++++++++-- Public/Src/IDE/VsCode/BuildXL.IDE.VsCode.dsc | 22 ++++++++------------ Shared/Scripts/bxl.ps1 | 2 +- config.dsc | 2 +- 5 files changed, 24 insertions(+), 18 deletions(-) diff --git a/Public/Sdk/Experimental/NodeJs/node.dsc b/Public/Sdk/Experimental/NodeJs/node.dsc index a65443eaf..7aaefd6b2 100644 --- a/Public/Sdk/Experimental/NodeJs/node.dsc +++ b/Public/Sdk/Experimental/NodeJs/node.dsc @@ -105,7 +105,7 @@ namespace Node { } @@public - export function tscCompile(workingDirectory: Directory, dependencies: StaticDirectory[]) : OpaqueDirectory { + export function tscCompile(workingDirectory: Directory, dependencies: Transformer.InputArtifact[]) : OpaqueDirectory { const outPath = d`${workingDirectory}/out`; const arguments: Argument[] = [ Cmd.argument(Artifact.none(f`${workingDirectory}/node_modules/typescript/lib/tsc.js`)), diff --git a/Public/Sdk/Experimental/NodeJs/npm.dsc b/Public/Sdk/Experimental/NodeJs/npm.dsc index 90bc7fb3c..1a5dbfe79 100644 --- a/Public/Sdk/Experimental/NodeJs/npm.dsc +++ b/Public/Sdk/Experimental/NodeJs/npm.dsc @@ -41,8 +41,13 @@ namespace Npm { }; } + export interface NpmInstallResult { + installDir: OpaqueDirectory; + newPackageLock: DerivedFile; + } + @@public - export function npmInstall(rootDir: StaticDirectory): OpaqueDirectory { + export function npmInstall(rootDir: StaticDirectory, packageLock: DerivedFile): NpmInstallResult { const wd = rootDir.root; const nodeModulesPath = d`${wd}/node_modules`; const npmCachePath = Context.getNewOutputDirectory('npm-install-cache'); @@ -50,6 +55,7 @@ namespace Npm { const arguments: Argument[] = [ Cmd.argument(Artifact.input(Node.npmCli)), Cmd.argument("install"), + Cmd.argument("--no-audit"), Cmd.option("--cache ", Artifact.none(npmCachePath)), // Forces the npm cache to use this output folder for this object so that it doesn't write to user folder ]; @@ -58,6 +64,7 @@ namespace Npm { workingDirectory: wd, dependencies: [ rootDir ], outputs: [ + { artifact: packageLock, existence: "required" }, // rewritten file in place { directory: wd, kind: "shared" }, npmCachePath, // Place the cache path as an output directory so it is cleaned each time. ], @@ -68,7 +75,10 @@ namespace Npm { ], }); - return result.getOutputDirectory(wd); + return { + installDir: result.getOutputDirectory(wd), + newPackageLock: result.getOutputFile(packageLock.path) + }; } @@public diff --git a/Public/Src/IDE/VsCode/BuildXL.IDE.VsCode.dsc b/Public/Src/IDE/VsCode/BuildXL.IDE.VsCode.dsc index 79035d7f2..7fabce0ac 100644 --- a/Public/Src/IDE/VsCode/BuildXL.IDE.VsCode.dsc +++ b/Public/Src/IDE/VsCode/BuildXL.IDE.VsCode.dsc @@ -18,21 +18,17 @@ namespace VsCode.Client { const clientCopy: OpaqueDirectory = Deployment.copyDirectory(clientSealDir.root, Context.getNewOutputDirectory("client-copy"), clientSealDir); + const copiedPackageLock = Transformer.copyFile( + f`${Context.getMount("CgNpmRoot").path}/package-lock.json`, + p`${clientCopy}/package-lock.json`); + @public - export const installRootDir: OpaqueDirectory = Npm.npmInstall(clientCopy); + export const npmInstallResult = Npm.npmInstall(clientCopy, copiedPackageLock); @@public - export const compileOutDir: OpaqueDirectory = Node.tscCompile(clientCopy.root, [clientCopy, installRootDir]); - - @@public - export const deployedNpmPackageLockFile = Deployment.copyFileFromOpaqueDirectory( - // Here we want to copy a file from inside an opaque output directory. - // If done using Transformer.copyFile we would have no way of specifying a dependency - // of that copy pip to the pip producing this opaque directory, so we wouldn't be able to - // ensure that the copy operation runs after the opaque directory is produced. - p`${installRootDir}/package-lock.json`, - p`${Context.getMount("CgNpmRoot").path}/package-lock.json`, - installRootDir); + export const compileOutDir: OpaqueDirectory = Node.tscCompile( + clientCopy.root, + [ clientCopy, npmInstallResult.installDir, npmInstallResult.newPackageLock ]); } namespace LanguageService.Server { @@ -115,7 +111,7 @@ namespace LanguageService.Server { }, { subfolder: a`node_modules`, - contents: [ Deployment.createDeployableOpaqueSubDirectory(VsCode.Client.installRootDir, r`node_modules`) ] + contents: [ Deployment.createDeployableOpaqueSubDirectory(VsCode.Client.npmInstallResult.installDir, r`node_modules`) ] }, { subfolder: a`out`, diff --git a/Shared/Scripts/bxl.ps1 b/Shared/Scripts/bxl.ps1 index fabc708b0..5229bab8a 100644 --- a/Shared/Scripts/bxl.ps1 +++ b/Shared/Scripts/bxl.ps1 @@ -560,7 +560,7 @@ if (!$skipFilter){ if ($Minimal) { # filtering by core deployment. - $AdditionalBuildXLArguments += "/f:(output='$($useDeployment.buildDir)\*'or(output='out\bin\$DeployConfig\Sdk\*')or($CacheOutputFilter))and~($CacheLongRunningFilter)ortag='protobufgenerator'oroutput='cg\*'" + $AdditionalBuildXLArguments += "/f:(output='$($useDeployment.buildDir)\*'or(output='out\bin\$DeployConfig\Sdk\*')or($CacheOutputFilter))and~($CacheLongRunningFilter)ortag='protobufgenerator'" } if ($Cache) { diff --git a/config.dsc b/config.dsc index d17198a8e..43b9fb6b3 100644 --- a/config.dsc +++ b/config.dsc @@ -643,7 +643,7 @@ config({ name: a`CgNpmRoot`, path: p`cg/npm`, trackSourceFileChanges: true, - isWritable: true, + isWritable: false, isReadable: true }, {