Remove the experimental fs watcher flag and hardcode it to enabled.

[ci skip-jvm-tests]

[ci skip-rust-tests]

src/python/pants/engine/internals/native.py

@@ -1003,7 +1003,6 @@ def ti(type_obj):
             execution_options.process_execution_use_local_cache,
             self.context.utf8_dict(execution_options.remote_execution_headers),
             execution_options.process_execution_local_enable_nailgun,
-            execution_options.experimental_fs_watcher,
         )
         if scheduler_result.is_throw:
             value = self.context.from_value(scheduler_result.throw_handle)

src/python/pants/option/global_options.py

@@ -102,7 +102,6 @@ class ExecutionOptions:
     remote_execution_extra_platform_properties: Any
     remote_execution_headers: Any
     process_execution_local_enable_nailgun: bool
-    experimental_fs_watcher: bool
 
     @classmethod
     def from_bootstrap_options(cls, bootstrap_options):
@@ -128,7 +127,6 @@ def from_bootstrap_options(cls, bootstrap_options):
             remote_execution_extra_platform_properties=bootstrap_options.remote_execution_extra_platform_properties,
             remote_execution_headers=bootstrap_options.remote_execution_headers,
             process_execution_local_enable_nailgun=bootstrap_options.process_execution_local_enable_nailgun,
-            experimental_fs_watcher=bootstrap_options.experimental_fs_watcher,
         )
 
 
@@ -154,7 +152,6 @@ def from_bootstrap_options(cls, bootstrap_options):
     remote_execution_extra_platform_properties=[],
     remote_execution_headers={},
     process_execution_local_enable_nailgun=False,
-    experimental_fs_watcher=True,
 )
 
 
@@ -903,6 +900,8 @@ def register_bootstrap_options(cls, register):
             type=bool,
             default=True,
             advanced=True,
+            removal_version="1.29.0.dev2",
+            removal_hint="Enabled by default: flag is disabled.",
             help="Whether to use the engine filesystem watcher which registers the workspace"
             " for kernel file change events",
         )
