Skip to content

Commit 06fdb1b

Browse files
iowillhoitmshanemcsvc-cli-bot
authored
feat: detect file moves in source tracking
* chore: clean up * fix: ignore type linting * chore: merge main, fix types * chore: bump deps for xnuts * chore(release): 6.0.5-dev.0 [skip ci] * chore: bump deps * fix: add performance markers * chore: bump deps * chore(release): 6.0.5-dev.1 [skip ci] * chore: pjson cleanup * chore: bump deps for xnuts * chore: early exits * chore: check includes first * chore: promise all getInfo * feat: add env var to skip moved file detection * chore: use sets for quick lookup * chore: slight perf improvment using set in getInfo - 5 percent * chore: emit telem if file move detection is skipped * refactor: rework how we compare matches * chore: const name * refactor: more functions! * chore(release): 6.0.5-dev.2 [skip ci] * chore: tslint * chore: clean up * test: temp logs * test: temp logs * test: more temp logs * test: temp logs * refactor: registry from STL * chore: debugging windows * chore: remove logs * fix: fix file move debug * chore: testing windows filepaths * refactor: win paths * chore: return order consistency * chore: bump deps for xNuts * chore(release): 6.0.5-dev.3 [skip ci] * chore: bump deps for xNuts * chore(release): 6.0.5-dev.4 [skip ci] * chore: opt-in with beta env var --------- Co-authored-by: mshanemc <shane.mclaughlin@salesforce.com> Co-authored-by: svc-cli-bot <Svc_cli_bot@salesforce.com>
1 parent 10f9a7d commit 06fdb1b

15 files changed

+945
-94
lines changed

package.json

