Skip to content

Commit

Permalink
Add symlink field to file metadata and cache
Browse files Browse the repository at this point in the history 
Summary:
This adds another entry to the `FileMetadata` tuple used throughout `metro-file-map`, to be used to compactly represent:

 a) Whether a file is regular or a symlink
 b) The symlink target, if known.

In doing so, it changes the persisted cache format - so this diff also bumps the `CACHE_BREAKER` key.

Note that because Metro only passes `enableSymlinks: false` to the `metro-file-map`, and this config isn't exposed to the user, *all* crawl results should currently have a `0` entry.

## Storing symlinks in the `files` map
I considered storing symlinks in a separate `Map` alongside `files`, which is slightly more compact. Ultimately though this is more difficult to work with - almost every lookup becomes two lookups and it's rare that we'd need to iterate over one but not the other. We could revisit this if we ever separate the serialised structure from the working-copy structure.

## Type: `0 | 1 | string`
There are a couple of considerations in this type, which aims to compactly capture both the type and (optionally) the symlink target:
 - As an intermediate state, it's useful to be able to record that a file is a symlink without necessarily knowing the target. In particular, this allows us to `readlink` downstream of non-Watchman crawlers and watchers that can't natively provide the target, e.g. in the multi-thread `processFile` phase, or indeed on demand.
 - We use `0 | 1` as opposed to `boolean` for serialised compactness, following the precedent of `H.VISITED`. (`null` would be ambiguous in this context.)

