[Do not merge] DirectoryWatcher rewrite

include/clang/DirectoryWatcher/DirectoryWatcher.h

@@ -1,67 +1,123 @@
 //===- DirectoryWatcher.h - Listens for directory file changes --*- C++ -*-===//
 //
-//                     The LLVM Compiler Infrastructure
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-/// \file
-/// \brief Utility class for listening for file system changes in a directory.
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_DIRECTORYWATCHER_DIRECTORYWATCHER_H
 #define LLVM_CLANG_DIRECTORYWATCHER_DIRECTORYWATCHER_H
 
-#include "clang/Basic/LLVM.h"
-#include "llvm/Support/Chrono.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
 #include <functional>
 #include <memory>
 #include <string>
 
 namespace clang {
-
-/// Provides notifications for file system changes in a directory.
+/// Provides notifications for file changes in a directory.
+///
+/// Invokes client-provided function on every filesystem event in the watched
+/// directory. Initially the the watched directory is scanned and for every file
+/// found, an event is synthesized as if the file was added.
+///
+/// This is not a general purpose directory monitoring tool - list of
+/// limitations follows.
+///
+/// Only flat directories with no subdirectories are supported. In case
+/// subdirectories are present the behavior is unspecified - events *might* be
+/// passed to Receiver on macOS (due to FSEvents being used) while they
+/// *probably* won't be passed on Linux (due to inotify being used).
+///
+/// Known potential inconsistencies
+/// - For files that are deleted befor the initial scan processed them, clients
+/// might receive Removed notification without any prior Added notification.
+/// - Multiple notifications might be produced when a file is added to the
+/// watched directory during the initial scan. We are choosing the lesser evil
+/// here as the only known alternative strategy would be to invalidate the
+/// watcher instance and force user to create a new one whenever filesystem
+/// event occurs during the initial scan but that would introduce continuous
+/// restarting failure mode (watched directory is not always "owned" by the same
+/// process that is consuming it). Since existing clients can handle duplicate
+/// events well, we decided for simplicity.
+///
+/// Notifications are provided only for changes done through local user-space
+/// filesystem interface. Specifically, it's unspecified if notification would
+/// be provided in case of a:
+/// - a file mmap-ed and changed
+/// - a file changed via remote (NFS) or virtual (/proc) FS access to monitored
+/// directory
+/// - another filesystem mounted to the watched directory
+///
+/// No support for LLVM VFS.
 ///
-/// Guarantees that the first time the directory is processed, the receiver will
-/// be invoked even if the directory is empty.
+/// It is unspecified whether notifications for files being deleted are sent in
+/// case the whole watched directory is sent.
+///
+/// Directories containing "too many" files and/or receiving events "too
+/// frequently" are not supported - if the initial scan can't be finished before
+/// the watcher instance gets invalidated (see WatcherGotInvalidated) there's no
+/// good error handling strategy - the only option for client is to destroy the
+/// watcher, restart watching with new instance and hope it won't repeat.
 class DirectoryWatcher {
 public:
-  enum class EventKind {
-    /// A file was added.
-    Added,
-    /// A file was removed.
-    Removed,
-    /// A file was modified.
-    Modified,
-    /// The watched directory got deleted. No more events will follow.
-    DirectoryDeleted,
-  };
-
   struct Event {
+    enum class EventKind {
+      Removed,
+      /// Content of a file was modified.
+      Modified,
+      /// The watched directory got deleted.
+      WatchedDirRemoved,
+      /// The DirectoryWatcher that originated this event is no longer valid and
+      /// its behavior is unspecified.
+      ///
+      /// The prime case is kernel signalling to OS-specific implementation of
+      /// DirectoryWatcher some resource limit being hit.
+      /// *Usually* kernel starts dropping or squashing events together after
+      /// that and so would DirectoryWatcher. This means that *some* events
+      /// might still be passed to Receiver but this behavior is unspecified.
+      ///
+      /// Another case is after the watched directory itself is deleted.
+      /// WatcherGotInvalidated will be received at least once during
+      /// DirectoryWatcher instance lifetime - when handling errors this is done
+      /// on best effort basis, when an instance is being destroyed then this is
+      /// guaranteed.
+      ///
+      /// The only proper response to this kind of event is to destruct the
+      /// originating DirectoryWatcher instance and create a new one.
+      WatcherGotInvalidated
+    };
+
     EventKind Kind;
+    /// Filename that this event is related to or an empty string in
+    /// case this event is related to the watched directory itself.
     std::string Filename;
-    llvm::sys::TimePoint<> ModTime;
-  };
-
-  typedef std::function<void(ArrayRef<Event> Events, bool isInitial)> EventReceiver;
 
-  ~DirectoryWatcher();
+    Event(EventKind Kind, llvm::StringRef Filename)
+        : Kind(Kind), Filename(Filename) {}
+  };
 
+  /// Returns nullptr if \param Path doesn't exist.
+  /// Returns nullptr if \param Path isn't a directory.
+  /// Returns nullptr if OS kernel API told us we can't start watching. In such
+  /// case it's unclear whether just retrying has any chance to succeeed.
   static std::unique_ptr<DirectoryWatcher>
-    create(StringRef Path, EventReceiver Receiver, bool waitInitialSync,
-           std::string &Error);
-
-private:
-  struct Implementation;
-  Implementation &Impl;
+  create(llvm::StringRef Path,
+         std::function<void(llvm::ArrayRef<DirectoryWatcher::Event> Events,
+                            bool IsInitial)>
+             Receiver,
+         bool WaitForInitialSync);
 
-  DirectoryWatcher();
+  virtual ~DirectoryWatcher() = default;
+  DirectoryWatcher(const DirectoryWatcher &) = delete;
+  DirectoryWatcher &operator=(const DirectoryWatcher &) = delete;
+  DirectoryWatcher(DirectoryWatcher &&) = default;
 
-  DirectoryWatcher(const DirectoryWatcher&) = delete;
-  DirectoryWatcher &operator =(const DirectoryWatcher&) = delete;
+protected:
+  DirectoryWatcher() = default;
 };
 
 } // namespace clang
 
