Watchman crawler: remove symlink_target

Summary:
Watchman "since" queries and subscriptions currently fail when `symlink_target` is requested and the result would include a deleted symlink *except* on Eden.

 - facebook/watchman#1084

Additionally, the Eden watcher lacks the potential bulk-fetch optimisation that would make it preferable to calling `readlink` ourselves:
 - https://github.com/facebook/watchman/blob/v2023.01.02.00/watchman/watcher/eden.cpp#L476-L485
 - https://github.com/facebook/watchman/blob/v2023.01.02.00/watchman/watcher/eden.cpp#L433-L434

This change removes the use of `symlink_target` until it is reliable and efficient on at least one backend.

Additionally, we gather and record the `watcher` from the `watch-project` response. In the first version of this diff we enabled `symlink_target` for `eden` only - the `watcher` field is now only used to provide an extra perf log annotation, but since it's gathered at no cost and potentially useful for Watcher-based query planning in future, I've left it in.

Changelog: [Internal] Remove `symlink_target` from Watchman crawler

Reviewed By: motiz88

Differential Revision: D42143103

fbshipit-source-id: 9f1cce64a7ef3c8ec3759f1e4c1e9317f728e118

flow-typed/fb-watchman.js

@@ -25,6 +25,7 @@ declare module 'fb-watchman' {
 
   declare type WatchmanWatchResponse = $ReadOnly<{
     watch: string,
+    watcher: string,
     relative_path: string,
     warning?: string,
     ...

packages/metro-file-map/src/crawlers/watchman/__tests__/planQuery-test.js

@@ -99,6 +99,22 @@ describe('planQuery with includeSymlinks: false', () => {
   });
 });
 
+it('does not request type if includeSymlinks == false', () => {
+  const {query, queryGenerator} = planQuery({
+    since: null,
+    directoryFilters: [],
+    extensions: ['js', 'ts'],
+    includeSha1: false,
+    includeSymlinks: false,
+  });
+  expect(queryGenerator).toBe('suffix');
+  expect(query).toEqual({
+    suffix: ['js', 'ts'],
+    expression: ['type', 'f'],
+    fields: ['name', 'exists', 'mtime_ms', 'size'],
+  });
+});
+
 describe('planQuery with includeSymlinks: true', () => {
   it('plans a "since" query when a clock and directories are given', () => {
     const {query, queryGenerator} = planQuery({
@@ -120,14 +136,7 @@ describe('planQuery with includeSymlinks: true', () => {
         ],
         ['anyof', ['dirname', '/dir1'], ['dirname', '/dir2']],
       ],
-      fields: [
-        'name',
-        'exists',
-        'mtime_ms',
-        'size',
-        'content.sha1hex',
-        'symlink_target',
-      ],
+      fields: ['name', 'exists', 'mtime_ms', 'size', 'content.sha1hex', 'type'],
     });
   });
 
@@ -147,14 +156,7 @@ describe('planQuery with includeSymlinks: true', () => {
         ['allof', ['type', 'f'], ['suffix', ['js', 'ts']]],
         ['type', 'l'],
       ],
-      fields: [
-        'name',
-        'exists',
-        'mtime_ms',
-        'size',
-        'content.sha1hex',
-        'symlink_target',
-      ],
+      fields: ['name', 'exists', 'mtime_ms', 'size', 'content.sha1hex', 'type'],
     });
   });
 
@@ -175,14 +177,7 @@ describe('planQuery with includeSymlinks: true', () => {
         ['allof', ['type', 'f'], ['suffix', ['js', 'ts']]],
         ['type', 'l'],
       ],
-      fields: [
-        'name',
-        'exists',
-        'mtime_ms',
-        'size',
-        'content.sha1hex',
-        'symlink_target',
-      ],
+      fields: ['name', 'exists', 'mtime_ms', 'size', 'content.sha1hex', 'type'],
     });
   });
 
@@ -201,14 +196,7 @@ describe('planQuery with includeSymlinks: true', () => {
         ['allof', ['type', 'f'], ['suffix', ['js', 'ts']]],
         ['type', 'l'],
       ],
-      fields: [
-        'name',
-        'exists',
-        'mtime_ms',
-        'size',
-        'content.sha1hex',
-        'symlink_target',
-      ],
+      fields: ['name', 'exists', 'mtime_ms', 'size', 'content.sha1hex', 'type'],
     });
   });
 