@@ -1063,6 +1062,8 @@ def validate_instance(cls, opts):
         Raises pants.option.errors.OptionsError on validation failure.
         """
         if opts.get("loop") and not opts.enable_pantsd:
+            # TODO: This remains the case today because there are two spots that
+            # call `run_goal_rules`: fixing in a followup.
             raise OptionsError(
                 "The `--loop` option requires `--enable-pantsd`, in order to watch files."
             )

src/rust/engine/engine_cffi/src/lib.rs

@@ -239,7 +239,6 @@ pub extern "C" fn scheduler_create(
   process_execution_use_local_cache: bool,
   remote_execution_headers_buf: BufferBuffer,
   process_execution_local_enable_nailgun: bool,
-  experimental_fs_watcher: bool,
 ) -> RawResult {
   match make_core(
     tasks_ptr,
@@ -270,7 +269,6 @@ pub extern "C" fn scheduler_create(
     process_execution_use_local_cache,
     remote_execution_headers_buf,
     process_execution_local_enable_nailgun,
-    experimental_fs_watcher,
   ) {
     Ok(core) => RawResult {
       is_throw: false,
@@ -314,7 +312,6 @@ fn make_core(
   process_execution_use_local_cache: bool,
   remote_execution_headers_buf: BufferBuffer,
   process_execution_local_enable_nailgun: bool,
-  experimental_fs_watcher: bool,
 ) -> Result<Core, String> {
   let root_type_ids = root_type_ids.to_vec();
   let ignore_patterns = ignore_patterns_buf
@@ -426,7 +423,6 @@ fn make_core(
     process_execution_use_local_cache,
     remote_execution_headers,
     process_execution_local_enable_nailgun,
-    experimental_fs_watcher,
   )
 }
 

src/rust/engine/src/context.rs

@@ -91,7 +91,6 @@ impl Core {
     process_execution_use_local_cache: bool,
     remote_execution_headers: BTreeMap<String, String>,
     process_execution_local_enable_nailgun: bool,
-    experimental_fs_watcher: bool,
   ) -> Result<Core, String> {
     // Randomize CAS address order to avoid thundering herds from common config.
     let mut remote_store_servers = remote_store_servers;
@@ -266,12 +265,7 @@ impl Core {
       GitignoreStyleExcludes::create_with_gitignore_file(ignore_patterns, gitignore_file)
         .map_err(|e| format!("Could not parse build ignore patterns: {:?}", e))?;
 
-    let watcher = InvalidationWatcher::new(
-      executor.clone(),
-      build_root.clone(),
-      ignorer.clone(),
-      experimental_fs_watcher,
-    )?;
+    let watcher = InvalidationWatcher::new(executor.clone(), build_root.clone(), ignorer.clone())?;
     executor.block_on({
       let watcher = watcher.clone();
       let graph = graph.clone();
@@ -384,6 +378,7 @@ impl NodeContext for Context {
       entry_id: Some(entry_id),
       core: self.core.clone(),
       session: self.session.clone(),
+      run_id: self.run_id,
     }
   }
 

src/rust/engine/watch/src/lib.rs

@@ -54,7 +54,6 @@ struct Inner {
   invalidatables: Vec<Weak<dyn Invalidatable>>,
   executor: Executor,
   liveness: Receiver<String>,
-  enabled: bool,
   // Until the background task has started, contains the relevant inputs to launch it via
   // start_background_thread. The decoupling of creating the `InvalidationWatcher` and starting it
   // is to allow for testing of the background thread.
@@ -75,7 +74,6 @@ impl InvalidationWatcher {
     executor: Executor,
     build_root: PathBuf,
     ignorer: Arc<GitignoreStyleExcludes>,
-    enabled: bool,
   ) -> Result<Arc<InvalidationWatcher>, String> {
     // Inotify events contain canonical paths to the files being watched.
     // If the build_root contains a symlink the paths returned in notify events
@@ -88,28 +86,27 @@ impl InvalidationWatcher {
       .map_err(|e| format!("Failed to begin watching the filesystem: {}", e))?;
 
     let (liveness_sender, liveness_receiver) = crossbeam_channel::unbounded();
-    if enabled {
-      // On darwin the notify API is much more efficient if you watch the build root
-      // recursively, so we set up that watch here and then return early when watch() is
-      // called by nodes that are running. On Linux the notify crate handles adding paths to watch
-      // much more efficiently so we do that instead on Linux.
-      if cfg!(target_os = "macos") {
-        watcher
-          .watch(canonical_build_root.clone(), RecursiveMode::Recursive)
-          .map_err(|e| {
-            format!(
-              "Failed to begin recursively watching files in the build root: {}",
-              e
-            )
-          })?
-      }
+
+    // On darwin the notify API is much more efficient if you watch the build root
+    // recursively, so we set up that watch here and then return early when watch() is
+    // called by nodes that are running. On Linux the notify crate handles adding paths to watch
+    // much more efficiently so we do that instead on Linux.
+    if cfg!(target_os = "macos") {
+      watcher
+        .watch(canonical_build_root.clone(), RecursiveMode::Recursive)
+        .map_err(|e| {
+          format!(
+            "Failed to begin recursively watching files in the build root: {}",
+            e
+          )
+        })?
     }
+
     Ok(Arc::new(InvalidationWatcher(Mutex::new(Inner {
       watcher,
       invalidatables: Vec::new(),
       executor,
       liveness: liveness_receiver,
-      enabled,
       background_task_inputs: Some((
         ignorer,
         canonical_build_root,
@@ -293,13 +290,14 @@ impl InvalidationWatcher {
   /// Add a path to the set of paths being watched by this invalidation watcher, non-recursively.
   ///
   pub async fn watch(self: &Arc<Self>, path: PathBuf) -> Result<(), notify::Error> {
+    if cfg!(target_os = "macos") {
+      // Short circuit here if we are on a Darwin platform because we should be watching
+      // the entire build root recursively already.
+      return Ok(());
+    }
+
     let executor = {
       let inner = self.0.lock();
-      if cfg!(target_os = "macos") || !inner.enabled {
-        // Short circuit here if we are on a Darwin platform because we should be watching
-        // the entire build root recursively already, or if we are not enabled.
-        return Ok(());
-      }
       inner.executor.clone()
     };
 

src/rust/engine/watch/src/tests.rs

@@ -34,7 +34,7 @@ async fn setup_watch(
   file_path: PathBuf,
 ) -> Arc<InvalidationWatcher> {
   let executor = Executor::new(Handle::current());
-  let watcher = InvalidationWatcher::new(executor, build_root, ignorer, /*enabled*/ true)
+  let watcher = InvalidationWatcher::new(executor, build_root, ignorer)
     .expect("Couldn't create InvalidationWatcher");
   watcher.add_invalidatable(invalidatable).await;
   watcher.watch(file_path).await.unwrap();

tests/python/pants_test/init/test_options_initializer.py

@@ -22,16 +22,9 @@ def test_invalid_version(self):
     def test_global_options_validation(self):
         # Specify an invalid combination of options.
         ob = OptionsBootstrapper.create(
-            args=[
-                "--backend-packages=[]",
-                "--backend-packages2=[]",
-                "--v2",
-                "--no-v1",
-                "--loop",
-                "--no-enable-pantsd",
-            ]
+            args=["--backend-packages=[]", "--backend-packages2=[]", "--remote-execution",]
         )
         build_config = BuildConfigInitializer.get(ob)
         with self.assertRaises(OptionsError) as exc:
             OptionsInitializer.create(ob, build_config)
-        self.assertIn("The `--loop` option requires `--enable-pantsd`", str(exc.exception))
+        self.assertIn("The `--remote-execution` option requires", str(exc.exception))