refactor(metro-file-map): Pre-resolve symlink targets and store normal posix paths

packages/metro-file-map/src/crawlers/watchman/index.js

@@ -327,6 +327,11 @@ export default async function watchmanCrawl({
         if (fileData.type === 'l') {
           symlinkInfo = fileData['symlink_target'] ?? 1;
         }
+        if (typeof symlinkInfo === 'string') {
+          symlinkInfo = normalizePathSeparatorsToPosix(
+            pathUtils.resolveSymlinkToNormal(relativeFilePath, symlinkInfo),
+          );
+        }
 
         const nextData: FileMetadata = [
           mtime,

packages/metro-file-map/src/flow-types.js

@@ -292,7 +292,9 @@ export type FileMetadata = [
   /* size */ number,
   /* visited */ 0 | 1,
   /* sha1 */ ?string,
-  /* symlink */ 0 | 1 | string, // string specifies target, if known
+  // A string is the symlink's target, lexically resolved to a normal path with
+  // POSIX separators, if known. The target need not exist.
+  /* symlink */ 0 | 1 | string,
   /* plugindata */
   ...
 ];

packages/metro-file-map/src/index.js

@@ -167,7 +167,7 @@ export type {
 // This should be bumped whenever a code change to `metro-file-map` itself
 // would cause a change to the cache data structure and/or content (for a given
 // filesystem state and build parameters).
-const CACHE_BREAKER = '11';
+const CACHE_BREAKER = '12';
 
 const CHANGE_INTERVAL = 30;
 
@@ -217,7 +217,8 @@ const WATCHMAN_REQUIRED_CAPABILITIES = [
  *   visited: boolean, // whether the file has been parsed or not.
  *   dependencies: Array<string>, // all relative dependencies of this file.
  *   sha1: ?string, // SHA-1 of the file, if requested via options.
- *   symlink: ?(1 | 0 | string), // Truthy if symlink, string is target
+ *   symlink: ?(1 | 0 | string), // Truthy if symlink, string is the target,
+ *                               // lexically resolved to a normal POSIX path
  * };
  *
  * // Modules can be targeted to a specific platform based on the file name.
@@ -592,7 +593,9 @@ export default class FileMap extends EventEmitter {
         .readlink(this.#pathUtils.normalToAbsolute(normalPath))
         .then(symlinkTarget => {
           fileMetadata[H.VISITED] = 1;
-          fileMetadata[H.SYMLINK] = symlinkTarget;
+          fileMetadata[H.SYMLINK] = normalizePathSeparatorsToPosix(
+            this.#pathUtils.resolveSymlinkToNormal(normalPath, symlinkTarget),
+          );
         });
     }
     return null;

packages/metro-file-map/src/lib/RootPathUtils.js

@@ -174,23 +174,25 @@ export class RootPathUtils {
     );
   }
 
-  // If a path is a direct ancestor of the project root (or the root itself),
-  // return a number with the degrees of separation, e.g. root=0, parent=1,..
-  // or null otherwise.
-  getAncestorOfRootIdx(normalPath: string): ?number {
-    if (normalPath === '') {
-      return 0;
-    }
-    if (normalPath === '..') {
-      return 1;
-    }
-    // Otherwise a *normal* path is only a root ancestor if it is a sequence of
-    // '../' segments followed by '..', so the length tells us the number of
-    // up fragments.
-    if (normalPath.endsWith(SEP_UP_FRAGMENT)) {
-      return (normalPath.length + 1) / 3;
-    }
-    return null;
+  resolveSymlinkToNormal(
+    symlinkNormalPath: string,
+    readlinkResult: string,
+  ): string {
+    // Lexically resolves the target against the symlink's directory. This is
+    // string manipulation only: symlinks within the target are not followed,
+    // and the target need not exist, so the result is not a real path.
+    //
+    // readlink returns whatever the link was created with, which need not be
+    // well-formed (e.g. '..', 'a/./b', 'a//b', or '/' separators on Windows),
+    // so resolve with node:path. This runs once per symlink, when its node is
+    // populated, not on traversal.
+    const normal = this.absoluteToNormal(
+      path.resolve(this.#rootDir, symlinkNormalPath, '..', readlinkResult),
+    );
+    // Normalization keeps a trailing separator when the result is the root or
+    // an ancestor of it (e.g. a link to '/'), and readlink itself may return
+    // one. A stored symlink target never has one.
+    return normal.endsWith(path.sep) ? normal.slice(0, -1) : normal;
   }
 
   // Takes a normal and relative path, and joins them efficiently into a normal

packages/metro-file-map/src/lib/TreeFS.js

@@ -21,6 +21,7 @@ import type {
 } from '../flow-types';
 
 import H from '../constants';
+import normalizePathSeparatorsToSystem from './normalizePathSeparatorsToSystem';
 import {RootPathUtils} from './RootPathUtils';
 import invariant from 'invariant';
 import path from 'node:path';
@@ -37,12 +38,6 @@ function isRegularFile(node: FileNode): boolean {
   return node[H.SYMLINK] === 0;
 }
 
-type NormalizedSymlinkTarget = {
-  ancestorOfRootIdx: ?number,
-  normalPath: string,
-  startOfBasenameIdx: number,
-};
-
 type DeserializedSnapshotInput = {
   rootDir: string,
   fileSystemData: DirectoryNode,
@@ -95,10 +90,16 @@ type MetadataIteratorOptions = Readonly<{
  *
  * SYMLINKS:
  *
- * Symlinks are represented as nodes whose metadata contains their literal
- * target. Literal targets are resolved to normal paths at runtime, and cached.
- * If a symlink is encountered during traversal, we restart traversal at the
- * root node targeting join(normal symlink target, remaining path suffix).
+ * Symlinks are represented as nodes whose metadata contains their target,
+ * lexically resolved to a normal path with POSIX separators when the node was
+ * populated. Lexical resolution only interprets the literal target relative to
+ * the symlink's directory: it does not follow symlinks in the target, and the
+ * target need not exist, so it is a normalPath, not a canonicalPath (see
+ * TERMINOLOGY). The target is the only path stored inside the tree, so it must
+ * not use system separators, or the snapshot would not be portable between
+ * operating systems. If a symlink is encountered during traversal, we restart
+ * traversal at the root node targeting join(normal symlink target, remaining
+ * path suffix).
  *
  * NODE TYPES:
  *
@@ -107,8 +108,8 @@ type MetadataIteratorOptions = Readonly<{
  * - A file is represented by an `Array`  (tuple) of metadata, of which:
  *   - A regular file has node[H.SYMLINK] === 0
  *   - A symlink has node[H.SYMLINK] === 1 or
- *     typeof node[H.SYMLINK] === 'string', where a string is the literal
- *     content of the symlink (i.e. from readlink), if known.
+ *     typeof node[H.SYMLINK] === 'string', where a string is the target
+ *     lexically resolved to a normal path with POSIX separators, if known.
  *
  * TERMINOLOGY:
  *
@@ -124,10 +125,6 @@ type MetadataIteratorOptions = Readonly<{
  *   a trailing slash
  */
 export default class TreeFS implements MutableFileSystem {
-  readonly #cachedNormalSymlinkTargets: WeakMap<
-    FileNode,
-    NormalizedSymlinkTarget,
-  > = new WeakMap();
   readonly #pathUtils: RootPathUtils;
   readonly #processFile: ProcessFileFunction;
   readonly #rootDir: Path;
@@ -723,11 +720,16 @@ export default class TreeFS implements MutableFileSystem {
           };
         }
 
-        // Symlink in a directory path
-        const normalSymlinkTarget = this.#resolveSymlinkTargetToNormalPath(
-          segmentNode,
-          currentPath,
+        // Symlink in a directory path. Targets are stored already lexically
+        // resolved to a normal path, with POSIX separators so that the
+        // snapshot is portable between operating systems.
+        const storedSymlinkTarget = segmentNode[H.SYMLINK];
+        invariant(
+          typeof storedSymlinkTarget === 'string',
+          'Expected symlink target to be populated.',
         );
+        const normalSymlinkTarget =
+          normalizePathSeparatorsToSystem(storedSymlinkTarget);
         if (opts.collectLinkPaths) {
           opts.collectLinkPaths.add(
             this.#pathUtils.normalToAbsolute(currentPath),
@@ -741,7 +743,7 @@ export default class TreeFS implements MutableFileSystem {
         // Append any subsequent path segments to the symlink target, and reset
         // with our new target.
         const joinedResult = this.#pathUtils.joinNormalToRelative(
-          normalSymlinkTarget.normalPath,
+          normalSymlinkTarget,
           remainingTargetPath,
         );
 
@@ -761,8 +763,7 @@ export default class TreeFS implements MutableFileSystem {
           collectAncestors &&
           !isLastSegment &&
           // No-op optimisation to bail out the common case of nothing to do.
-          (normalSymlinkTarget.ancestorOfRootIdx === 0 ||
-            joinedResult.collapsedSegments > 0)
+          (normalSymlinkTarget === '' || joinedResult.collapsedSegments > 0)
         ) {
           let node: MixedNode = this.#rootNode;
           let collapsedPath = '';
@@ -773,20 +774,12 @@ export default class TreeFS implements MutableFileSystem {
             /* for Flow, always true: */ isDirectory(node);
             i++
           ) {
-            if (
-              // Add the root only if the target is the root or we have
-              // collapsed segments.
-              i > 0 ||
-              normalSymlinkTarget.ancestorOfRootIdx === 0 ||
-              joinedResult.collapsedSegments > 0
-            ) {
-              reverseAncestors.push({
-                ancestorOfRootIdx: i,
-                node,
-                normalPath: collapsedPath,
-                segmentName: this.#pathUtils.getBasenameOfNthAncestor(i),
-              });
-            }
+            reverseAncestors.push({
+              ancestorOfRootIdx: i,
+              node,
+              normalPath: collapsedPath,
+              segmentName: this.#pathUtils.getBasenameOfNthAncestor(i),
+            });
             node = node.get('..') ?? new Map();
             collapsedPath =
               collapsedPath === '' ? '..' : collapsedPath + path.sep + '..';
@@ -800,7 +793,7 @@ export default class TreeFS implements MutableFileSystem {
         // the symlink target, and start collecting ancestors only
         // from the target itself (ie, the basename of the normal target path)
         // onwards.
-        unseenPathFromIdx = normalSymlinkTarget.startOfBasenameIdx;
+        unseenPathFromIdx = normalSymlinkTarget.lastIndexOf(path.sep) + 1;
 
         if (seen == null) {
           // Optimisation: set this lazily only when we've encountered a symlink
@@ -1222,43 +1215,6 @@ export default class TreeFS implements MutableFileSystem {
     }
   }
 
-  #resolveSymlinkTargetToNormalPath(
-    symlinkNode: FileMetadata,
-    canonicalPathOfSymlink: Path,
-  ): NormalizedSymlinkTarget {
-    const cachedResult = this.#cachedNormalSymlinkTargets.get(symlinkNode);
-    if (cachedResult != null) {
-      return cachedResult;
-    }
-
-    const literalSymlinkTarget = symlinkNode[H.SYMLINK];
-    invariant(
-      typeof literalSymlinkTarget === 'string',
-      'Expected symlink target to be populated.',
-    );
-    let absoluteSymlinkTarget = path.resolve(
-      this.#rootDir,
-      canonicalPathOfSymlink,
-      '..', // Symlink target is relative to its containing directory.
-      literalSymlinkTarget, // May be absolute, in which case the above are ignored
-    );
-    if (absoluteSymlinkTarget.endsWith(path.sep)) {
-      absoluteSymlinkTarget = absoluteSymlinkTarget.slice(0, -1);
-    }
-    const normalSymlinkTarget = this.#pathUtils.absoluteToNormal(
-      absoluteSymlinkTarget,
-    );
-
-    const result = {
-      ancestorOfRootIdx:
-        this.#pathUtils.getAncestorOfRootIdx(normalSymlinkTarget),
-      normalPath: normalSymlinkTarget,
-      startOfBasenameIdx: normalSymlinkTarget.lastIndexOf(path.sep) + 1,
-    };
-    this.#cachedNormalSymlinkTargets.set(symlinkNode, result);
-    return result;
-  }
-
   #getFileData(
     filePath: Path,
     opts: {followLeaf: boolean} = {followLeaf: true},

packages/metro-file-map/src/lib/__tests__/RootPathUtils-test.js

@@ -164,18 +164,6 @@ describe.each([['win32'], ['posix']])('RootPathUtils on %s', platform => {
     );
   });
 
-  test.each([
-    ['foo', null],
-    ['', 0],
-    ['..', 1],
-    [p('../..'), 2],
-    [p('../../..'), 3],
-    [p('../../../foo'), null],
-    [p('../../../..foo'), null],
-  ])('getAncestorOfRootIdx (%s => %s)', (input, expected) => {
-    expect(pathUtils.getAncestorOfRootIdx(input)).toEqual(expected);
-  });
-
   if (platform === 'win32') {
     describe('cross-drive absolute paths (Windows)', () => {
       test.each([['C:\\project\\root'], ['C:\\']])(
@@ -246,4 +234,99 @@ describe.each([['win32'], ['posix']])('RootPathUtils on %s', platform => {
       );
     });
   }
+
+  describe('resolveSymlinkToNormal', () => {
+    beforeEach(() => {
+      pathUtils = new RootPathUtils(p('/project/root'));
+    });
+
+    test.each([
+      ['foo/link', './target.js', p('foo/target.js')],
+      ['foo/link', '../bar.js', 'bar.js'],
+      ['link', 'target.js', 'target.js'],
+      [p('a/b/link'), p('../../c.js'), 'c.js'],
+      [p('a/b/link'), p('../../../outside/f.js'), p('../outside/f.js')],
+    ])(
+      'resolves relative target (%s -> %s) to %s',
+      (symlinkPath, readlinkResult, expected) => {
+        expect(
+          pathUtils.resolveSymlinkToNormal(p(symlinkPath), readlinkResult),
+        ).toEqual(expected);
+      },
+    );
+
+    // readlink returns the target as the link was created, which need not be
+    // well-formed. On Windows, these also use '/' separators.
+    test.each([
+      ['a/link', '..', ''],
+      ['a/link', '.', 'a'],
+      ['a/link', './', 'a'],
+      ['a/link', 'b/..', 'a'],
+      ['a/link', 'b/../c', p('a/c')],
+      ['a/link', 'b/./c', p('a/b/c')],
+      ['a/link', 'b//c', p('a/b/c')],
+      ['a/b/link', '../..', ''],
+      ['a/b/link', '../../..', '..'],
+    ])(
+      'resolves non-well-formed target (%s -> %s) to %s',
+      (symlinkPath, readlinkResult, expected) => {
+        expect(
+          pathUtils.resolveSymlinkToNormal(p(symlinkPath), readlinkResult),
+        ).toEqual(expected);
+      },
+    );
+
+    test.each([
+      ['link', p('/project/root/target.js'), 'target.js'],
+      ['link', p('/project/root/a/b.js'), p('a/b.js')],
+      ['link', p('/outside/foo.js'), p('../../outside/foo.js')],
+      [p('a/link'), p('/project/root'), ''],
+    ])(
+      'resolves absolute target (%s -> %s) to %s',
+      (symlinkPath, readlinkResult, expected) => {
+        expect(
+          pathUtils.resolveSymlinkToNormal(p(symlinkPath), readlinkResult),
+        ).toEqual(expected);
+      },
+    );
+
+    test('strips trailing separator from target', () => {
+      expect(
+        pathUtils.resolveSymlinkToNormal('link', p('/project/root/dir/')),
+      ).toEqual('dir');
+    });
+
+    // A filesystem root is the one absolute target that consists only of a
+    // separator, so it must not be trimmed before we test for absoluteness.
+    // Relative targets may also reach the filesystem root, or pass it: '..' at
+    // the filesystem root is the filesystem root itself.
+    test.each([
+      ['link', p('/'), p('../..')],
+      [p('a/link'), p('/'), p('../..')],
+      ['link', '../..', p('../..')],
+      ['link', '../../../../..', p('../..')],
+      [p('a/link'), '../../../../../..', p('../..')],
+    ])(
+      'resolves filesystem root target (%s -> %s) to %s',
+      (symlinkPath, readlinkResult, expected) => {
+        expect(
+          pathUtils.resolveSymlinkToNormal(symlinkPath, readlinkResult),
+        ).toEqual(expected);
+      },
+    );
+
+    if (platform === 'win32') {
+      test.each([
+        ['D:\\', '..\\..\\..\\D:'],
+        ['D:\\ext\\', '..\\..\\..\\D:\\ext'],
+      ])(
+        'resolves cross-drive root target (%s) to %s',
+        (readlinkResult, expected) => {
+          expect(
+            pathUtils.resolveSymlinkToNormal('link', readlinkResult),
+          ).toEqual(expected);
+        },
+      );
+    }
+  });
 });