@@ -227,7 +215,7 @@ describe('planQuery with includeSymlinks: true', () => {
         ['allof', ['type', 'f'], ['suffix', ['js', 'ts']]],
         ['type', 'l'],
       ],
-      fields: ['name', 'exists', 'mtime_ms', 'size', 'symlink_target'],
+      fields: ['name', 'exists', 'mtime_ms', 'size', 'type'],
     });
   });
 });

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

@@ -29,7 +29,10 @@ import {performance} from 'perf_hooks';
 
 const watchman = require('fb-watchman');
 
-type WatchmanRoots = Map<string, Array<string>>;
+type WatchmanRoots = Map<
+  string,
+  $ReadOnly<{directoryFilters: Array<string>, watcher: string}>,
+>;
 
 const WATCHMAN_WARNING_INITIAL_DELAY_MILLISECONDS = 10000;
 const WATCHMAN_WARNING_INTERVAL_MILLISECONDS = 20000;
@@ -129,7 +132,7 @@ module.exports = async function watchmanCrawl({
     roots: $ReadOnlyArray<Path>,
   ): Promise<WatchmanRoots> {
     perfLogger?.point('watchmanCrawl/getWatchmanRoots_start');
-    const watchmanRoots = new Map<string, Array<string>>();
+    const watchmanRoots: WatchmanRoots = new Map();
     await Promise.all(
       roots.map(async (root, index) => {
         perfLogger?.point(`watchmanCrawl/watchProject_${index}_start`);
@@ -141,19 +144,24 @@ module.exports = async function watchmanCrawl({
         const existing = watchmanRoots.get(response.watch);
         // A root can only be filtered if it was never seen with a
         // relative_path before.
-        const canBeFiltered = !existing || existing.length > 0;
+        const canBeFiltered = !existing || existing.directoryFilters.length > 0;
 
         if (canBeFiltered) {
           if (response.relative_path) {
-            watchmanRoots.set(
-              response.watch,
-              (existing || []).concat(response.relative_path),
-            );
+            watchmanRoots.set(response.watch, {
+              watcher: response.watcher,
+              directoryFilters: (existing?.directoryFilters || []).concat(
+                response.relative_path,
+              ),
+            });
           } else {
             // Make the filter directories an empty array to signal that this
             // root was already seen and needs to be watched for all files or
             // directories.
-            watchmanRoots.set(response.watch, []);
+            watchmanRoots.set(response.watch, {
+              watcher: response.watcher,
+              directoryFilters: [],
+            });
           }
         }
       }),
@@ -168,7 +176,7 @@ module.exports = async function watchmanCrawl({
     let isFresh = false;
     await Promise.all(
       Array.from(rootProjectDirMappings).map(
-        async ([root, directoryFilters], index) => {
+        async ([root, {directoryFilters, watcher}], index) => {
           // Jest is only going to store one type of clock; a string that
           // represents a local clock. However, the Watchman crawler supports
           // a second type of clock that can be written by automation outside of
@@ -197,6 +205,7 @@ module.exports = async function watchmanCrawl({
 
           perfLogger?.annotate({
             string: {
+              [`watchmanCrawl/query_${index}_watcher`]: watcher ?? 'unknown',
               [`watchmanCrawl/query_${index}_generator`]: queryGenerator,
             },
           });
@@ -290,7 +299,7 @@ module.exports = async function watchmanCrawl({
     );
 
     for (const fileData of response.files) {
-      if (fileData.symlink_target != null) {
+      if (fileData.type === 'l') {
         // TODO: Process symlinks
         continue;
       }

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

@@ -35,8 +35,22 @@ export function planQuery({
   if (includeSha1) {
     fields.push('content.sha1hex');
   }
+
+  /**
+   * Note on symlink_target:
+   *
+   * Watchman supports requesting the symlink_target field, which is
+   * *potentially* more efficient if targets can be read from metadata without
+   * reading/materialising files. However, at the time of writing, Watchman has
+   * issues reporting symlink_target on some backends[1]. Additionally, though
+   * the Eden watcher is known to work, it reads links serially[2] on demand[3]
+   *  - less efficiently than we can do ourselves.
+   * [1] https://github.com/facebook/watchman/issues/1084
+   * [2] https://github.com/facebook/watchman/blob/v2023.01.02.00/watchman/watcher/eden.cpp#L476-L485
+   * [3] https://github.com/facebook/watchman/blob/v2023.01.02.00/watchman/watcher/eden.cpp#L433-L434
+   */
   if (includeSymlinks) {
-    fields.push('symlink_target');
+    fields.push('type');
   }
 
   const allOfTerms: Array<WatchmanExpression> = includeSymlinks