+11-6
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@salesforce/source-tracking",
33
"description": "API for tracking local and remote Salesforce metadata changes",
4-
"version": "6.0.4",
4+
"version": "6.0.5-dev.4",
55
"author": "Salesforce",
66
"license": "BSD-3-Clause",
77
"main": "lib/index.js",
@@ -23,6 +23,11 @@
2323
"test": "wireit",
2424
"test:nuts": "nyc mocha \"**/*.nut.ts\" --slow 4500 --timeout 600000 --parallel",
2525
"test:nuts:local": "mocha \"**/local/*.nut.ts\" --slow 4500 --timeout 600000 --parallel",
26+
"test:nuts:local:moved": "mocha \"**/nuts/local/localTrackingFileMoves*.nut.ts\" --slow 4500 --timeout 600000 --parallel",
27+
"test:nuts:local:moved:scale": "mocha \"**/nuts/local/localTrackingFileMovesScale.nut.ts\" --slow 4500 --timeout 600000 --parallel",
28+
"test:nuts:local:moved:image": "mocha \"**/nuts/local/localTrackingFileMovesImage.nut.ts\" --slow 4500 --timeout 600000 --parallel",
29+
"test:nuts:local:moved:child": "mocha \"**/nuts/local/localTrackingFileMovesDecomposedChild.nut.ts\" --slow 4500 --timeout 600000 --parallel",
30+
"test:unit:local:moved": "mocha \"test/unit/localDetectMovedFiles.test.ts\" --slow 4500 --timeout 600000",
2631
"test:only": "wireit"
2732
},
2833
"keywords": [
@@ -44,21 +49,21 @@
4449
"node": ">=18.0.0"
4550
},
4651
"dependencies": {
47-
"@oclif/core": "^3.26.4",
48-
"@salesforce/core": "^7.3.0",
52+
"@oclif/core": "^3.26.5",
53+
"@salesforce/core": "^7.3.5",
4954
"@salesforce/kit": "^3.1.1",
50-
"@salesforce/source-deploy-retrieve": "^11.0.1",
55+
"@salesforce/source-deploy-retrieve": "^11.4.4",
5156
"@salesforce/ts-types": "^2.0.9",
5257
"fast-xml-parser": "^4.3.6",
5358
"graceful-fs": "^4.2.11",
5459
"isomorphic-git": "1.23.0",
5560
"ts-retry-promise": "^0.8.0"
5661
},
5762
"devDependencies": {
58-
"@salesforce/cli-plugins-testkit": "^5.2.1",
63+
"@salesforce/cli-plugins-testkit": "^5.3.2",
5964
"@salesforce/dev-scripts": "^9.0.0",
6065
"@types/graceful-fs": "^4.1.9",
61-
"eslint-plugin-sf-plugin": "^1.18.1",
66+
"eslint-plugin-sf-plugin": "^1.18.3",
6267
"ts-node": "^10.9.2",
6368
"ts-patch": "^3.1.2",
6469
"typescript": "^5.4.5"

src/shared/localShadowRepo.ts

+240-11
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,19 @@
88
import path from 'node:path';
99
import * as os from 'node:os';
1010
import * as fs from 'graceful-fs';
11-
import { NamedPackageDir, Logger, SfError } from '@salesforce/core';
11+
import { NamedPackageDir, Lifecycle, Logger, SfError } from '@salesforce/core';
1212
import { env } from '@salesforce/kit';
1313
// @ts-expect-error isogit has both ESM and CJS exports but node16 module/resolution identifies it as ESM
14-
import git from 'isomorphic-git';
14+
import git, { Walker } from 'isomorphic-git';
1515
import { Performance } from '@oclif/core';
16+
import {
17+
RegistryAccess,
18+
MetadataResolver,
19+
VirtualTreeContainer,
20+
SourceComponent,
21+
} from '@salesforce/source-deploy-retrieve';
1622
import { chunkArray, excludeLwcLocalOnlyTest, folderContainsPath } from './functions';
23+
import { sourceComponentGuard } from './guards';
1724

1825
/** returns the full path to where we store the shadow repo */
1926
const getGitDir = (orgId: string, projectPath: string): string =>
@@ -35,10 +42,15 @@ const redirectToCliRepoError = (e: unknown): never => {
3542
throw e;
3643
};
3744

45+
type FileInfo = { filename: string; hash: string; basename: string };
46+
type StringMap = Map<string, string>;
47+
type AddAndDeleteMaps = { addedMap: StringMap; deletedMap: StringMap };
48+
3849
type ShadowRepoOptions = {
3950
orgId: string;
4051
projectPath: string;
4152
packageDirs: NamedPackageDir[];
53+
registry: RegistryAccess;
4254
};
4355

4456
// https://isomorphic-git.org/docs/en/statusMatrix#docsNav
@@ -48,6 +60,7 @@ type StatusRow = [file: string, head: number, workdir: number, stage: number];
4860
const FILE = 0;
4961
const HEAD = 1;
5062
const WORKDIR = 2;
63+
// We don't use STAGE (StatusRow[3]). Changes are added and committed in one step
5164

5265
type CommitRequest = {
5366
deployedFiles?: string[];
@@ -66,6 +79,7 @@ export class ShadowRepo {
6679
private status!: StatusRow[];
6780
private logger!: Logger;
6881
private readonly isWindows: boolean;
82+
private readonly registry: RegistryAccess;
6983

7084
/** do not try to add more than this many files at a time through isogit. You'll hit EMFILE: too many open files even with graceful-fs */
7185
private readonly maxFileAdd: number;
@@ -75,6 +89,7 @@ export class ShadowRepo {
7589
this.projectPath = options.projectPath;
7690
this.packageDirs = options.packageDirs;
7791
this.isWindows = os.type() === 'Windows_NT';
92+
this.registry = options.registry;
7893

7994
this.maxFileAdd = env.getNumber(
8095
'SF_SOURCE_TRACKING_BATCH_SIZE',
@@ -165,6 +180,16 @@ export class ShadowRepo {
165180
// isogit uses `startsWith` for filepaths so it's possible to get a false positive
166181
pkgDirs.some(folderContainsPath(f)),
167182
});
183+
184+
// Check for moved files and update local git status accordingly
185+
if (env.getBoolean('SF_BETA_TRACK_FILE_MOVES') === true) {
186+
await Lifecycle.getInstance().emitTelemetry({ eventName: 'moveFileDetectionEnabled' });
187+
await this.detectMovedFiles();
188+
} else {
189+
// Adding this telemetry for easier tracking of how many users are using the beta feature
190+
// This telemetry even will remain when the feature is GA and we switch to opt-out
191+
await Lifecycle.getInstance().emitTelemetry({ eventName: 'moveFileDetectionDisabled' });
192+
}
168193
} catch (e) {
169194
redirectToCliRepoError(e);
170195
}
@@ -193,7 +218,7 @@ export class ShadowRepo {
193218
}
194219

195220
public async getDeletes(): Promise<StatusRow[]> {
196-
return (await this.getStatus()).filter((file) => file[WORKDIR] === 0);
221+
return (await this.getStatus()).filter(isDeleted);
197222
}
198223

199224
public async getDeleteFilenames(): Promise<string[]> {
@@ -215,7 +240,7 @@ export class ShadowRepo {
215240
}
216241

217242
public async getAdds(): Promise<StatusRow[]> {
218-
return (await this.getStatus()).filter((file) => file[HEAD] === 0 && file[WORKDIR] === 2);
243+
return (await this.getStatus()).filter(isAdded);
219244
}
220245

221246
public async getAddFilenames(): Promise<string[]> {
@@ -294,14 +319,26 @@ export class ShadowRepo {
294319
}
295320
}
296321

297-
for (const filepath of [...new Set(this.isWindows ? deletedFiles.map(normalize).map(ensurePosix) : deletedFiles)]) {
298-
try {
299-
// these need to be done sequentially because isogit manages file locking. Isogit remove does not support multiple files at once
300-
// eslint-disable-next-line no-await-in-loop
301-
await git.remove({ fs, dir: this.projectPath, gitdir: this.gitDir, filepath });
302-
} catch (e) {
303-
redirectToCliRepoError(e);
322+
if (deletedFiles.length) {
323+
// Using a cache here speeds up the performance by ~24.4%
324+
let cache = {};
325+
const deleteMarker = Performance.mark('@salesforce/source-tracking', 'localShadowRepo.commitChanges#delete', {
326+
deletedFiles: deletedFiles.length,
327+
});
328+
for (const filepath of [
329+
...new Set(this.isWindows ? deletedFiles.map(normalize).map(ensurePosix) : deletedFiles),
330+
]) {
331+
try {
332+
// these need to be done sequentially because isogit manages file locking. Isogit remove does not support multiple files at once
333+
// eslint-disable-next-line no-await-in-loop
334+
await git.remove({ fs, dir: this.projectPath, gitdir: this.gitDir, filepath, cache });
335+
} catch (e) {
336+
redirectToCliRepoError(e);
337+
}
304338
}
339+
// clear cache
340+
cache = {};
341+
deleteMarker?.stop();
305342
}
306343

307344
try {
@@ -325,6 +362,74 @@ export class ShadowRepo {
325362
}
326363
marker?.stop();
327364
}
365+
366+
private async detectMovedFiles(): Promise<void> {
367+
const { addedFilenamesWithMatches, deletedFilenamesWithMatches } = getMatches(await this.getStatus()) ?? {};
368+
if (!addedFilenamesWithMatches || !deletedFilenamesWithMatches) return;
369+
370+
const movedFilesMarker = Performance.mark('@salesforce/source-tracking', 'localShadowRepo.detectMovedFiles');
371+
372+
// Track how long it takes to gather the oid information from the git trees
373+
const getInfoMarker = Performance.mark('@salesforce/source-tracking', 'localShadowRepo.detectMovedFiles#getInfo', {
374+
addedFiles: addedFilenamesWithMatches.length,
375+
deletedFiles: deletedFilenamesWithMatches.length,
376+
});
377+
378+
const getInfo = async (targetTree: Walker, filenameSet: Set<string>): Promise<FileInfo[]> =>
379+
// Unable to properly type the return value of git.walk without using "as", ignoring linter
380+
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
381+
git.walk({
382+
fs,
383+
dir: this.projectPath,
384+
gitdir: this.gitDir,
385+
trees: [targetTree],
386+
map: async (filename, [tree]) =>
387+
filenameSet.has(filename) && (await tree?.type()) === 'blob'
388+
? {
389+
filename,
390+
hash: await tree?.oid(),
391+
basename: path.basename(filename),
392+
}
393+
: undefined,
394+
});
395+
396+
// We found file adds and deletes with the same basename
397+
// The have likely been moved, confirm by comparing their hashes (oids)
398+
const [addedInfo, deletedInfo] = await Promise.all([
399+
getInfo(git.WORKDIR(), new Set(addedFilenamesWithMatches)),
400+
getInfo(git.TREE({ ref: 'HEAD' }), new Set(deletedFilenamesWithMatches)),
401+
]);
402+
403+
getInfoMarker?.stop();
404+
405+
const matchingNameAndHashes = compareHashes(await buildMaps(addedInfo, deletedInfo));
406+
if (matchingNameAndHashes.size === 0) {
407+
return movedFilesMarker?.stop();
408+
}
409+
const matches = removeNonMatches(matchingNameAndHashes, this.registry, this.isWindows);
410+
411+
if (matches.size === 0) {
412+
return movedFilesMarker?.stop();
413+
}
414+
415+
this.logger.debug(
416+
[
417+
'Files have moved. Committing moved files:',
418+
[...matches.entries()].map(([add, del]) => `- File ${del} was moved to ${add}`).join(os.EOL),
419+
].join(os.EOL)
420+
);
421+
422+
movedFilesMarker?.addDetails({ filesMoved: matches.size });
423+
424+
// Commit the moved files and refresh the status
425+
await this.commitChanges({
426+
deletedFiles: [...matches.values()],
427+
deployedFiles: [...matches.keys()],
428+
message: 'Committing moved files',
429+
});
430+
431+
movedFilesMarker?.stop();
432+
}
328433
}
329434

330435
const packageDirToRelativePosixPath =
@@ -336,4 +441,128 @@ const packageDirToRelativePosixPath =
336441
: path.relative(projectPath, packageDir.fullPath);
337442

338443
const normalize = (filepath: string): string => path.normalize(filepath);
444+
const ensureWindows = (filepath: string): string => path.win32.normalize(filepath);
339445
const ensurePosix = (filepath: string): string => filepath.split(path.sep).join(path.posix.sep);
446+
447+
const buildMap = (info: FileInfo[]): StringMap[] => {
448+
const map: StringMap = new Map();
449+
const ignore: StringMap = new Map();
450+
info.forEach((i) => {
451+
const key = `${i.hash}#${i.basename}`;
452+
// If we find a duplicate key, we need to remove it and ignore it in the future.
453+
// Finding duplicate hash#basename means that we cannot accurately determine where it was moved to or from
454+
if (map.has(key) || ignore.has(key)) {
455+
map.delete(key);
456+
ignore.set(key, i.filename);
457+
} else {
458+
map.set(key, i.filename);
459+
}
460+
});
461+
return [map, ignore];
462+
};
463+
464+
/** compare delete and adds from git.status, matching basenames of the files. returns early when there's nothing to match */
465+
const getMatches = (
466+
status: StatusRow[]
467+
): { deletedFilenamesWithMatches: string[]; addedFilenamesWithMatches: string[] } | undefined => {
468+
// We check for moved files in incremental steps and exit as early as we can to avoid any performance degradation
469+
// Deleted files will be more rare than added files, so we'll check them first and exit early if there are none
470+
const deletedFiles = status.filter(isDeleted);
471+
if (!deletedFiles.length) return;
472+
473+
const addedFiles = status.filter(isAdded);
474+
if (!addedFiles.length) return;
475+
476+
// Both arrays have contents, look for matching basenames
477+
const addedFilenames = toFilenames(addedFiles);
478+
const deletedFilenames = toFilenames(deletedFiles);
479+
480+
// Build Sets of basenames for added and deleted files for quick lookups
481+
const addedBasenames = new Set(addedFilenames.map((filename) => path.basename(filename)));
482+
const deletedBasenames = new Set(deletedFilenames.map((filename) => path.basename(filename)));
483+
484+
// Again, we filter over the deleted files first and exit early if there are no filename matches
485+
const deletedFilenamesWithMatches = deletedFilenames.filter((f) => addedBasenames.has(path.basename(f)));
486+
if (!deletedFilenamesWithMatches.length) return;
487+
488+
const addedFilenamesWithMatches = addedFilenames.filter((f) => deletedBasenames.has(path.basename(f)));
489+
if (!addedFilenamesWithMatches.length) return;
490+
491+
return { addedFilenamesWithMatches, deletedFilenamesWithMatches };
492+
};
493+
494+
const isDeleted = (status: StatusRow): boolean => status[WORKDIR] === 0;
495+
const isAdded = (status: StatusRow): boolean => status[HEAD] === 0 && status[WORKDIR] === 2;
496+
497+
/** build maps of the add/deletes with filenames, returning the matches Logs if non-matches */
498+
const buildMaps = async (addedInfo: FileInfo[], deletedInfo: FileInfo[]): Promise<AddAndDeleteMaps> => {
499+
const [addedMap, addedIgnoredMap] = buildMap(addedInfo);
500+
const [deletedMap, deletedIgnoredMap] = buildMap(deletedInfo);
501+
502+
// If we detected any files that have the same basename and hash, emit a warning and send telemetry
503+
// These files will still show up as expected in the `sf project deploy preview` output
504+
// We could add more logic to determine and display filepaths that we ignored...
505+
// but this is likely rare enough to not warrant the added complexity
506+
// Telemetry will help us determine how often this occurs
507+
if (addedIgnoredMap.size || deletedIgnoredMap.size) {
508+
const message = 'Files were found that have the same basename and hash. Skipping the commit of these files';
509+
const logger = Logger.childFromRoot('ShadowRepo.compareHashes');
510+
logger.warn(message);
511+
const lifecycle = Lifecycle.getInstance();
512+
await Promise.all([
513+
lifecycle.emitWarning(message),
514+
lifecycle.emitTelemetry({ eventName: 'moveFileHashBasenameCollisionsDetected' }),
515+
]);
516+
}
517+
return { addedMap, deletedMap };
518+
};
519+
520+
/** builds a map of the values from both maps */
521+
const compareHashes = ({ addedMap, deletedMap }: AddAndDeleteMaps): StringMap => {
522+
const matches: StringMap = new Map();
523+
524+
for (const [addedKey, addedValue] of addedMap) {
525+
const deletedValue = deletedMap.get(addedKey);
526+
if (deletedValue) {
527+
matches.set(addedValue, deletedValue);
528+
}
529+
}
530+
531+
return matches;
532+
};
533+
534+
const resolveType = (resolver: MetadataResolver, filenames: string[]): SourceComponent[] =>
535+
filenames
536+
.flatMap((filename) => {
537+
try {
538+
return resolver.getComponentsFromPath(filename);
539+
} catch (e) {
540+
const logger = Logger.childFromRoot('ShadowRepo.compareTypes');
541+
logger.warn(`unable to resolve ${filename}`);
542+
return undefined;
543+
}
544+
})
545+
.filter(sourceComponentGuard);
546+
547+
const removeNonMatches = (matches: StringMap, registry: RegistryAccess, isWindows: boolean): StringMap => {
548+
const addedFiles = isWindows ? [...matches.keys()].map(ensureWindows) : [...matches.keys()];
549+
const deletedFiles = isWindows ? [...matches.values()].map(ensureWindows) : [...matches.values()];
550+
const resolverAdded = new MetadataResolver(registry, VirtualTreeContainer.fromFilePaths(addedFiles));
551+
const resolverDeleted = new MetadataResolver(registry, VirtualTreeContainer.fromFilePaths(deletedFiles));
552+
553+
return new Map(
554+
[...matches.entries()].filter(([addedFile, deletedFile]) => {
555+
// we're only ever using the first element of the arrays
556+
const [resolvedAdded] = resolveType(resolverAdded, isWindows ? [ensureWindows(addedFile)] : [addedFile]);
557+
const [resolvedDeleted] = resolveType(resolverDeleted, isWindows ? [ensureWindows(deletedFile)] : [deletedFile]);
558+
return (
559+
// they could match, or could both be undefined (because unresolved by SDR)
560+
resolvedAdded?.type.name === resolvedDeleted?.type.name &&
561+
// parent names match, if resolved and there are parents
562+
resolvedAdded?.parent?.name === resolvedDeleted?.parent?.name &&
563+
// parent types match, if resolved and there are parents
564+
resolvedAdded?.parent?.type.name === resolvedDeleted?.parent?.type.name
565+
);
566+
})
567+
);
568+
};

src/sourceTracking.ts

+1
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,7 @@ export class SourceTracking extends AsyncCreatable {
512512
orgId: this.orgId,
513513
projectPath: normalize(this.projectPath),
514514
packageDirs: this.packagesDirs,
515+
registry: this.registry,
515516
});
516517
// loads the status from file so that it's cached
517518
await this.localRepo.getStatus();

0 commit comments

Comments
 (0)