Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.73.x] Hotfix release 0.73.9 - sync watcher backends with 0.76.0 #949

Merged
merged 8 commits into from
Mar 21, 2023

Commits on Mar 21, 2023

  1. Refactor watcher change handling to promises

    Summary:
    Some lightweight modernising of `metro-file-map` watcher backends to allow the cleaner addition of an another async step (`readLink`), to follow.
    
    (Re `graceful-fs` -> `fs` in `FSEventsWatcher` - this only affects the call to `lstat`, which is [wrapped by `graceful-fs`](https://l.facebook.com/l.php?u=https%3A%2F%2Fgithub.com%2Fisaacs%2Fnode-graceful-fs%2Fblob%2F1f19b0b467e4144260b397343cd60f37c5bdcfda%2Fpolyfills.js%23L292&h=AT1P8P4IV8T7USaqoeaSrADFV0TYg4fSes1ACxBJTf1xD0thUYMKLdwbJ9DsI-jp3yG-j52_EJE7rec2VOa0H_0k5Prll3CAT747p8IGaxvZrscX40tFFtsWIB4OAB695JLank07cGR4vH57l13fVuxo) *only* to fix a very old `guid`/`uid` reporting issue irrelevant to us - in short, `graceful-fs` isn't adding anything here).
    
    Changelog: [Internal]
    
    Reviewed By: jacdebug
    
    Differential Revision: D41522808
    
    fbshipit-source-id: 03819fc489710d3091505a83a3babfa5aceaf4a7
    robhogan committed Mar 21, 2023
    Configuration menu
    Copy the full SHA
    f302c29 View commit details
    Browse the repository at this point in the history
  2. Add Watcher tests for behaviour on pre-existing files

    Summary:
    Refactor watcher integration tests to extract a lot of supporting code for clarity, and add some fixtures *before* creating the Watcher in order to test behaviour when pre-existing files are modified or deleted. Use those fixtures for two additional tests.
    
    (Motivation: Similar tests for symlinks have inconsistent results, fixed in the next diff)
    
    Changelog: [Internal]
    
    Reviewed By: jacdebug
    
    Differential Revision: D42110285
    
    fbshipit-source-id: 0721cbc5a41ce3eddef7ff4c4aff0b7ffff34156
    robhogan committed Mar 21, 2023
    Configuration menu
    Copy the full SHA
    c8416e2 View commit details
    Browse the repository at this point in the history
  3. Consistent reporting of symlink changes across watcher backends

    Summary:
    Currently, symlinks are reported (similarly to regular files) by all watcher backends when added or removed (if not ignored), *except* in some specific cases:
     - Symlinks discovered when adding a new subtree are not always reported by `NodeWatcher`.
     - Symlinks deleted are not reported by `FSEventsWatcher` or `NodeWatcher`.
    
    That's simply because we use `walker` to scan files, and it distinguishes symlinks from regular files - we were not listening to the `symlink` event.
     - In the case of symlinks in very new directories, that meant we didn't emit an `add` event unless we already had a watch on the parent - the scan is expected to race against the recursive watch, so this was non-deterministic.
     - In the case of symlink deletions, these were not reported by `NodeWatcher` or `FSEventsWatcher` because those rely on an internal set of "tracked" files provided by `walker`, and don't emit events for untracked files that no longer exist.
    
    This diff adds a listener for discovered symlinks and tests to verify pre-existing symlinks correctly have their deletion reported.
    
    Changelog: [Internal]
    
    *(symlink changes are not yet propagated out of `metro-file-map`, so this fix isn't technically observable)*
    
    Reviewed By: jacdebug
    
    Differential Revision: D41774723
    
    fbshipit-source-id: 0e9e7d0ecd0bdfbb316af7657e7f2599b5dfb49d
    robhogan committed Mar 21, 2023
    Configuration menu
    Copy the full SHA
    c4055fb View commit details
    Browse the repository at this point in the history
  4. Fix Windows platform detection, Watchman detection in tests

    Summary:
    While checking some `metro-file-map` behaviour on Windows I noticed the Watchman detection we use in integration tests was all wrong :)
     - `os.platform()` returns `win32` on Windows, not `windows`.
     - `where` works in a command prompt but not PowerShell - there it's usually an alias for `Where-Object`. `where.exe` works on both.
     - `where.exe` prints "INFO: Could not find files for the given pattern(s)" to stderr on no match. Use `/Q` for silent running - the exit code is all we need.
    
    Reviewed By: motiz88
    
    Differential Revision: D42266748
    
    fbshipit-source-id: 4494cd39dd94da7dc17442264a3828a35aaf9c30
    robhogan committed Mar 21, 2023
    Configuration menu
    Copy the full SHA
    740e9f0 View commit details
    Browse the repository at this point in the history
  5. Watcher backends: Remove unused hasIgnore

    Summary:
    The various "ignore" mechanisms in `metro-file-map` are in a bit of tangle - we have explicit and implicit glob ignores, a set of regex arrays, and a `dot` parameter to exclude dotfiles.
    
    This is a small step in the right direction by removing the `hasIgnore` variable that's always true for Metro, and has odd side effects when it's false.
    
    Changelog: [Internal]
    
    (There's no observable effect on Metro since we always set `ignorePattern`, and so `hasIgnore` is always `true`)
    
    Reviewed By: motiz88
    
    Differential Revision: D43234562
    
    fbshipit-source-id: 166dca72774f3d6c3fb8c6825bf1a8d429439999
    robhogan committed Mar 21, 2023
    Configuration menu
    Copy the full SHA
    b6d51d8 View commit details
    Browse the repository at this point in the history
  6. Watcher backends: Don't apply glob filter to symlink/directory change…

    … events
    
    Summary:
    Metro uses the `glob` filter of the Watcher backends to include:
     - `package.json`
     - Files with a watched extension
     - Health check files
    
    All of these only make sense to apply against regular files - all directory events are ignored downstream, and symlinks must be included because they may (at some point) point to a directory.
    
    Separately, we have an `ignorePattern` to exclude source control patterns and the user-provided `blockList`. It continues to make sense to apply `ignorePattern` to all events, and for some backends it helps avoid walking ignored subtrees unnecessarily.
    
    Currently, backends apply both the glob filter and ignore pattern to all files. This diff:
     - Modifies tests to more closely resemble the way Metro uses the backends - providing a glob filter to allowlist file extensions.
     - Adds a `type` parameter to `common.isFileIncluded` (-> `isIncluded`) so that glob patterns are only applied to known regular files.
    
    D43214089 follows by making similar changes downstream on the `Watcher`.
    
    Changelog: [Internal]
    
    Reviewed By: motiz88
    
    Differential Revision: D43234543
    
    fbshipit-source-id: f9f2289c74e7e0d4c119eee05e0ede76fe7230d4
    robhogan committed Mar 21, 2023
    Configuration menu
    Copy the full SHA
    615a884 View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    99bbe6e View commit details
    Browse the repository at this point in the history
  8. Bump to 0.73.9

    robhogan committed Mar 21, 2023
    Configuration menu
    Copy the full SHA
    b2a70b2 View commit details
    Browse the repository at this point in the history