Fix --watchfs

src/main/java/com/google/devtools/build/lib/skyframe/LocalDiffAwareness.java

@@ -30,7 +30,6 @@
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsProvider;
 import java.io.IOException;
-import java.nio.file.FileSystems;
 import java.nio.file.Path;
 import java.util.Set;
 import javax.annotation.Nullable;
@@ -105,14 +104,14 @@ public DiffAwareness maybeCreate(
           return null;
         }
       }
-      String resolvedPath =
-          StringUtil.reencodeInternalToJava(resolvedPathEntryFragment.getPathString());
+      Path watchRoot =
+          Path.of(StringUtil.reencodeInternalToJava(resolvedPathEntryFragment.getPathString()));
       // On OSX uses FsEvents due to https://bugs.openjdk.java.net/browse/JDK-7133447
       if (OS.getCurrent() == OS.DARWIN) {
-        return new MacOSXFsEventsDiffAwareness(resolvedPath);
+        return new MacOSXFsEventsDiffAwareness(watchRoot);
       }
 
-      return new WatchServiceDiffAwareness(resolvedPath, ignoredPaths);
+      return new WatchServiceDiffAwareness(watchRoot, ignoredPaths);
     }
   }
 
@@ -135,10 +134,10 @@ public static boolean areInSequence(SequentialView oldView, SequentialView newVi
   private int numGetCurrentViewCalls = 0;
 
   /** Root directory to watch. This is an absolute path. */
-  protected final Path watchRootPath;
+  protected final Path watchRoot;
 
-  protected LocalDiffAwareness(String watchRoot) {
-    this.watchRootPath = FileSystems.getDefault().getPath(watchRoot);
+  protected LocalDiffAwareness(Path watchRoot) {
+    this.watchRoot = watchRoot;
   }
 
   /**
@@ -201,13 +200,13 @@ public ModifiedFileSet getDiff(@Nullable View oldView, View newView)
 
     ModifiedFileSet.Builder resultBuilder = ModifiedFileSet.builder();
     for (Path modifiedPath : newSequentialView.modifiedAbsolutePaths) {
-      if (!modifiedPath.startsWith(watchRootPath)) {
+      if (!modifiedPath.startsWith(watchRoot)) {
         throw new BrokenDiffAwarenessException(
-            String.format("%s is not under %s", modifiedPath, watchRootPath));
+            String.format("%s is not under %s", modifiedPath, watchRoot));
       }
       PathFragment relativePath =
           PathFragment.create(
-              StringUtil.reencodeJavaToInternal(watchRootPath.relativize(modifiedPath).toString()));
+              StringUtil.reencodeJavaToInternal(watchRoot.relativize(modifiedPath).toString()));
       if (!relativePath.isEmpty()) {
         resultBuilder.modify(relativePath);
       }

src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java

@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.skyframe;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.jni.JniLoader;
@@ -41,25 +43,24 @@ public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness {
   private boolean opened;
 
   /**
-   * Watch changes on the file system under <code>watchRoot</code> with a granularity of
-   * <code>delay</code> seconds.
+   * Watch changes on the file system under <code>watchRoot</code> with a granularity of <code>delay
+   * </code> seconds.
    */
-  MacOSXFsEventsDiffAwareness(String watchRoot, double latency) {
+  MacOSXFsEventsDiffAwareness(Path watchRoot, double latency) {
     super(watchRoot);
     this.latency = latency;
   }
 
-  /**
-   * Watch changes on the file system under <code>watchRoot</code> with a granularity of 5ms.
-   */
-  MacOSXFsEventsDiffAwareness(String watchRoot) {
+  /** Watch changes on the file system under <code>watchRoot</code> with a granularity of 5ms. */
+  MacOSXFsEventsDiffAwareness(Path watchRoot) {
     this(watchRoot, 0.005);
   }
 
   /**
-   * Helper function to start the watch of <code>paths</code>, called by the constructor.
+   * Helper function to start the watch of <code>paths</code>, which is expected to be an array of
+   * byte arrays containing the UTF-8 of the paths to watch, called by the constructor.
    */
-  private native void create(String[] paths, double latency);
+  private native void create(Object[] paths, double latency);
 
   /**
    * Runs the main loop to listen for fsevents.
@@ -76,7 +77,7 @@ private void init() {
     // TODO(jmmv): This can break if the user interrupts as anywhere in this function.
     Preconditions.checkState(!opened);
     opened = true;
-    create(new String[] {watchRootPath.toAbsolutePath().toString()}, latency);
+    create(new Object[] {watchRoot.toAbsolutePath().toString().getBytes(UTF_8)}, latency);
 
     // Start a thread that just contains the OS X run loop.
     CountDownLatch listening = new CountDownLatch(1);
@@ -110,10 +111,10 @@ public void close() {
   /**
    * JNI code returning the list of absolute path modified since last call.
    *
-   * @return the list of paths modified since the last call, or null if we can't precisely tell what
-   *     changed
+   * @return the array of paths (in the form of byte arrays containing the UTF-8 representation)
+   *     modified since the last call, or null if we can't precisely tell what changed
    */
-  private native String[] poll();
+  private native Object[] poll();
 
   static {
     boolean loadJniWorked = false;
@@ -147,13 +148,13 @@ public View getCurrentView(OptionsProvider options)
       return EVERYTHING_MODIFIED;
     }
     Preconditions.checkState(!closed);
-    String[] polledPaths = poll();
+    Object[] polledPaths = poll();
     if (polledPaths == null) {
       return EVERYTHING_MODIFIED;
     } else {
       ImmutableSet.Builder<Path> paths = ImmutableSet.builder();
-      for (String path : polledPaths) {
-        paths.add(Paths.get(path));
+      for (Object pathBytes : polledPaths) {
+        paths.add(Paths.get(new String((byte[]) pathBytes, UTF_8)));
       }
       return newView(paths.build());
     }

src/main/java/com/google/devtools/build/lib/skyframe/WatchServiceDiffAwareness.java

@@ -24,7 +24,6 @@
 import com.google.devtools.common.options.OptionsProvider;
 import java.io.IOException;
 import java.nio.file.ClosedWatchServiceException;
-import java.nio.file.FileSystems;
 import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
@@ -57,15 +56,15 @@ public final class WatchServiceDiffAwareness extends LocalDiffAwareness {
 
   private final IgnoredSubdirectories ignoredPaths;
 
-  WatchServiceDiffAwareness(String watchRoot, IgnoredSubdirectories ignoredPaths) {
+  WatchServiceDiffAwareness(Path watchRoot, IgnoredSubdirectories ignoredPaths) {
     super(watchRoot);
     this.ignoredPaths = ignoredPaths;
   }
 
   private void init() {
     Preconditions.checkState(watchService == null);
     try {
-      watchService = FileSystems.getDefault().newWatchService();
+      watchService = watchRoot.getFileSystem().newWatchService();
     } catch (IOException ignored) {
       // According to the docs, this can never happen with the default file system provider.
     }
@@ -125,7 +124,7 @@ public View getCurrentView(OptionsProvider options) throws BrokenDiffAwarenessEx
     Set<Path> modifiedAbsolutePaths;
     if (isFirstCall()) {
       try {
-        registerSubDirectories(watchRootPath);
+        registerSubDirectories(watchRoot);
       } catch (IOException e) {
         close();
         throw new BrokenDiffAwarenessException(
@@ -243,7 +242,7 @@ private Set<Path> collectChanges() throws BrokenDiffAwarenessException, IOExcept
     }
     if (watchKeyToDirBiMap.isEmpty()) {
       // No more directories to watch, something happened the root directory being watched.
-      throw new IOException("Root directory " + watchRootPath + " became inaccessible.");
+      throw new IOException("Root directory " + watchRoot + " became inaccessible.");
     }
 
     Set<Path> changedPaths = new HashSet<>();

src/main/native/darwin/fsevents.cc

@@ -98,18 +98,20 @@ Java_com_google_devtools_build_lib_skyframe_MacOSXFsEventsDiffAwareness_create(
   context.release = nullptr;
   context.copyDescription = nullptr;
 
-  // Create an CFArrayRef of CFStringRef from the Java array of String
+  // Create an CFArrayRef of CFStringRef from the Java array of UTF-8 byte
+  // strings.
   jsize length = env->GetArrayLength(paths);
   CFStringRef *pathsArray = new CFStringRef[length];
   for (int i = 0; i < length; i++) {
-    jstring path = (jstring)env->GetObjectArrayElement(paths, i);
-    const char *pathCStr = env->GetStringUTFChars(path, nullptr);
-    pathsArray[i] =
-        CFStringCreateWithCString(nullptr, pathCStr, kCFStringEncodingUTF8);
-    env->ReleaseStringUTFChars(path, pathCStr);
+    jbyteArray path = (jbyteArray)env->GetObjectArrayElement(paths, i);
+    jbyte* pathBytes = env->GetByteArrayElements(path, nullptr);
+    jsize pathLength = env->GetArrayLength(path);
+    pathsArray[i] = CFStringCreateWithBytes(
+        nullptr, (UInt8*) pathBytes, pathLength, kCFStringEncodingUTF8, false);
+    env->ReleaseByteArrayElements(path, pathBytes, JNI_ABORT);
   }
   CFArrayRef pathsToWatch =
-      CFArrayCreate(nullptr, (const void **)pathsArray, 1, nullptr);
+      CFArrayCreate(nullptr, (const void **)pathsArray, length, nullptr);
   delete[] pathsArray;
   info->stream = FSEventStreamCreate(
       nullptr, &FsEventsDiffAwarenessCallback, &context, pathsToWatch,
@@ -160,11 +162,15 @@ Java_com_google_devtools_build_lib_skyframe_MacOSXFsEventsDiffAwareness_poll(
   if (info->everything_changed) {
     result = nullptr;
   } else {
-    jclass classString = env->FindClass("java/lang/String");
-    result = env->NewObjectArray(info->paths.size(), classString, nullptr);
+    jclass classByteArray = env->FindClass("[B");
+    result = env->NewObjectArray(info->paths.size(), classByteArray, nullptr);
     int i = 0;
     for (auto it = info->paths.begin(); it != info->paths.end(); it++, i++) {
-      env->SetObjectArrayElement(result, i, env->NewStringUTF(it->c_str()));
+      jbyteArray path = env->NewByteArray(it->size());
+      env->SetByteArrayRegion(
+          path, 0, it->size(), reinterpret_cast<const jbyte *>(it->c_str()));
+      env->SetObjectArrayElement(result, i, path);
+      env->DeleteLocalRef(path);
     }
   }
 

src/test/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwarenessTest.java

@@ -80,7 +80,7 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx
   @Before
   public void setUp() throws Exception {
     watchedPath = com.google.common.io.Files.createTempDir().getCanonicalFile().toPath();
-    underTest = new MacOSXFsEventsDiffAwareness(watchedPath.toString());
+    underTest = new MacOSXFsEventsDiffAwareness(watchedPath);
     LocalDiffAwareness.Options localDiffOptions = new LocalDiffAwareness.Options();
     localDiffOptions.watchFS = true;
     watchFsEnabledProvider = FakeOptions.of(localDiffOptions);

src/test/shell/integration/watchfs_test.sh

@@ -99,4 +99,34 @@ function test_both() {
   expect_not_log "VFS stat.*${pkg}/whocares.in"
 }
 
+function test_special_chars() {
+  local -r subdir="${FUNCNAME[0]} bazel 🌱"
+  local -r workspace="$TEST_TMPDIR/$subdir/workspace"
+  mkdir -p "$workspace" || fail "mkdir -p $workspace"
+
+  cd "$workspace" || fail "cd $workspace"
+  setup_module_dot_bazel
+
+  touch BUILD
+  bazel build --watchfs //... &> "$TEST_log" || fail "Expected success."
+  expect_not_log "Hello, Unicode!"
+
+  mkdir pkg || fail "mkdir pkg"
+  cat > pkg/BUILD << 'EOF'
+print("Hello, Unicode!")
+
+genrule(
+    name = "foo",
+    outs = ["foo.out"],
+    cmd = "echo Hi there > $@",
+)
+EOF
+
+  sleep 1
+  bazel build --watchfs //... &> "$TEST_log" || fail "Expected success."
+  expect_not_log "WARNING:"
+  expect_log "Hello, Unicode!"
+  assert_contains "Hi there" "${PRODUCT_NAME}-bin/pkg/foo.out"
+}
+
 run_suite "Integration tests for --watchfs."