Skip to content

Commit

Permalink
Rework HierarchicalFileWatcherUpdater to only watch root build dir
Browse files Browse the repository at this point in the history
  • Loading branch information
wolfs committed Aug 3, 2020
1 parent ecdbd99 commit 6a5735a
Show file tree
Hide file tree
Showing 13 changed files with 128 additions and 126 deletions.
Expand Up @@ -16,7 +16,6 @@

package org.gradle.internal.watch

import org.gradle.internal.os.OperatingSystem
import org.gradle.util.Requires
import org.gradle.util.TestPrecondition
import spock.lang.Issue
Expand All @@ -25,7 +24,6 @@ import spock.lang.Unroll
@Unroll
@Requires(TestPrecondition.SYMLINKS)
class SymlinkFileSystemWatchingIntegrationTest extends AbstractFileSystemWatchingIntegrationTest {
private static final String UNABLE_TO_WATCH_MESSAGE = "Unable to watch the file system for changes."

@Issue("https://github.com/gradle/gradle/issues/11851")
def "gracefully handle when declaring the same path as an input via symlinks"() {
Expand Down Expand Up @@ -142,53 +140,4 @@ class SymlinkFileSystemWatchingIntegrationTest extends AbstractFileSystemWatchin
then:
executedAndNotSkipped ":myTask"
}

def "disable file system watching when trying to watch symlinked directory"() {
def actualDir = file("parent/inputDir")
def symlink = file("symlinkedParent")
symlink.createLink(file("parent"))
def projectDir = file("projectDir")

def fileToChange = actualDir.file("actualFile")
fileToChange.createFile()

projectDir.file("build.gradle") << """
task myTask {
def outputFile = file("build/output.txt")
inputs.file("../symlinkedParent/inputDir/actualFile")
outputs.file(outputFile)
doLast {
outputFile.text = "Hello world"
}
}
"""
projectDir.file("settings.gradle").createFile()
executer.beforeExecute {
inDirectory(projectDir)
}

when:
withWatchFs().run "myTask"
then:
executedAndNotSkipped ":myTask"
if (OperatingSystem.current().macOsX) {
outputContains(UNABLE_TO_WATCH_MESSAGE)
outputContains("Unable to watch '${new File(symlink, "inputDir").absolutePath}' since itself or one of its parent is a symbolic link (canonical path: '${actualDir.absolutePath}')")
} else {
outputDoesNotContain(UNABLE_TO_WATCH_MESSAGE)
}

when:
withWatchFs().run "myTask"
then:
skipped(":myTask")

when:
file(fileToChange).text = "changed"
waitForChangesToBePickedUp()
withWatchFs().run "myTask"
then:
executedAndNotSkipped ":myTask"
}
}
Expand Up @@ -250,7 +250,7 @@ class WatchedDirectoriesFileSystemWatchingIntegrationTest extends AbstractFileSy
succeeds "help"
}

