From 508429e586535daf0626e6eb02b3217703ca6714 Mon Sep 17 00:00:00 2001 From: David Michon Date: Thu, 26 Sep 2024 22:02:56 +0000 Subject: [PATCH 1/6] [heft] Update file copy layer to support incremental disk cache --- .../operations/runners/TaskOperationRunner.ts | 1 + .../pluginFramework/IncrementalBuildInfo.ts | 156 ++++++++++++++++++ apps/heft/src/plugins/CopyFilesPlugin.ts | 75 ++++++++- .../config/rush-project.json | 3 +- 4 files changed, 231 insertions(+), 4 deletions(-) create mode 100644 apps/heft/src/pluginFramework/IncrementalBuildInfo.ts diff --git a/apps/heft/src/operations/runners/TaskOperationRunner.ts b/apps/heft/src/operations/runners/TaskOperationRunner.ts index 4fbe1b98462..66274a696a1 100644 --- a/apps/heft/src/operations/runners/TaskOperationRunner.ts +++ b/apps/heft/src/operations/runners/TaskOperationRunner.ts @@ -183,6 +183,7 @@ export class TaskOperationRunner implements IOperationRunner { ? copyFilesAsync( copyOperations, logger.terminal, + `${taskSession.tempFolderPath}/file-copy.json`, isWatchMode ? getWatchFileSystemAdapter() : undefined ) : Promise.resolve(), diff --git a/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts b/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts new file mode 100644 index 00000000000..2b36d198420 --- /dev/null +++ b/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts @@ -0,0 +1,156 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import * as path from 'path'; + +import { FileSystem, Path } from '@rushstack/node-core-library'; + +/** + * Information about an incremental build. This information is used to determine which files need to be rebuilt. + * @beta + */ +export interface IIncrementalBuildInfo { + /** + * A string that represents the configuration inputs for the build. + * If the configuration changes, the old build info object should be discarded. + */ + configHash: string; + + /** + * A map of absolute input file paths to their version strings. + * The version string should change if the file changes. + */ + inputFileVersions: Map; + + /** + * A map of absolute output file paths to the input files they were computed from. + */ + fileDependencies?: Map; +} + +/** + * Serialized version of {@link IIncrementalBuildInfo}. + * @beta + */ +export interface ISerializedIncrementalBuildInfo { + /** + * A string that represents the configuration inputs for the build. + * If the configuration changes, the old build info object should be discarded. + */ + configHash: string; + + /** + * A map of input files to their version strings. + * File paths are specified relative to the folder containing the build info file. + */ + inputFileVersions: [string, string][]; + + /** + * Map of output file names to the input file indices used to compute them. + * File paths are specified relative to the folder containing the build info file. + */ + fileDependencies?: [string, number | number[]][]; +} + +/** + * Writes a build info object to disk. + * @param state - The build info to write + * @param filePath - The file path to write the build info to + * @beta + */ +export async function writeBuildInfoAsync(state: IIncrementalBuildInfo, filePath: string): Promise { + const fileIndices: Map = new Map(); + const inputFileVersions: [string, string][] = []; + const directory: string = path.dirname(filePath); + + for (const [absolutePath, version] of state.inputFileVersions) { + const relativePath: string = Path.convertToSlashes(path.relative(directory, absolutePath)); + fileIndices.set(absolutePath, fileIndices.size); + inputFileVersions.push([relativePath, version]); + } + + const { fileDependencies: newFileDependencies } = state; + let fileDependencies: [string, number | number[]][] | undefined; + if (newFileDependencies) { + fileDependencies = []; + for (const [absolutePath, dependencies] of newFileDependencies) { + const relativePath: string = Path.convertToSlashes(path.relative(directory, absolutePath)); + const indices: number[] = []; + for (const dependency of dependencies) { + const index: number | undefined = fileIndices.get(dependency); + if (index === undefined) { + throw new Error(`Dependency not found: ${dependency}`); + } + indices.push(index); + } + + fileDependencies.push([relativePath, indices.length === 1 ? indices[0] : indices]); + } + } + + const serializedBuildInfo: ISerializedIncrementalBuildInfo = { + configHash: state.configHash, + inputFileVersions, + fileDependencies + }; + + // This file is meant only for machine reading, so don't pretty-print it. + const stringified: string = JSON.stringify(serializedBuildInfo); + + await FileSystem.writeFileAsync(filePath, stringified, { ensureFolderExists: true }); +} + +/** + * Reads a build info object from disk. + * @param filePath - The file path to read the build info from + * @returns The build info object, or undefined if the file does not exist or cannot be parsed + * @beta + */ +export async function tryReadBuildInfoAsync(filePath: string): Promise { + let serializedBuildInfo: ISerializedIncrementalBuildInfo | undefined; + try { + const fileContents: string = await FileSystem.readFileAsync(filePath); + serializedBuildInfo = JSON.parse(fileContents) as ISerializedIncrementalBuildInfo; + } catch (error) { + if (FileSystem.isNotExistError(error)) { + return; + } + throw error; + } + + const dirname: string = path.dirname(filePath); + + const inputFileVersions: Map = new Map(); + const absolutePathByIndex: string[] = []; + for (const [relativePath, version] of serializedBuildInfo.inputFileVersions) { + const absolutePath: string = path.resolve(dirname, relativePath); + absolutePathByIndex.push(absolutePath); + inputFileVersions.set(absolutePath, version); + } + + let fileDependencies: Map | undefined; + const { fileDependencies: serializedFileDependencies } = serializedBuildInfo; + if (serializedFileDependencies) { + fileDependencies = new Map(); + for (const [relativeOutputFile, indices] of serializedFileDependencies) { + const absoluteOutputFile: string = path.resolve(dirname, relativeOutputFile); + const dependencies: string[] = []; + for (const index of Array.isArray(indices) ? indices : [indices]) { + const dependencyAbsolutePath: string | undefined = absolutePathByIndex[index]; + if (dependencyAbsolutePath === undefined) { + throw new Error(`Dependency index not found: ${index}`); + } + dependencies.push(dependencyAbsolutePath); + } + fileDependencies.set(absoluteOutputFile, dependencies); + } + } + + const buildInfo: IIncrementalBuildInfo = { + configHash: serializedBuildInfo.configHash, + inputFileVersions, + fileDependencies + }; + + return buildInfo; +} diff --git a/apps/heft/src/plugins/CopyFilesPlugin.ts b/apps/heft/src/plugins/CopyFilesPlugin.ts index e887452531b..cc7df95594c 100644 --- a/apps/heft/src/plugins/CopyFilesPlugin.ts +++ b/apps/heft/src/plugins/CopyFilesPlugin.ts @@ -1,8 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import { createHash, type Hash } from 'crypto'; import type * as fs from 'fs'; import * as path from 'path'; + import { AlreadyExistsBehavior, FileSystem, Async } from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; @@ -16,6 +18,11 @@ import type { HeftConfiguration } from '../configuration/HeftConfiguration'; import type { IHeftTaskPlugin } from '../pluginFramework/IHeftPlugin'; import type { IHeftTaskSession, IHeftTaskFileOperations } from '../pluginFramework/HeftTaskSession'; import type { WatchFileSystemAdapter } from '../utilities/WatchFileSystemAdapter'; +import { + type IIncrementalBuildInfo, + tryReadBuildInfoAsync, + writeBuildInfoAsync +} from '../pluginFramework/IncrementalBuildInfo'; /** * Used to specify a selection of files to copy from a specific source folder to one @@ -82,14 +89,21 @@ export function normalizeCopyOperation(rootFolderPath: string, copyOperation: IC export async function copyFilesAsync( copyOperations: Iterable, terminal: ITerminal, + buildInfoPath: string, watchFileSystemAdapter?: WatchFileSystemAdapter ): Promise { + const hasher: Hash = createHash('sha256'); + for (const copyOperation of copyOperations) { + hasher.update(JSON.stringify(copyOperation)); + } + const configHash: string = hasher.digest('base64'); + const copyDescriptorByDestination: Map = await _getCopyDescriptorsAsync( copyOperations, watchFileSystemAdapter ); - await _copyFilesInnerAsync(copyDescriptorByDestination, terminal); + await _copyFilesInnerAsync(copyDescriptorByDestination, configHash, buildInfoPath, terminal); } async function _getCopyDescriptorsAsync( @@ -158,16 +172,69 @@ async function _getCopyDescriptorsAsync( async function _copyFilesInnerAsync( copyDescriptors: Map, + configHash: string, + buildInfoPath: string, terminal: ITerminal ): Promise { if (copyDescriptors.size === 0) { return; } + let oldBuildInfo: IIncrementalBuildInfo | undefined = await tryReadBuildInfoAsync(buildInfoPath); + if (oldBuildInfo && oldBuildInfo.configHash !== configHash) { + terminal.writeVerboseLine(`File copy configuration changed, discarding incremental state.`); + oldBuildInfo = undefined; + } + + const inputFileVersions: Map = new Map(); + + const buildInfo: IIncrementalBuildInfo = { + configHash, + inputFileVersions + }; + + const allInputFiles: Set = new Set(); + for (const copyDescriptor of copyDescriptors.values()) { + allInputFiles.add(copyDescriptor.sourcePath); + } + + await Async.forEachAsync( + allInputFiles, + async (inputFilePath: string) => { + const fileContent: Buffer = await FileSystem.readFileToBufferAsync(inputFilePath); + const fileHash: string = createHash('sha256').update(fileContent).digest('base64'); + inputFileVersions.set(inputFilePath, fileHash); + }, + { + concurrency: Constants.maxParallelism + } + ); + + const copyDescriptorsWithWork: ICopyDescriptor[] = []; + for (const copyDescriptor of copyDescriptors.values()) { + const { sourcePath } = copyDescriptor; + + const sourceFileHash: string | undefined = inputFileVersions.get(sourcePath); + if (!sourceFileHash) { + throw new Error(`Missing hash for input file: ${sourcePath}`); + } + + if (oldBuildInfo?.inputFileVersions.get(sourcePath) === sourceFileHash) { + continue; + } + + copyDescriptorsWithWork.push(copyDescriptor); + } + + if (copyDescriptorsWithWork.length === 0) { + terminal.writeLine('All requested file copy operations are up to date. Nothing to do.'); + return; + } + let copiedFolderOrFileCount: number = 0; let linkedFileCount: number = 0; await Async.forEachAsync( - copyDescriptors.values(), + copyDescriptorsWithWork, async (copyDescriptor: ICopyDescriptor) => { if (copyDescriptor.hardlink) { linkedFileCount++; @@ -196,9 +263,11 @@ async function _copyFilesInnerAsync( const folderOrFilesPlural: string = copiedFolderOrFileCount === 1 ? '' : 's'; terminal.writeLine( - `Copied ${copiedFolderOrFileCount} folder${folderOrFilesPlural} or file${folderOrFilesPlural} and ` + + `Copied ${copiedFolderOrFileCount} file${folderOrFilesPlural} and ` + `linked ${linkedFileCount} file${linkedFileCount === 1 ? '' : 's'}` ); + + await writeBuildInfoAsync(buildInfo, buildInfoPath); } const PLUGIN_NAME: 'copy-files-plugin' = 'copy-files-plugin'; diff --git a/build-tests/heft-copy-files-test/config/rush-project.json b/build-tests/heft-copy-files-test/config/rush-project.json index 4a1a3f04ef5..c7051638984 100644 --- a/build-tests/heft-copy-files-test/config/rush-project.json +++ b/build-tests/heft-copy-files-test/config/rush-project.json @@ -13,7 +13,8 @@ "out-images2", "out-images3", "out-images4", - "out-images5" + "out-images5", + "temp/build" ] } ] From 1a5020c49dfc81530ea6b483be3c66973a1a30c4 Mon Sep 17 00:00:00 2001 From: David Michon Date: Thu, 26 Sep 2024 22:16:27 +0000 Subject: [PATCH 2/6] rush change --- .../heft/copy-file-if-changed_2024-09-26-22-16.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@rushstack/heft/copy-file-if-changed_2024-09-26-22-16.json diff --git a/common/changes/@rushstack/heft/copy-file-if-changed_2024-09-26-22-16.json b/common/changes/@rushstack/heft/copy-file-if-changed_2024-09-26-22-16.json new file mode 100644 index 00000000000..5939542e2f8 --- /dev/null +++ b/common/changes/@rushstack/heft/copy-file-if-changed_2024-09-26-22-16.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/heft", + "comment": "Update file copy logic to use an incremental cache file in the temp directory for the current task to avoid unnecessary file writes.", + "type": "minor" + } + ], + "packageName": "@rushstack/heft" +} \ No newline at end of file From 8b27f1851d3fbdd31a58447a6536ba6968e8a50e Mon Sep 17 00:00:00 2001 From: David Michon Date: Thu, 26 Sep 2024 22:52:49 +0000 Subject: [PATCH 3/6] (chore) Remove references to copying folders --- apps/heft/src/plugins/CopyFilesPlugin.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/heft/src/plugins/CopyFilesPlugin.ts b/apps/heft/src/plugins/CopyFilesPlugin.ts index cc7df95594c..329c466ca3d 100644 --- a/apps/heft/src/plugins/CopyFilesPlugin.ts +++ b/apps/heft/src/plugins/CopyFilesPlugin.ts @@ -231,7 +231,7 @@ async function _copyFilesInnerAsync( return; } - let copiedFolderOrFileCount: number = 0; + let copiedFileCount: number = 0; let linkedFileCount: number = 0; await Async.forEachAsync( copyDescriptorsWithWork, @@ -247,7 +247,7 @@ async function _copyFilesInnerAsync( `Linked "${copyDescriptor.sourcePath}" to "${copyDescriptor.destinationPath}".` ); } else { - copiedFolderOrFileCount++; + copiedFileCount++; await FileSystem.copyFilesAsync({ sourcePath: copyDescriptor.sourcePath, destinationPath: copyDescriptor.destinationPath, @@ -261,9 +261,8 @@ async function _copyFilesInnerAsync( { concurrency: Constants.maxParallelism } ); - const folderOrFilesPlural: string = copiedFolderOrFileCount === 1 ? '' : 's'; terminal.writeLine( - `Copied ${copiedFolderOrFileCount} file${folderOrFilesPlural} and ` + + `Copied ${copiedFileCount} file${copiedFileCount === 1 ? '' : 's'} and ` + `linked ${linkedFileCount} file${linkedFileCount === 1 ? '' : 's'}` ); From ffea9a717f9643fced578156d78fb7b6d8b8090b Mon Sep 17 00:00:00 2001 From: David Michon Date: Fri, 27 Sep 2024 21:49:03 +0000 Subject: [PATCH 4/6] Use object format, add unit tests --- .../pluginFramework/IncrementalBuildInfo.ts | 122 ++++++++++++------ .../tests/IncrementalBuildInfo.test.ts | 111 ++++++++++++++++ .../IncrementalBuildInfo.test.ts.snap | 35 +++++ 3 files changed, 228 insertions(+), 40 deletions(-) create mode 100644 apps/heft/src/pluginFramework/tests/IncrementalBuildInfo.test.ts create mode 100644 apps/heft/src/pluginFramework/tests/__snapshots__/IncrementalBuildInfo.test.ts.snap diff --git a/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts b/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts index 2b36d198420..e0619f0ad91 100644 --- a/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts +++ b/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts @@ -43,38 +43,41 @@ export interface ISerializedIncrementalBuildInfo { * A map of input files to their version strings. * File paths are specified relative to the folder containing the build info file. */ - inputFileVersions: [string, string][]; + inputFileVersions: Record; /** * Map of output file names to the input file indices used to compute them. * File paths are specified relative to the folder containing the build info file. */ - fileDependencies?: [string, number | number[]][]; + fileDependencies?: Record; } /** - * Writes a build info object to disk. - * @param state - The build info to write - * @param filePath - The file path to write the build info to + * Serializes a build info object to a portable format that can be written to disk. + * @param state - The build info to serialize + * @param makePathPortable - A function that converts an absolute path to a portable path + * @returns The serialized build info * @beta */ -export async function writeBuildInfoAsync(state: IIncrementalBuildInfo, filePath: string): Promise { +export function serializeBuildInfo( + state: IIncrementalBuildInfo, + makePathPortable: (absolutePath: string) => string +): ISerializedIncrementalBuildInfo { const fileIndices: Map = new Map(); - const inputFileVersions: [string, string][] = []; - const directory: string = path.dirname(filePath); + const inputFileVersions: Record = {}; for (const [absolutePath, version] of state.inputFileVersions) { - const relativePath: string = Path.convertToSlashes(path.relative(directory, absolutePath)); + const relativePath: string = makePathPortable(absolutePath); fileIndices.set(absolutePath, fileIndices.size); - inputFileVersions.push([relativePath, version]); + inputFileVersions[relativePath] = version; } const { fileDependencies: newFileDependencies } = state; - let fileDependencies: [string, number | number[]][] | undefined; + let fileDependencies: Record | undefined; if (newFileDependencies) { - fileDependencies = []; + fileDependencies = {}; for (const [absolutePath, dependencies] of newFileDependencies) { - const relativePath: string = Path.convertToSlashes(path.relative(directory, absolutePath)); + const relativePath: string = makePathPortable(absolutePath); const indices: number[] = []; for (const dependency of dependencies) { const index: number | undefined = fileIndices.get(dependency); @@ -84,7 +87,7 @@ export async function writeBuildInfoAsync(state: IIncrementalBuildInfo, filePath indices.push(index); } - fileDependencies.push([relativePath, indices.length === 1 ? indices[0] : indices]); + fileDependencies[relativePath] = indices.length === 1 ? indices[0] : indices; } } @@ -94,36 +97,23 @@ export async function writeBuildInfoAsync(state: IIncrementalBuildInfo, filePath fileDependencies }; - // This file is meant only for machine reading, so don't pretty-print it. - const stringified: string = JSON.stringify(serializedBuildInfo); - - await FileSystem.writeFileAsync(filePath, stringified, { ensureFolderExists: true }); + return serializedBuildInfo; } /** - * Reads a build info object from disk. - * @param filePath - The file path to read the build info from - * @returns The build info object, or undefined if the file does not exist or cannot be parsed - * @beta + * Deserializes a build info object from its portable format. + * @param serializedBuildInfo - The build info to deserialize + * @param makePathAbsolute - A function that converts a portable path to an absolute path + * @returns The deserialized build info */ -export async function tryReadBuildInfoAsync(filePath: string): Promise { - let serializedBuildInfo: ISerializedIncrementalBuildInfo | undefined; - try { - const fileContents: string = await FileSystem.readFileAsync(filePath); - serializedBuildInfo = JSON.parse(fileContents) as ISerializedIncrementalBuildInfo; - } catch (error) { - if (FileSystem.isNotExistError(error)) { - return; - } - throw error; - } - - const dirname: string = path.dirname(filePath); - +export function deserializeBuildInfo( + serializedBuildInfo: ISerializedIncrementalBuildInfo, + makePathAbsolute: (relativePath: string) => string +): IIncrementalBuildInfo { const inputFileVersions: Map = new Map(); const absolutePathByIndex: string[] = []; - for (const [relativePath, version] of serializedBuildInfo.inputFileVersions) { - const absolutePath: string = path.resolve(dirname, relativePath); + for (const [relativePath, version] of Object.entries(serializedBuildInfo.inputFileVersions)) { + const absolutePath: string = makePathAbsolute(relativePath); absolutePathByIndex.push(absolutePath); inputFileVersions.set(absolutePath, version); } @@ -132,8 +122,8 @@ export async function tryReadBuildInfoAsync(filePath: string): Promise { + const basePath: string = path.dirname(filePath); + + const serializedBuildInfo: ISerializedIncrementalBuildInfo = serializeBuildInfo( + state, + (absolutePath: string) => { + return Path.convertToSlashes(path.relative(basePath, absolutePath)); + } + ); + + // This file is meant only for machine reading, so don't pretty-print it. + const stringified: string = JSON.stringify(serializedBuildInfo); + + await FileSystem.writeFileAsync(filePath, stringified, { ensureFolderExists: true }); +} + +/** + * Reads a build info object from disk. + * @param filePath - The file path to read the build info from + * @returns The build info object, or undefined if the file does not exist or cannot be parsed + * @beta + */ +export async function tryReadBuildInfoAsync(filePath: string): Promise { + let serializedBuildInfo: ISerializedIncrementalBuildInfo | undefined; + try { + const fileContents: string = await FileSystem.readFileAsync(filePath); + serializedBuildInfo = JSON.parse(fileContents) as ISerializedIncrementalBuildInfo; + } catch (error) { + if (FileSystem.isNotExistError(error)) { + return; + } + throw error; + } + + const basePath: string = path.dirname(filePath); + + const buildInfo: IIncrementalBuildInfo = deserializeBuildInfo( + serializedBuildInfo, + (relativePath: string) => { + return path.resolve(basePath, relativePath); + } + ); + + return buildInfo; +} diff --git a/apps/heft/src/pluginFramework/tests/IncrementalBuildInfo.test.ts b/apps/heft/src/pluginFramework/tests/IncrementalBuildInfo.test.ts new file mode 100644 index 00000000000..6704b6a7d28 --- /dev/null +++ b/apps/heft/src/pluginFramework/tests/IncrementalBuildInfo.test.ts @@ -0,0 +1,111 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import path from 'path'; + +import { Path } from '@rushstack/node-core-library'; + +import { + serializeBuildInfo, + deserializeBuildInfo, + type IIncrementalBuildInfo, + type ISerializedIncrementalBuildInfo +} from '../IncrementalBuildInfo'; + +const posixBuildInfo: IIncrementalBuildInfo = { + configHash: 'foobar', + inputFileVersions: new Map([ + ['/a/b/c/file1', '1'], + ['/a/b/c/file2', '2'] + ]), + fileDependencies: new Map([ + ['/a/b/c/output1', ['/a/b/c/file1']], + ['/a/b/c/output2', ['/a/b/c/file1', '/a/b/c/file2']] + ]) +}; + +const win32BuildInfo: IIncrementalBuildInfo = { + configHash: 'foobar', + inputFileVersions: new Map([ + ['A:\\b\\c\\file1', '1'], + ['A:\\b\\c\\file2', '2'] + ]), + fileDependencies: new Map([ + ['A:\\b\\c\\output1', ['A:\\b\\c\\file1']], + ['A:\\b\\c\\output2', ['A:\\b\\c\\file1', 'A:\\b\\c\\file2']] + ]) +}; + +const posixBasePath: string = '/a/b/temp'; +const win32BasePath: string = 'A:\\b\\temp'; + +function posixToPortable(absolutePath: string): string { + return path.posix.relative(posixBasePath, absolutePath); +} +function portableToPosix(portablePath: string): string { + return path.posix.resolve(posixBasePath, portablePath); +} + +function win32ToPortable(absolutePath: string): string { + return Path.convertToSlashes(path.win32.relative(win32BasePath, absolutePath)); +} +function portableToWin32(portablePath: string): string { + return path.win32.resolve(win32BasePath, portablePath); +} + +describe(serializeBuildInfo.name, () => { + it('Round trips correctly (POSIX)', () => { + const serialized: ISerializedIncrementalBuildInfo = serializeBuildInfo(posixBuildInfo, posixToPortable); + + const deserialized: IIncrementalBuildInfo = deserializeBuildInfo(serialized, portableToPosix); + + expect(deserialized).toEqual(posixBuildInfo); + }); + + it('Round trips correctly (Win32)', () => { + function makePathPortable(absolutePath: string): string { + return Path.convertToSlashes(path.win32.relative(win32BasePath, absolutePath)); + } + function makePathAbsolute(portablePath: string): string { + return path.win32.resolve(win32BasePath, portablePath); + } + + const serialized: ISerializedIncrementalBuildInfo = serializeBuildInfo(win32BuildInfo, makePathPortable); + + const deserialized: IIncrementalBuildInfo = deserializeBuildInfo(serialized, makePathAbsolute); + + expect(deserialized).toEqual(win32BuildInfo); + }); + + it('Converts (POSIX to Win32)', () => { + const serialized: ISerializedIncrementalBuildInfo = serializeBuildInfo(posixBuildInfo, posixToPortable); + + const deserialized: IIncrementalBuildInfo = deserializeBuildInfo(serialized, portableToWin32); + + expect(deserialized).toEqual(win32BuildInfo); + }); + + it('Converts (Win32 to POSIX)', () => { + const serialized: ISerializedIncrementalBuildInfo = serializeBuildInfo(win32BuildInfo, win32ToPortable); + + const deserialized: IIncrementalBuildInfo = deserializeBuildInfo(serialized, portableToPosix); + + expect(deserialized).toEqual(posixBuildInfo); + }); + + it('Has expected serialized format', () => { + const serializedPosix: ISerializedIncrementalBuildInfo = serializeBuildInfo( + posixBuildInfo, + posixToPortable + ); + const serializedWin32: ISerializedIncrementalBuildInfo = serializeBuildInfo( + win32BuildInfo, + win32ToPortable + ); + + expect(serializedPosix).toMatchSnapshot('posix'); + expect(serializedWin32).toMatchSnapshot('win32'); + + expect(serializedPosix).toEqual(serializedWin32); + }); +}); diff --git a/apps/heft/src/pluginFramework/tests/__snapshots__/IncrementalBuildInfo.test.ts.snap b/apps/heft/src/pluginFramework/tests/__snapshots__/IncrementalBuildInfo.test.ts.snap new file mode 100644 index 00000000000..abc236eee5c --- /dev/null +++ b/apps/heft/src/pluginFramework/tests/__snapshots__/IncrementalBuildInfo.test.ts.snap @@ -0,0 +1,35 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`serializeBuildInfo Has expected serialized format: posix 1`] = ` +Object { + "configHash": "foobar", + "fileDependencies": Object { + "../c/output1": 0, + "../c/output2": Array [ + 0, + 1, + ], + }, + "inputFileVersions": Object { + "../c/file1": "1", + "../c/file2": "2", + }, +} +`; + +exports[`serializeBuildInfo Has expected serialized format: win32 1`] = ` +Object { + "configHash": "foobar", + "fileDependencies": Object { + "../c/output1": 0, + "../c/output2": Array [ + 0, + 1, + ], + }, + "inputFileVersions": Object { + "../c/file1": "1", + "../c/file2": "2", + }, +} +`; From 5c004aea7de2ded7fcc4cbf9aea23f8a17a8659f Mon Sep 17 00:00:00 2001 From: David Michon Date: Fri, 27 Sep 2024 23:02:48 +0000 Subject: [PATCH 5/6] Always use array --- .../pluginFramework/IncrementalBuildInfo.ts | 28 ++++++++++++++----- .../IncrementalBuildInfo.test.ts.snap | 8 ++++-- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts b/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts index e0619f0ad91..400d5b89fa6 100644 --- a/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts +++ b/apps/heft/src/pluginFramework/IncrementalBuildInfo.ts @@ -46,16 +46,30 @@ export interface ISerializedIncrementalBuildInfo { inputFileVersions: Record; /** - * Map of output file names to the input file indices used to compute them. + * Map of output file names to the corresponding index in `Object.entries(inputFileVersions)`. * File paths are specified relative to the folder containing the build info file. */ - fileDependencies?: Record; + fileDependencies?: Record; } +/** + * Converts an absolute path to a path relative to a base path. + */ +const makePathRelative: (absolutePath: string, basePath: string) => string = + process.platform === 'win32' + ? (absolutePath: string, basePath: string) => { + // On Windows, need to normalize slashes + return Path.convertToSlashes(path.win32.relative(basePath, absolutePath)); + } + : (absolutePath: string, basePath: string) => { + // On POSIX, can preserve existing slashes + return path.posix.relative(basePath, absolutePath); + }; + /** * Serializes a build info object to a portable format that can be written to disk. * @param state - The build info to serialize - * @param makePathPortable - A function that converts an absolute path to a portable path + * @param makePathPortable - A function that converts an absolute path to a portable path. This is a separate argument to support cross-platform tests. * @returns The serialized build info * @beta */ @@ -73,7 +87,7 @@ export function serializeBuildInfo( } const { fileDependencies: newFileDependencies } = state; - let fileDependencies: Record | undefined; + let fileDependencies: Record | undefined; if (newFileDependencies) { fileDependencies = {}; for (const [absolutePath, dependencies] of newFileDependencies) { @@ -87,7 +101,7 @@ export function serializeBuildInfo( indices.push(index); } - fileDependencies[relativePath] = indices.length === 1 ? indices[0] : indices; + fileDependencies[relativePath] = indices; } } @@ -103,7 +117,7 @@ export function serializeBuildInfo( /** * Deserializes a build info object from its portable format. * @param serializedBuildInfo - The build info to deserialize - * @param makePathAbsolute - A function that converts a portable path to an absolute path + * @param makePathAbsolute - A function that converts a portable path to an absolute path. This is a separate argument to support cross-platform tests. * @returns The deserialized build info */ export function deserializeBuildInfo( @@ -157,7 +171,7 @@ export async function writeBuildInfoAsync(state: IIncrementalBuildInfo, filePath const serializedBuildInfo: ISerializedIncrementalBuildInfo = serializeBuildInfo( state, (absolutePath: string) => { - return Path.convertToSlashes(path.relative(basePath, absolutePath)); + return makePathRelative(basePath, absolutePath); } ); diff --git a/apps/heft/src/pluginFramework/tests/__snapshots__/IncrementalBuildInfo.test.ts.snap b/apps/heft/src/pluginFramework/tests/__snapshots__/IncrementalBuildInfo.test.ts.snap index abc236eee5c..390725566a9 100644 --- a/apps/heft/src/pluginFramework/tests/__snapshots__/IncrementalBuildInfo.test.ts.snap +++ b/apps/heft/src/pluginFramework/tests/__snapshots__/IncrementalBuildInfo.test.ts.snap @@ -4,7 +4,9 @@ exports[`serializeBuildInfo Has expected serialized format: posix 1`] = ` Object { "configHash": "foobar", "fileDependencies": Object { - "../c/output1": 0, + "../c/output1": Array [ + 0, + ], "../c/output2": Array [ 0, 1, @@ -21,7 +23,9 @@ exports[`serializeBuildInfo Has expected serialized format: win32 1`] = ` Object { "configHash": "foobar", "fileDependencies": Object { - "../c/output1": 0, + "../c/output1": Array [ + 0, + ], "../c/output2": Array [ 0, 1, From 88686a4d2c9fa251c083437881be458cd22814bc Mon Sep 17 00:00:00 2001 From: David Michon Date: Fri, 27 Sep 2024 23:04:26 +0000 Subject: [PATCH 6/6] Update unit test --- .../tests/IncrementalBuildInfo.test.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/apps/heft/src/pluginFramework/tests/IncrementalBuildInfo.test.ts b/apps/heft/src/pluginFramework/tests/IncrementalBuildInfo.test.ts index 6704b6a7d28..e2921fb22e3 100644 --- a/apps/heft/src/pluginFramework/tests/IncrementalBuildInfo.test.ts +++ b/apps/heft/src/pluginFramework/tests/IncrementalBuildInfo.test.ts @@ -63,16 +63,9 @@ describe(serializeBuildInfo.name, () => { }); it('Round trips correctly (Win32)', () => { - function makePathPortable(absolutePath: string): string { - return Path.convertToSlashes(path.win32.relative(win32BasePath, absolutePath)); - } - function makePathAbsolute(portablePath: string): string { - return path.win32.resolve(win32BasePath, portablePath); - } - - const serialized: ISerializedIncrementalBuildInfo = serializeBuildInfo(win32BuildInfo, makePathPortable); + const serialized: ISerializedIncrementalBuildInfo = serializeBuildInfo(win32BuildInfo, win32ToPortable); - const deserialized: IIncrementalBuildInfo = deserializeBuildInfo(serialized, makePathAbsolute); + const deserialized: IIncrementalBuildInfo = deserializeBuildInfo(serialized, portableToWin32); expect(deserialized).toEqual(win32BuildInfo); });