Skip to content

Commit

Permalink
TreeFS: Return traversed symlinks and first missing segments in looku…
Browse files Browse the repository at this point in the history 
…p results (#1217)

Summary:
Pull Request resolved: #1217

A module resolution should be invalidated whenever the (non-)existence of a path checked during resolution may have changed. To implement this, we'll need more information from the `FileSystem` APIs.

## Existence
A positive existence check is straightforwardly invalidated when either:
 - The resolved `realPath` is deleted (we already watch for this), or
 - Any symlink traversed on the way to the real path is deleted or modified. We will need to track symlinks traversed by their real paths.

## Non-existence
For non-existence, note that it is not sufficient to simply watch for file events whose path matches the path originally looked up, because file events are emitted for real paths, and the checked path may resolve through one or more symlinks.

We will require:
 - The first missing *real* path segment encountered (because we can watch for additions at this path, but not necessarily any of its descendants, should a symlink be created at this path), *and*
 - The symlinks traversed to get to that first missing segment (because changes to these invalidate which real path we should watch for an addition).

## This diff
This adds `links`, the set of traversed links by their canonical paths, and `missing` (for non-existence) - the first non-navigable path segment by its canonical path.

Changelog: Internal

Reviewed By: huntie

Differential Revision: D52391404

fbshipit-source-id: 40ec2717df6a4f5837832d789e3a6a80a2178c83
  • Loading branch information
@robhogan @facebook-github-bot
robhogan authored and facebook-github-bot committed Feb 8, 2024
1 parent 6338512 commit 451e2a1
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 33 deletions.
19 changes: 18 additions & 1 deletion packages/metro-file-map/src/flow-types.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,26 @@ export type Glob = string;

export type LookupResult =
| {
// The node is missing from the FileSystem implementation (note this
// could indicate an unwatched path, or a directory containing no watched
// files).
exists: false,
// The real, normal, absolute paths of any symlinks traversed.
links: $ReadOnlySet<string>,
// The real, normal, absolute path of the first path segment
// encountered that does not exist, or cannot be navigated through.
missing: string,
}
| {exists: true, realPath: string, type: 'd' | 'f' | 'l'};
| {
exists: true,
// The real, normal, absolute paths of any symlinks traversed.
links: $ReadOnlySet<string>,
// The real, normal, absolute path of the file or directory.
realPath: string,
// Currently lookup always follows symlinks, so can only return
// directories or regular files, but this may be extended.
type: 'd' | 'f',
};

export interface MockMap {
getMockModule(name: string): ?Path;
Expand Down
67 changes: 57 additions & 10 deletions packages/metro-file-map/src/lib/TreeFS.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,34 @@ export default class TreeFS implements MutableFileSystem {
const normalPath = this._normalizePath(mixedPath);
const result = this._lookupByNormalPath(normalPath, {followLeaf: true});
if (!result.exists) {
const {canonicalMissingPath, canonicalLinkPaths} = result;
return {
exists: false,
links: new Set(
canonicalLinkPaths.map(canonicalPath =>
this.#pathUtils.normalToAbsolute(canonicalPath),
),
),
missing: this.#pathUtils.normalToAbsolute(canonicalMissingPath),
};
}
const {node, canonicalPath} = result;
const {canonicalPath, canonicalLinkPaths, node} = result;
const type = node instanceof Map ? 'd' : node[H.SYMLINK] === 0 ? 'f' : 'l';
invariant(
type !== 'l',
'lookup follows symlinks, so should never return one (%s -> %s)',
mixedPath,
canonicalPath,
);
return {
exists: true,
links: new Set(
canonicalLinkPaths.map(canonicalPath =>
this.#pathUtils.normalToAbsolute(canonicalPath),
),
),
realPath: this.#pathUtils.normalToAbsolute(canonicalPath),
type: node instanceof Map ? 'd' : node[H.SYMLINK] === 0 ? 'f' : 'l',
type,
};
}
Expand Down Expand Up @@ -345,20 +364,28 @@ export default class TreeFS implements MutableFileSystem {
} = {followLeaf: true, makeDirectories: false},
):
| {
canonicalLinkPaths: Array<string>,
canonicalPath: string,
exists: true,
node: MixedNode,
parentNode: DirectoryNode,
}
| {
canonicalLinkPaths: Array<string>,
canonicalPath: string,
exists: true,
node: DirectoryNode,
parentNode: null,
}
| {exists: false} {
| {
canonicalLinkPaths: Array<string>,
canonicalMissingPath: string,
exists: false,
} {
// We'll update the target if we hit a symlink.
let targetNormalPath = requestedNormalPath;
// Set of traversed symlink paths to return.
const canonicalLinkPaths: Array<string> = [];
// Lazy-initialised set of seen target paths, to detect symlink cycles.
let seen: ?Set<string>;
// Pointer to the first character of the current path segment in
Expand All @@ -380,14 +407,21 @@ export default class TreeFS implements MutableFileSystem {
}

let segmentNode = parentNode.get(segmentName);

if (segmentNode == null) {
if (opts.makeDirectories !== true) {
if (opts.makeDirectories !== true && segmentName !== '..') {
return {
canonicalLinkPaths,
canonicalMissingPath: isLastSegment
? targetNormalPath
: targetNormalPath.slice(0, fromIdx - 1),
exists: false,
};
}
segmentNode = new Map();
parentNode.set(segmentName, segmentNode);
if (opts.makeDirectories === true) {
parentNode.set(segmentName, segmentNode);
}
}

// If there are no more '/' to come, we're done unless this is a symlink
Expand All @@ -399,6 +433,7 @@ export default class TreeFS implements MutableFileSystem {
opts.followLeaf === false)
) {
return {
canonicalLinkPaths,
canonicalPath: targetNormalPath,
exists: true,
node: segmentNode,
Expand All @@ -410,18 +445,25 @@ export default class TreeFS implements MutableFileSystem {
if (segmentNode instanceof Map) {
parentNode = segmentNode;
} else {
const currentPath = isLastSegment
? targetNormalPath
: targetNormalPath.slice(0, fromIdx - 1);

if (segmentNode[H.SYMLINK] === 0) {
// Regular file in a directory path
return {exists: false};
return {
canonicalLinkPaths,
canonicalMissingPath: currentPath,
exists: false,
};
}

// Symlink in a directory path
const normalSymlinkTarget = this._resolveSymlinkTargetToNormalPath(
segmentNode,
isLastSegment
? targetNormalPath
: targetNormalPath.slice(0, fromIdx - 1),
currentPath,
);
canonicalLinkPaths.push(currentPath);

// Append any subsequent path segments to the symlink target, and reset
// with our new target.
Expand All @@ -434,7 +476,11 @@ export default class TreeFS implements MutableFileSystem {
}
if (seen.has(targetNormalPath)) {
// TODO: Warn `Symlink cycle detected: ${[...seen, node].join(' -> ')}`
return {exists: false};
return {
canonicalLinkPaths,
canonicalMissingPath: targetNormalPath,
exists: false,
};
}
seen.add(targetNormalPath);
fromIdx = 0;
Expand All @@ -443,6 +489,7 @@ export default class TreeFS implements MutableFileSystem {
}
invariant(parentNode === this.#rootNode, 'Unexpectedly escaped traversal');
return {
canonicalLinkPaths,
canonicalPath: targetNormalPath,
exists: true,
node: this.#rootNode,
Expand Down
114 changes: 92 additions & 22 deletions packages/metro-file-map/src/lib/__tests__/TreeFS-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*/

import type {FileData} from '../../flow-types';
import type TreeFS from '../TreeFS';
import type TreeFSType from '../TreeFS';

let mockPathModule;
jest.mock('path', () => mockPathModule);
Expand All @@ -23,11 +23,13 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
? filePath.replace(/\//g, '\\').replace(/^\\/, 'C:\\')
: filePath;

let tfs: TreeFS;
let tfs: TreeFSType;
let TreeFS: Class<TreeFSType>;

beforeEach(() => {
jest.resetModules();
mockPathModule = jest.requireActual<{}>('path')[platform];
const TreeFS = require('../TreeFS').default;
TreeFS = require('../TreeFS').default;
tfs = new TreeFS({
rootDir: p('/project'),
files: new Map([
Expand Down Expand Up @@ -95,26 +97,58 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {

describe('lookup', () => {
test.each([
[p('/project/foo/link-to-another.js'), p('/project/foo/another.js')],
[p('/project/foo/link-to-bar.js'), p('/project/bar.js')],
[p('link-to-foo/link-to-another.js'), p('/project/foo/another.js')],
[p('/project/root/outside/external.js'), p('/outside/external.js')],
[p('/outside/../project/bar.js'), p('/project/bar.js')],
])('%s -> %s', (givenPath, expectedRealPath) =>
expect(tfs.lookup(givenPath)).toMatchObject({
exists: true,
realPath: expectedRealPath,
type: 'f',
}),
[
p('/project/foo/link-to-another.js'),
p('/project/foo/another.js'),
[p('/project/foo/link-to-another.js')],
],
[
p('/project/foo/link-to-bar.js'),
p('/project/bar.js'),
[p('/project/foo/link-to-bar.js')],
],
[
p('link-to-foo/link-to-another.js'),
p('/project/foo/another.js'),
[p('/project/link-to-foo'), p('/project/foo/link-to-another.js')],
],
[
p('/project/root/outside/external.js'),
p('/outside/external.js'),
[p('/project/root')],
],
[p('/outside/../project/bar.js'), p('/project/bar.js'), []],
])(
'%s -> %s through expected symlinks',
(givenPath, expectedRealPath, expectedSymlinks) =>
expect(tfs.lookup(givenPath)).toEqual({
exists: true,
links: new Set(expectedSymlinks),
realPath: expectedRealPath,
type: 'f',
}),
);

test.each([
[p('/project/bar.js/bad-parent')],
[p('/project/link-to-nowhere')],
[p('/project/not/exists')],
[p('/project/bar.js/bad-parent'), [], p('/project/bar.js')],
[
p('/project/link-to-nowhere'),
[p('/project/link-to-nowhere')],
p('/project/nowhere'),
],
[p('/project/not/exists'), [], p('/project/not')],
[p('/project/root/missing'), [p('/project/root')], p('/missing')],
[p('/project/../missing'), [], p('/missing')],
[p('/project/foo/../../missing'), [], p('/missing')],
[p('/project/foo/../../project/missing'), [], p('/project/missing')],
])(
'returns exists: false for missing files, bad paths or broken links: %s',
givenPath => expect(tfs.lookup(givenPath).exists).toEqual(false),
'non-existence for bad paths, missing files or broken links %s',
(givenPath, expectedSymlinks, missingPath) =>
expect(tfs.lookup(givenPath)).toEqual({
exists: false,
links: new Set(expectedSymlinks),
missing: missingPath,
}),
);

test.each([[p('/project/foo')], [p('/project/root/outside')]])(
Expand All @@ -125,6 +159,42 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
type: 'd',
}),
);

test('traversing the same symlink multiple times does not imply a cycle', () => {
expect(
tfs.lookup(p('/project/foo/owndir/owndir/another.js')),
).toMatchObject({
exists: true,
realPath: p('/project/foo/another.js'),
type: 'f',
});
});

test('ancestors of the root are not reported as missing', () => {
const tfs = new TreeFS({
rootDir: p('/deep/project/root'),
files: new Map([
[p('foo/index.js'), ['', 123, 0, 0, '', '', 0]],
[p('link-up'), ['', 123, 0, 0, '', '', p('..')]],
]),
});
expect(tfs.lookup(p('/deep/missing/bar.js'))).toMatchObject({
exists: false,
missing: p('/deep/missing'),
});
expect(tfs.lookup(p('link-up/bar.js'))).toMatchObject({
exists: false,
missing: p('/deep/project/bar.js'),
});
expect(tfs.lookup(p('../../baz.js'))).toMatchObject({
exists: false,
missing: p('/deep/baz.js'),
});
expect(tfs.lookup(p('../../project/root/baz.js'))).toMatchObject({
exists: false,
missing: p('/deep/project/root/baz.js'),
});
});
});

describe('getDifference', () => {
Expand Down Expand Up @@ -436,7 +506,7 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
});

describe('metadataIterator', () => {
test('iterates over all files with Haste names, skipping node_modules', () => {
test('iterates over all files with Haste names, skipping node_modules and symlinks', () => {
expect([
...tfs.metadataIterator({
includeSymlinks: false,
Expand All @@ -461,7 +531,7 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
]);
});

test('iterates over all files with Haste names, skipping node_modules', () => {
test('iterates over all files with Haste names, including node_modules, skipping symlinks', () => {
expect([
...tfs.metadataIterator({
includeSymlinks: false,
Expand All @@ -478,7 +548,7 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
);
});

test('iterates over all files with Haste names, skipping node_modules', () => {
test('iterates over all files with Haste names, including node_modules and symlinks', () => {
expect([
...tfs.metadataIterator({
includeSymlinks: true,
Expand Down

0 comments on commit 451e2a1

Please sign in to comment.