def "watches the roots of #repositoryType file repositories"() {
def "does not watch files in #repositoryType file repositories"() {
def repo = this."${repositoryType}"("repo")
def moduleA = repo.module('group', 'projectA', '9.1')
moduleA.publish()
Expand All @@ -271,7 +271,7 @@ class WatchedDirectoriesFileSystemWatchingIntegrationTest extends AbstractFileSy
when:
withWatchFs().run "retrieve", "--info"
then:
assertWatchedHierarchies([projectDir, moduleA."${artifactFileProperty}".parentFile])
assertWatchedHierarchies([projectDir])

where:
repositoryType | artifactFileProperty
Expand Down
Expand Up @@ -32,20 +32,20 @@ public interface FileWatcherUpdater extends SnapshotHierarchy.SnapshotDiffListen
*
* @throws WatchingNotSupportedException when the native watchers can't be updated.
*/
void updateRootProjectDirectories(Collection<File> updatedRootProjectDirectories);
void updateRootProjectDirectories(Collection<File> updatedRootProjectDirectories, SnapshotHierarchy root);

/**
* {@inheritDoc}.
*
* @throws WatchingNotSupportedException when the native watchers can't be updated.
*/
@Override
void changed(Collection<CompleteFileSystemLocationSnapshot> removedSnapshots, Collection<CompleteFileSystemLocationSnapshot> addedSnapshots);
void changed(Collection<CompleteFileSystemLocationSnapshot> removedSnapshots, Collection<CompleteFileSystemLocationSnapshot> addedSnapshots, SnapshotHierarchy root);

/**
* Notifies the updater that the build has been finished, so it can do some internal bookkeeping updates.
*
* Used by the hierarchical watchers to avoid stop watching root project directories during a build.
*/
void buildFinished();
void buildFinished(SnapshotHierarchy root);
}
Expand Up @@ -17,11 +17,10 @@
package org.gradle.internal.watch.registry.impl;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Multimap;
import net.rubygrapefruit.platform.file.FileWatcher;
import org.gradle.internal.file.FileMetadata;
import org.gradle.internal.snapshot.CompleteFileSystemLocationSnapshot;
import org.gradle.internal.snapshot.SnapshotHierarchy;
import org.gradle.internal.watch.registry.FileWatcherUpdater;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -49,7 +48,7 @@
* The root project directories are discovered as included builds are encountered at the start of a build, and then they are removed when the build finishes.
*
* This is the lifecycle for the watched root project directories:
* - During a build, there will be various calls to {@link #updateRootProjectDirectories(Collection)},
* - During a build, there will be various calls to {@link FileWatcherUpdater#updateRootProjectDirectories(Collection, org.gradle.internal.snapshot.SnapshotHierarchy)},
* each call augmenting the collection. The watchers will be updated accordingly.
* - When updating the watches, we watch root project directories or old root project directories instead of
* directories inside them.
Expand All @@ -61,12 +60,11 @@
public class HierarchicalFileWatcherUpdater implements FileWatcherUpdater {
private static final Logger LOGGER = LoggerFactory.getLogger(HierarchicalFileWatcherUpdater.class);

private final Multimap<String, Path> trackedDirectoriesForSnapshot = HashMultimap.create();

private final Set<Path> watchedHierarchies = new HashSet<>();

private final Set<Path> knownRootProjectDirectoriesFromCurrentBuild = new HashSet<>();
private final Set<Path> watchedRootProjectDirectoriesFromPreviousBuild = new HashSet<>();
private final Set<Path> allowedDirectoriesToWatch = new HashSet<>();

private final FileWatcher watcher;
private final FileSystemLocationToWatchValidator locationToWatchValidator;
Expand All @@ -77,27 +75,35 @@ public HierarchicalFileWatcherUpdater(FileWatcher watcher, FileSystemLocationToW
}

@Override
public void changed(Collection<CompleteFileSystemLocationSnapshot> removedSnapshots, Collection<CompleteFileSystemLocationSnapshot> addedSnapshots) {
removedSnapshots.forEach(snapshot -> trackedDirectoriesForSnapshot.removeAll(snapshot.getAbsolutePath()));
addedSnapshots.forEach(snapshot -> {
ImmutableList<Path> directoriesToWatch = SnapshotWatchedDirectoryFinder.getDirectoriesToWatch(snapshot);
trackedDirectoriesForSnapshot.putAll(snapshot.getAbsolutePath(), directoriesToWatch);
});
determineAndUpdateWatchedHierarchies();
public void changed(Collection<CompleteFileSystemLocationSnapshot> removedSnapshots, Collection<CompleteFileSystemLocationSnapshot> addedSnapshots, SnapshotHierarchy root) {
determineAndUpdateDirectoriesToWatch(root);
}

@Override
public void buildFinished() {
public void buildFinished(SnapshotHierarchy root) {
watchedRootProjectDirectoriesFromPreviousBuild.addAll(knownRootProjectDirectoriesFromCurrentBuild);
watchedRootProjectDirectoriesFromPreviousBuild.retainAll(watchedHierarchies);
knownRootProjectDirectoriesFromCurrentBuild.clear();
determineAndUpdateWatchedHierarchies();
allowedDirectoriesToWatch.clear();
allowedDirectoriesToWatch.addAll(watchedHierarchies);
determineAndUpdateDirectoriesToWatch(root);
LOGGER.warn("Watching {} directory hierarchies to track changes", watchedHierarchies.size());
LOGGER.info("Watched directory hierarchies: {}", watchedHierarchies);
}

private void determineAndUpdateDirectoriesToWatch(SnapshotHierarchy root) {
Set<Path> directoriesWithStuffInside = allowedDirectoriesToWatch.stream()
.filter(locationToWatch -> {
CheckIfNonEmptySnapshotVisitor checkIfNonEmptySnapshotVisitor = new CheckIfNonEmptySnapshotVisitor();
root.visitSnapshotRoots(locationToWatch.toString(), checkIfNonEmptySnapshotVisitor);
return !checkIfNonEmptySnapshotVisitor.empty;
})
.collect(Collectors.toSet());
updateWatchedHierarchies(directoriesWithStuffInside);
}

@Override
public void updateRootProjectDirectories(Collection<File> updatedRootProjectDirectories) {
public void updateRootProjectDirectories(Collection<File> updatedRootProjectDirectories, SnapshotHierarchy root) {
Set<Path> rootPaths = updatedRootProjectDirectories.stream()
.map(File::toPath)
.map(Path::toAbsolutePath)
Expand All @@ -109,23 +115,11 @@ public void updateRootProjectDirectories(Collection<File> updatedRootProjectDire
knownRootProjectDirectoriesFromCurrentBuild.addAll(newRootProjectDirectories);
watchedRootProjectDirectoriesFromPreviousBuild.removeAll(knownRootProjectDirectoriesFromCurrentBuild);

determineAndUpdateWatchedHierarchies();
}

private void determineAndUpdateWatchedHierarchies() {
Set<Path> hierarchiesToWatch = determineHierarchiesToWatch();
updateWatchedHierarchies(hierarchiesToWatch);
}

private Set<Path> determineHierarchiesToWatch() {
Set<Path> directoriesToWatch = trackedDirectoriesForSnapshot.values().stream()
.map(trackedDirectory -> Stream.concat(knownRootProjectDirectoriesFromCurrentBuild.stream(), watchedRootProjectDirectoriesFromPreviousBuild.stream())
.filter(trackedDirectory::startsWith)
.findFirst()
.orElse(trackedDirectory))
.collect(Collectors.toSet());
allowedDirectoriesToWatch.clear();
allowedDirectoriesToWatch.addAll(resolveHierarchiesToWatch(Stream.concat(knownRootProjectDirectoriesFromCurrentBuild.stream(), watchedRootProjectDirectoriesFromPreviousBuild.stream())
.collect(Collectors.toSet())));

return resolveHierarchiesToWatch(directoriesToWatch);
determineAndUpdateDirectoriesToWatch(root);
}

private void updateWatchedHierarchies(Set<Path> newHierarchiesToWatch) {
Expand Down Expand Up @@ -187,4 +181,19 @@ public interface FileSystemLocationToWatchValidator {

void validateLocationToWatch(File location);
}

private static class CheckIfNonEmptySnapshotVisitor implements SnapshotHierarchy.SnapshotVisitor {
private boolean empty = true;

@Override
public void visitSnapshotRoot(CompleteFileSystemLocationSnapshot rootSnapshot) {
if (rootSnapshot.getAccessType() == FileMetadata.AccessType.DIRECT) {
empty = false;
}
}

public boolean isEmpty() {
return empty;
}
}
}
Expand Up @@ -25,6 +25,7 @@
import org.gradle.internal.snapshot.CompleteDirectorySnapshot;
import org.gradle.internal.snapshot.CompleteFileSystemLocationSnapshot;
import org.gradle.internal.snapshot.FileSystemSnapshotVisitor;
import org.gradle.internal.snapshot.SnapshotHierarchy;
import org.gradle.internal.watch.WatchingNotSupportedException;
import org.gradle.internal.watch.registry.FileWatcherUpdater;
import org.slf4j.Logger;
Expand Down Expand Up @@ -52,7 +53,7 @@ public NonHierarchicalFileWatcherUpdater(FileWatcher fileWatcher) {
}

@Override
public void changed(Collection<CompleteFileSystemLocationSnapshot> removedSnapshots, Collection<CompleteFileSystemLocationSnapshot> addedSnapshots) {
public void changed(Collection<CompleteFileSystemLocationSnapshot> removedSnapshots, Collection<CompleteFileSystemLocationSnapshot> addedSnapshots, SnapshotHierarchy root) {
Map<String, Integer> changedWatchedDirectories = new HashMap<>();

removedSnapshots.forEach(snapshot -> {
Expand All @@ -71,12 +72,12 @@ public void changed(Collection<CompleteFileSystemLocationSnapshot> removedSnapsh
}

@Override
public void buildFinished() {
public void buildFinished(SnapshotHierarchy root) {
LOGGER.warn("Watching {} directories to track changes", watchedRoots.entrySet().size());
}

@Override
public void updateRootProjectDirectories(Collection<File> updatedRootProjectDirectories) {
public void updateRootProjectDirectories(Collection<File> updatedRootProjectDirectories, SnapshotHierarchy root) {
}

private void updateWatchedDirectories(Map<String, Integer> changedWatchDirectories) {
Expand Down
Expand Up @@ -35,9 +35,9 @@ public SnapshotCollectingDiffListener(Predicate<String> watchFilter) {
this.watchFilter = watchFilter;
}

public void publishSnapshotDiff(SnapshotHierarchy.SnapshotDiffListener snapshotDiffListener) {
public void publishSnapshotDiff(SnapshotHierarchy.SnapshotDiffListener snapshotDiffListener, SnapshotHierarchy root) {
if (!removedSnapshots.isEmpty() || !addedSnapshots.isEmpty()) {
snapshotDiffListener.changed(removedSnapshots, addedSnapshots);
snapshotDiffListener.changed(removedSnapshots, addedSnapshots, root);
}
}

Expand Down
Expand Up @@ -88,7 +88,7 @@ private SnapshotHierarchy updateRootNotifyingWatchers(SnapshotHierarchy currentR
} else {
SnapshotCollectingDiffListener diffListener = new SnapshotCollectingDiffListener(watchFilter);
SnapshotHierarchy newRoot = updateFunction.update(currentRoot, diffListener);
return withWatcherChangeErrorHandling(newRoot, () -> diffListener.publishSnapshotDiff(watchRegistry.getFileWatcherUpdater()));
return withWatcherChangeErrorHandling(newRoot, () -> diffListener.publishSnapshotDiff(watchRegistry.getFileWatcherUpdater(), newRoot));
}
}

Expand Down Expand Up @@ -119,7 +119,7 @@ public void registerRootDirectoryForWatching(File buildRootDirectory) {
}
return withWatcherChangeErrorHandling(
currentRoot,
() -> watchRegistry.getFileWatcherUpdater().updateRootProjectDirectories(rootDirectoriesForWatching)
() -> watchRegistry.getFileWatcherUpdater().updateRootProjectDirectories(rootDirectoriesForWatching, currentRoot)
);
});
}
Expand All @@ -140,7 +140,7 @@ public void beforeBuildFinished(boolean watchingEnabled) {
SnapshotHierarchy newRoot = removeSymbolicLinks(currentRoot);
newRoot = handleWatcherRegistryEvents(newRoot, "for current build");
if (watchRegistry != null) {
newRoot = withWatcherChangeErrorHandling(newRoot, () -> watchRegistry.getFileWatcherUpdater().buildFinished());
newRoot = withWatcherChangeErrorHandling(newRoot, () -> watchRegistry.getFileWatcherUpdater().buildFinished(currentRoot));
}
printStatistics(newRoot, "retains", "till next build");
return newRoot;
Expand Down Expand Up @@ -191,7 +191,7 @@ public void handleLostState() {
stopWatchingAndInvalidateHierarchy();
}
});
watchRegistry.getFileWatcherUpdater().updateRootProjectDirectories(rootDirectoriesForWatching);
watchRegistry.getFileWatcherUpdater().updateRootProjectDirectories(rootDirectoriesForWatching, currentRoot);
long endTime = System.currentTimeMillis() - startTime;
LOGGER.warn("Spent {} ms registering watches for file system events", endTime);
// TODO: Move start watching early enough so that the root is always empty
Expand Down
Expand Up @@ -60,7 +60,7 @@ abstract class AbstractFileWatcherUpdaterTest extends Specification {
void update(VirtualFileSystem.UpdateFunction updateFunction) {
def diffListener = new SnapshotCollectingDiffListener({ path -> true})
root = updateFunction.update(root, diffListener)
diffListener.publishSnapshotDiff(updater)
diffListener.publishSnapshotDiff(updater, root)
}
}

Expand Down

0 comments on commit 6a5735a

Please sign in to comment.