diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/LocalDiffAwareness.java b/src/main/java/com/google/devtools/build/lib/skyframe/LocalDiffAwareness.java index 44a278586096ee..aedc46e2bf7f2a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/LocalDiffAwareness.java +++ b/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); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java b/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java index d8c5d175d735d7..016f9cd47d18cc 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java +++ b/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 watchRoot with a granularity of - * delay seconds. + * Watch changes on the file system under watchRoot with a granularity of delay + * seconds. */ - MacOSXFsEventsDiffAwareness(String watchRoot, double latency) { + MacOSXFsEventsDiffAwareness(Path watchRoot, double latency) { super(watchRoot); this.latency = latency; } - /** - * Watch changes on the file system under watchRoot with a granularity of 5ms. - */ - MacOSXFsEventsDiffAwareness(String watchRoot) { + /** Watch changes on the file system under watchRoot with a granularity of 5ms. */ + MacOSXFsEventsDiffAwareness(Path watchRoot) { this(watchRoot, 0.005); } /** - * Helper function to start the watch of paths, called by the constructor. + * Helper function to start the watch of paths, 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 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()); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WatchServiceDiffAwareness.java b/src/main/java/com/google/devtools/build/lib/skyframe/WatchServiceDiffAwareness.java index f38153e5257012..3e0e79f5ef00a6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WatchServiceDiffAwareness.java +++ b/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,7 +56,7 @@ 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; } @@ -65,7 +64,7 @@ public final class WatchServiceDiffAwareness extends LocalDiffAwareness { 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 modifiedAbsolutePaths; if (isFirstCall()) { try { - registerSubDirectories(watchRootPath); + registerSubDirectories(watchRoot); } catch (IOException e) { close(); throw new BrokenDiffAwarenessException( @@ -243,7 +242,7 @@ private Set 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 changedPaths = new HashSet<>(); diff --git a/src/main/native/darwin/fsevents.cc b/src/main/native/darwin/fsevents.cc index 76e933cbd07978..e38a6536016942 100644 --- a/src/main/native/darwin/fsevents.cc +++ b/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(it->c_str())); + env->SetObjectArrayElement(result, i, path); + env->DeleteLocalRef(path); } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwarenessTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwarenessTest.java index 8c65a4f1d76db3..9d23af672ba468 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwarenessTest.java +++ b/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); diff --git a/src/test/shell/integration/watchfs_test.sh b/src/test/shell/integration/watchfs_test.sh index db261d9a6e92c4..3d6ef647fdae3d 100755 --- a/src/test/shell/integration/watchfs_test.sh +++ b/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."