-#endif
+#endif // LLVM_CLANG_DIRECTORYWATCHER_DIRECTORYWATCHER_H

include/clang/Index/IndexDataStore.h

@@ -36,16 +36,15 @@ class IndexDataStore {
   static unsigned getFormatVersion();
 
   enum class UnitEventKind {
-    Added,
     Removed,
     Modified,
     /// The directory got deleted. No more events will follow.
     DirectoryDeleted,
+    Failure
   };
   struct UnitEvent {
     UnitEventKind Kind;
     StringRef UnitName;
-    llvm::sys::TimePoint<> ModTime;
   };
   struct UnitEventNotification {
     bool IsInitial;

include/indexstore/IndexStoreCXX.h

@@ -19,7 +19,6 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
-#include <ctime>
 
 namespace indexstore {
   using llvm::ArrayRef;
@@ -152,28 +151,26 @@ class IndexStore {
     UnitEvent(indexstore_unit_event_t obj) : obj(obj) {}
 
     enum class Kind {
-      Added,
       Removed,
       Modified,
       DirectoryDeleted,
+      Failure
     };
     Kind getKind() const {
       indexstore_unit_event_kind_t c_k = indexstore_unit_event_get_kind(obj);
       Kind K;
       switch (c_k) {
-      case INDEXSTORE_UNIT_EVENT_ADDED: K = Kind::Added; break;
       case INDEXSTORE_UNIT_EVENT_REMOVED: K = Kind::Removed; break;
       case INDEXSTORE_UNIT_EVENT_MODIFIED: K = Kind::Modified; break;
       case INDEXSTORE_UNIT_EVENT_DIRECTORY_DELETED: K = Kind::DirectoryDeleted; break;
+      case INDEXSTORE_UNIT_EVENT_FAILURE: K = Kind::Failure; break;
       }
       return K;
     }
 
     StringRef getUnitName() const {
       return stringFromIndexStoreStringRef(indexstore_unit_event_get_unit_name(obj));
     }
-
-    timespec getModificationTime() const { return indexstore_unit_event_get_modification_time(obj); }
   };
 
   class UnitEventNotification {
@@ -258,24 +255,6 @@ class IndexStore {
     nameBuf.append(unitName.begin(), unitName.begin()+nameLen);
   }
 
-  llvm::Optional<timespec>
-  getUnitModificationTime(StringRef unitName, std::string &error) {
-    llvm::SmallString<64> buf = unitName;
-    int64_t seconds, nanoseconds;
-    indexstore_error_t c_err = nullptr;
-    bool err = indexstore_store_get_unit_modification_time(obj, buf.c_str(),
-      &seconds, &nanoseconds, &c_err);
-    if (err && c_err) {
-      error = indexstore_error_get_description(c_err);
-      indexstore_error_dispose(c_err);
-      return llvm::None;
-    }
-    timespec ts;
-    ts.tv_sec = seconds;
-    ts.tv_nsec = nanoseconds;
-    return ts;
-  }
-
   void purgeStaleData() {
     indexstore_store_purge_stale_data(obj);
   }

include/indexstore/indexstore.h

@@ -17,7 +17,6 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
-#include <time.h>
 
 /**
  * \brief The version constants for the Index Store C API.
@@ -134,10 +133,10 @@ INDEXSTORE_PUBLIC bool
 indexstore_unit_event_notification_is_initial(indexstore_unit_event_notification_t);
 
 typedef enum {
-  INDEXSTORE_UNIT_EVENT_ADDED = 1,
-  INDEXSTORE_UNIT_EVENT_REMOVED = 2,
-  INDEXSTORE_UNIT_EVENT_MODIFIED = 3,
-  INDEXSTORE_UNIT_EVENT_DIRECTORY_DELETED = 4,
+  INDEXSTORE_UNIT_EVENT_REMOVED = 1,
+  INDEXSTORE_UNIT_EVENT_MODIFIED = 2,
+  INDEXSTORE_UNIT_EVENT_DIRECTORY_DELETED = 3,
+  INDEXSTORE_UNIT_EVENT_FAILURE = 4,
 } indexstore_unit_event_kind_t;
 
 INDEXSTORE_PUBLIC indexstore_unit_event_kind_t
@@ -146,9 +145,6 @@ indexstore_unit_event_get_kind(indexstore_unit_event_t);
 INDEXSTORE_PUBLIC indexstore_string_ref_t
 indexstore_unit_event_get_unit_name(indexstore_unit_event_t);
 
-INDEXSTORE_PUBLIC struct timespec
-indexstore_unit_event_get_modification_time(indexstore_unit_event_t);
-
 #if INDEXSTORE_HAS_BLOCKS
 typedef void (^indexstore_unit_event_handler_t)(indexstore_unit_event_notification_t);
 
@@ -197,14 +193,6 @@ indexstore_store_get_unit_name_from_output_path(indexstore_t store,
                                                 char *name_buf,
                                                 size_t buf_size);
 
-/// \returns true if an error occurred, false otherwise.
-INDEXSTORE_PUBLIC bool
-indexstore_store_get_unit_modification_time(indexstore_t store,
-                                            const char *unit_name,
-                                            int64_t *seconds,
-                                            int64_t *nanoseconds,
-                                            indexstore_error_t *error);
-
 typedef void *indexstore_symbol_t;
 
 typedef enum {

lib/DirectoryWatcher/CMakeLists.txt

@@ -2,13 +2,26 @@ include(CheckIncludeFiles)
 
 set(LLVM_LINK_COMPONENTS support)
 
-add_clang_library(clangDirectoryWatcher
-  DirectoryWatcher.cpp
-  )
+set(DIRECTORY_WATCHER_SOURCES DirectoryScanner.cpp)
+set(DIRECTORY_WATCHER_LINK_LIBS "")
 
 if(APPLE)
-  check_include_files("CoreServices/CoreServices.h" HAVE_CORESERVICES_H)
-  if(HAVE_CORESERVICES_H)
-    target_link_libraries(clangDirectoryWatcher PRIVATE "-framework CoreServices")
+  check_include_files("CoreServices/CoreServices.h" HAVE_CORESERVICES)
+  if(HAVE_CORESERVICES)
+    list(APPEND DIRECTORY_WATCHER_SOURCES mac/DirectoryWatcher-mac.cpp)
+    set(DIRECTORY_WATCHER_LINK_LIBS "-framework CoreServices")
+  endif()
+elseif(CMAKE_SYSTEM_NAME MATCHES "Linux")
+  check_include_files("sys/inotify.h" HAVE_INOTIFY)
+  if(HAVE_INOTIFY)
+    list(APPEND DIRECTORY_WATCHER_SOURCES linux/DirectoryWatcher-linux.cpp)
+    find_package(Threads REQUIRED)
+    set(DIRECTORY_WATCHER_LINK_LIBS ${CMAKE_THREAD_LIBS_INIT})
   endif()
 endif()
+
+add_clang_library(clangDirectoryWatcher
+  ${DIRECTORY_WATCHER_SOURCES}
+  )
+
+target_link_libraries(clangDirectoryWatcher PRIVATE ${DIRECTORY_WATCHER_LINK_LIBS})

lib/DirectoryWatcher/DirectoryScanner.cpp

@@ -0,0 +1,54 @@
+//===- DirectoryScanner.cpp - Utility functions for DirectoryWatcher ------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DirectoryScanner.h"
+
+#include "llvm/Support/Path.h"
+
+namespace clang {
+
+using namespace llvm;
+
+Optional<sys::fs::file_status> getFileStatus(StringRef Path) {
+  sys::fs::file_status Status;
+  std::error_code EC = status(Path, Status);
+  if (EC)
+    return None;
+  return Status;
+}
+
+std::vector<std::string> scanDirectory(StringRef Path) {
+  using namespace llvm::sys;
+  std::vector<std::string> Result;
+
+  std::error_code EC;
+  for (auto It = fs::directory_iterator(Path, EC),
+            End = fs::directory_iterator();
+       !EC && It != End; It.increment(EC)) {
+    auto status = getFileStatus(It->path());
+    if (!status.hasValue())
+      continue;
+    Result.emplace_back(sys::path::filename(It->path()));
+  }
+
+  return Result;
+}
+
+std::vector<DirectoryWatcher::Event>
+getAsFileEvents(const std::vector<std::string> &Scan) {
+  std::vector<DirectoryWatcher::Event> Events;
+  Events.reserve(Scan.size());
+
+  for (const auto &File : Scan) {
+    Events.emplace_back(DirectoryWatcher::Event{
+        DirectoryWatcher::Event::EventKind::Modified, File});
+  }
+  return Events;
+}
+
+} // namespace clang