NOTE: `metadata[H.SYMLINK] === 0` should be used to check for regular files at the low level - truthiness is unreliable because [POSIX says](https://pubs.opengroup.org/onlinepubs/9699919799/functions/symlink.html) the target is an arbitrary string (including an empty string) - (in practice - macOS allows empty symlinks, linux doesn't).

Reviewed By: huntie

Differential Revision: D42131590

fbshipit-source-id: 1fa39576988159887b88057f003de6018b85a583
  • Loading branch information
@robhogan @facebook-github-bot
robhogan authored and facebook-github-bot committed Jan 3, 2023
1 parent 2d60a02 commit 58c88b8
Show file tree
Hide file tree
Showing 9 changed files with 141 additions and 62 deletions.
49 changes: 42 additions & 7 deletions packages/metro-file-map/src/__tests__/index-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jest.mock('../crawlers/watchman', () =>
const relativeFilePath = path.relative(rootDir, file);
if (list[file]) {
const hash = computeSha1 ? mockHashContents(list[file]) : null;
changedFiles.set(relativeFilePath, ['', 32, 42, 0, [], hash]);
changedFiles.set(relativeFilePath, ['', 32, 42, 0, [], hash, 0]);
} else {
const fileData = previousState.files.get(relativeFilePath);
if (fileData) {
Expand Down Expand Up @@ -402,6 +402,7 @@ describe('HasteMap', () => {
1,
'Strawberry',
null,
0,
],
[path.join('fruits', 'Pear.js')]: [
'Pear',
Expand All @@ -410,6 +411,7 @@ describe('HasteMap', () => {
1,
'Banana\0Strawberry',
null,
0,
],
[path.join('fruits', 'Strawberry.js')]: [
'Strawberry',
Expand All @@ -418,6 +420,7 @@ describe('HasteMap', () => {
1,
'',
null,
0,
],
[path.join('fruits', '__mocks__', 'Pear.js')]: [
'',
Expand All @@ -426,8 +429,17 @@ describe('HasteMap', () => {
1,
'Melon',
null,
0,
],
[path.join('vegetables', 'Melon.js')]: [
'Melon',
32,
42,
1,
'',
null,
0,
],
[path.join('vegetables', 'Melon.js')]: ['Melon', 32, 42, 1, '', null],
}),
);

Expand Down Expand Up @@ -512,6 +524,7 @@ describe('HasteMap', () => {
0,
'Strawberry',
null,
0,
],
[path.join('fruits', 'Pear.js')]: [
'Pear',
Expand All @@ -520,6 +533,7 @@ describe('HasteMap', () => {
0,
'Banana\0Strawberry',
null,
0,
],
[path.join('fruits', 'Strawberry.js')]: [
'Strawberry',
Expand All @@ -528,6 +542,7 @@ describe('HasteMap', () => {
0,
'',
null,
0,
],
[path.join('fruits', '__mocks__', 'Pear.js')]: [
'',
Expand All @@ -536,8 +551,17 @@ describe('HasteMap', () => {
0,
'Melon',
null,
0,
],
[path.join('vegetables', 'Melon.js')]: [
'Melon',
32,
42,
0,
'',
null,
0,
],
[path.join('vegetables', 'Melon.js')]: ['Melon', 32, 42, 0, '', null],
});

return Promise.resolve({
Expand All @@ -564,6 +588,7 @@ describe('HasteMap', () => {
1,
'Strawberry',
'7772b628e422e8cf59c526be4bb9f44c0898e3d1',
0,
],
[path.join('fruits', 'Pear.js')]: [
'Pear',
Expand All @@ -572,6 +597,7 @@ describe('HasteMap', () => {
1,
'Banana\0Strawberry',
'89d0c2cc11dcc5e1df50b8af04ab1b597acfba2f',
0,
],
[path.join('fruits', 'Strawberry.js')]: [
'Strawberry',
Expand All @@ -580,6 +606,7 @@ describe('HasteMap', () => {
1,
'',
'e8aa38e232b3795f062f1d777731d9240c0f8c25',
0,
],
[path.join('fruits', '__mocks__', 'Pear.js')]: [
'',
Expand All @@ -588,6 +615,7 @@ describe('HasteMap', () => {
1,
'Melon',
'8d40afbb6e2dc78e1ba383b6d02cafad35cceef2',
0,
],
[path.join('vegetables', 'Melon.js')]: [
'Melon',
Expand All @@ -596,6 +624,7 @@ describe('HasteMap', () => {
1,
'',
'f16ccf6f2334ceff2ddb47628a2c5f2d748198ca',
0,
],
}),
);
Expand Down Expand Up @@ -647,7 +676,7 @@ describe('HasteMap', () => {
cacheContent.files.get(
path.join('fruits', 'node_modules', 'fbjs', 'fbjs.js'),
),
).toEqual(['', 32, 42, 0, [], null]);
).toEqual(['', 32, 42, 0, [], null, 0]);

expect(cacheContent.map.get('fbjs')).not.toBeDefined();

Expand Down Expand Up @@ -769,6 +798,7 @@ describe('HasteMap', () => {
1,
'Blackberry',
null,
0,
],
[path.join('fruits', 'Strawberry.ios.js')]: [
'Strawberry',
Expand All @@ -777,6 +807,7 @@ describe('HasteMap', () => {
1,
'Raspberry',
null,
0,
],
[path.join('fruits', 'Strawberry.js')]: [
'Strawberry',
Expand All @@ -785,6 +816,7 @@ describe('HasteMap', () => {
1,
'Banana',
null,
0,
],
}),
);
Expand Down Expand Up @@ -879,6 +911,7 @@ describe('HasteMap', () => {
1,
'Kiwi',
null,
0,
]);

expect(deepNormalize(data.files)).toEqual(files);
Expand Down Expand Up @@ -1119,7 +1152,7 @@ describe('HasteMap', () => {
const invalidFilePath = path.join('fruits', 'invalid', 'file.js');
watchman.mockImplementation(async options => {
const {changedFiles} = await mockImpl(options);
changedFiles.set(invalidFilePath, ['', 34, 44, 0, []]);
changedFiles.set(invalidFilePath, ['', 34, 44, 0, [], null, 0]);
return {
changedFiles,
removedFiles: new Map(),
Expand Down Expand Up @@ -1214,7 +1247,7 @@ describe('HasteMap', () => {
node.mockImplementation(options => {
return Promise.resolve({
changedFiles: createMap({
[path.join('fruits', 'Banana.js')]: ['', 32, 42, 0, '', null],
[path.join('fruits', 'Banana.js')]: ['', 32, 42, 0, '', null, 0],
}),
removedFiles: new Map(),
});
Expand All @@ -1233,6 +1266,7 @@ describe('HasteMap', () => {
1,
'Strawberry',
null,
0,
],
}),
);
Expand All @@ -1250,7 +1284,7 @@ describe('HasteMap', () => {
node.mockImplementation(options => {
return Promise.resolve({
changedFiles: createMap({
[path.join('fruits', 'Banana.js')]: ['', 32, 42, 0, '', null],
[path.join('fruits', 'Banana.js')]: ['', 32, 42, 0, '', null, 0],
}),
removedFiles: new Map(),
});
Expand All @@ -1270,6 +1304,7 @@ describe('HasteMap', () => {
1,
'Strawberry',
null,
0,
],
}),
);
Expand Down
1 change: 1 addition & 0 deletions packages/metro-file-map/src/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const constants/*: HType */ = {
VISITED: 3,
DEPENDENCIES: 4,
SHA1: 5,
SYMLINK: 6,

/* module map attributes */
PATH: 0,
Expand Down
21 changes: 12 additions & 9 deletions packages/metro-file-map/src/crawlers/__tests__/integration-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
* LICENSE file in the root directory of this source tree.
*
* @format
* @flow strict-local
* @oncall react_native
*/

import nodeCrawl from '../node';
import watchmanCrawl from '../watchman';
import {execSync} from 'child_process';
import invariant from 'invariant';
import os from 'os';
import {join} from 'path';
import type {CrawlerOptions, FileData} from '../../flow-types';

// At runtime we use a more sophisticated + robust Watchman capability check,
// but this simple heuristic is fast to check, synchronous (we can't
Expand All @@ -33,10 +34,7 @@ const isWatchmanOnPath = () => {
const mockUseNativeFind = jest.fn();
jest.mock('../node/hasNativeFindSupport', () => () => mockUseNativeFind());

type Crawler = (opts: CrawlerOptions) => Promise<{
removedFiles: FileData,
changedFiles: FileData,
}>;
type Crawler = typeof nodeCrawl | typeof watchmanCrawl;

const CRAWLERS: {[key: string]: ?Crawler} = {
'node-find': opts => {
Expand All @@ -59,28 +57,33 @@ describe.each(Object.keys(CRAWLERS))(
const maybeTest = crawl ? test : test.skip;

maybeTest('Finds the expected files', async () => {
invariant(crawl, 'crawl should not be null within maybeTest');
const result = await crawl({
previousState: {
files: new Map([['removed.js', ['', 123, 234, 0, '', null]]]),
files: new Map([['removed.js', ['', 123, 234, 0, '', null, 0]]]),
clocks: new Map(),
},
enableSymlinks: false,
extensions: ['js'],
ignore: path => path.includes('ignored'),
roots: [FIXTURES_DIR],
rootDir: FIXTURES_DIR,
abortSignal: null,
computeSha1: false,
forceNodeFilesystemAPI: false,
onStatus: () => {},
});

// Map comparison is unordered, which is what we want
expect(result).toMatchObject({
changedFiles: new Map([
[
join('directory', 'bar.js'),
['', expect.any(Number), 245, 0, '', null],
['', expect.any(Number), 245, 0, '', null, 0],
],
['foo.js', ['', expect.any(Number), 245, 0, '', null]],
['foo.js', ['', expect.any(Number), 245, 0, '', null, 0]],
]),
removedFiles: new Map([['removed.js', ['', 123, 234, 0, '', null]]]),
removedFiles: new Map([['removed.js', ['', 123, 234, 0, '', null, 0]]]),
});
if (crawlerName === 'watchman') {
expect(result.clocks).toBeInstanceOf(Map);
Expand Down
Loading

0 comments on commit 58c88b8

Please sign in to comment.