Skip to content

[release-2.8] Do not watch child directories of the sym link folders #22986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/compiler/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ namespace ts {
* Make `elements` into a `NodeArray<T>`. If `elements` is `undefined`, returns an empty `NodeArray<T>`.
*/
export function createNodeArray<T extends Node>(elements?: ReadonlyArray<T>, hasTrailingComma?: boolean): NodeArray<T> {
if (elements) {
if (!elements || elements === emptyArray) {
elements = [];
}
else {
if (isNodeArray(elements)) {
return elements;
}
}
else {
elements = [];
}

const array = <NodeArray<T>>elements;
array.pos = -1;
Expand Down
37 changes: 23 additions & 14 deletions src/compiler/sys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,10 @@ namespace ts {
/*@internal*/
export interface RecursiveDirectoryWatcherHost {
watchDirectory: HostWatchDirectory;
getAccessileSortedChildDirectories(path: string): ReadonlyArray<string>;
getAccessibleSortedChildDirectories(path: string): ReadonlyArray<string>;
directoryExists(dir: string): boolean;
filePathComparer: Comparer<string>;
realpath(s: string): string;
}

/**
Expand Down Expand Up @@ -392,9 +393,14 @@ namespace ts {
function watchChildDirectories(parentDir: string, existingChildWatches: ChildWatches, callback: DirectoryWatcherCallback): ChildWatches {
let newChildWatches: DirectoryWatcher[] | undefined;
enumerateInsertsAndDeletes<string, DirectoryWatcher>(
host.directoryExists(parentDir) ? host.getAccessileSortedChildDirectories(parentDir) : emptyArray,
host.directoryExists(parentDir) ? mapDefined(host.getAccessibleSortedChildDirectories(parentDir), child => {
const childFullName = getNormalizedAbsolutePath(child, parentDir);
// Filter our the symbolic link directories since those arent included in recursive watch
// which is same behaviour when recursive: true is passed to fs.watch
return host.filePathComparer(childFullName, host.realpath(childFullName)) === Comparison.EqualTo ? childFullName : undefined;
}) : emptyArray,
existingChildWatches,
(child, childWatcher) => host.filePathComparer(getNormalizedAbsolutePath(child, parentDir), childWatcher.dirName),
(child, childWatcher) => host.filePathComparer(child, childWatcher.dirName),
createAndAddChildDirectoryWatcher,
closeFileWatcher,
addChildDirectoryWatcher
Expand All @@ -406,7 +412,7 @@ namespace ts {
* Create new childDirectoryWatcher and add it to the new ChildDirectoryWatcher list
*/
function createAndAddChildDirectoryWatcher(childName: string) {
const result = createDirectoryWatcher(getNormalizedAbsolutePath(childName, parentDir), callback);
const result = createDirectoryWatcher(childName, callback);
addChildDirectoryWatcher(result);
}

Expand Down Expand Up @@ -594,14 +600,7 @@ namespace ts {
exit(exitCode?: number): void {
process.exit(exitCode);
},
realpath(path: string): string {
try {
return _fs.realpathSync(path);
}
catch {
return path;
}
},
realpath,
debugMode: some(<string[]>process.execArgv, arg => /^--(inspect|debug)(-brk)?(=\d+)?$/i.test(arg)),
tryEnableSourceMapsForHost() {
try {
Expand Down Expand Up @@ -682,8 +681,9 @@ namespace ts {
const watchDirectoryRecursively = createRecursiveDirectoryWatcher({
filePathComparer: useCaseSensitiveFileNames ? compareStringsCaseSensitive : compareStringsCaseInsensitive,
directoryExists,
getAccessileSortedChildDirectories: path => getAccessibleFileSystemEntries(path).directories,
watchDirectory
getAccessibleSortedChildDirectories: path => getAccessibleFileSystemEntries(path).directories,
watchDirectory,
realpath
});

return (directoryName, callback, recursive) => {
Expand Down Expand Up @@ -1026,6 +1026,15 @@ namespace ts {
return filter<string>(_fs.readdirSync(path), dir => fileSystemEntryExists(combinePaths(path, dir), FileSystemEntryKind.Directory));
}

function realpath(path: string): string {
try {
return _fs.realpathSync(path);
}
catch {
return path;
}
}

function getModifiedTime(path: string) {
try {
return _fs.statSync(path).mtime;
Expand Down
50 changes: 47 additions & 3 deletions src/harness/unittests/tscWatchMode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2355,9 +2355,53 @@ declare module "fs" {
verifyRenamingFileInSubFolder(TestFSWithWatch.Tsc_WatchDirectory.NonRecursiveWatchDirectory);
});

// it("uses non recursive dynamic polling when renaming file in subfolder", () => {
// verifyRenamingFileInSubFolder(TestFSWithWatch.Tsc_WatchDirectory.DynamicPolling);
// });
it("uses non recursive dynamic polling when renaming file in subfolder", () => {
verifyRenamingFileInSubFolder(TestFSWithWatch.Tsc_WatchDirectory.DynamicPolling);
});

it("when there are symlinks to folders in recursive folders", () => {
const cwd = "/home/user/projects/myproject";
const file1: FileOrFolder = {
path: `${cwd}/src/file.ts`,
content: `import * as a from "a"`
};
const tsconfig: FileOrFolder = {
path: `${cwd}/tsconfig.json`,
content: `{ "compilerOptions": { "extendedDiagnostics": true, "traceResolution": true }}`
};
const realA: FileOrFolder = {
path: `${cwd}/node_modules/reala/index.d.ts`,
content: `export {}`
};
const realB: FileOrFolder = {
path: `${cwd}/node_modules/realb/index.d.ts`,
content: `export {}`
};
const symLinkA: FileOrFolder = {
path: `${cwd}/node_modules/a`,
symLink: `${cwd}/node_modules/reala`
};
const symLinkB: FileOrFolder = {
path: `${cwd}/node_modules/b`,
symLink: `${cwd}/node_modules/realb`
};
const symLinkBInA: FileOrFolder = {
path: `${cwd}/node_modules/reala/node_modules/b`,
symLink: `${cwd}/node_modules/b`
};
const symLinkAInB: FileOrFolder = {
path: `${cwd}/node_modules/realb/node_modules/a`,
symLink: `${cwd}/node_modules/a`
};
const files = [file1, tsconfig, realA, realB, symLinkA, symLinkB, symLinkBInA, symLinkAInB];
const environmentVariables = createMap<string>();
environmentVariables.set("TSC_WATCHDIRECTORY", TestFSWithWatch.Tsc_WatchDirectory.NonRecursiveWatchDirectory);
const host = createWatchedSystem(files, { environmentVariables, currentDirectory: cwd });
createWatchOfConfigFile("tsconfig.json", host);
checkWatchedDirectories(host, emptyArray, /*recursive*/ true);
checkWatchedDirectories(host, [cwd, `${cwd}/node_modules`, `${cwd}/node_modules/@types`, `${cwd}/node_modules/reala`, `${cwd}/node_modules/realb`,
`${cwd}/node_modules/reala/node_modules`, `${cwd}/node_modules/realb/node_modules`, `${cwd}/src`], /*recursive*/ false);
});
});
});
}
42 changes: 26 additions & 16 deletions src/harness/virtualFileSystemWithWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ interface Array<T> {}`
}

interface Folder extends FSEntry {
entries: FSEntry[];
entries: SortedArray<FSEntry>;
}

interface SymLink extends FSEntry {
Expand Down Expand Up @@ -276,12 +276,14 @@ interface Array<T> {}`
DynamicPolling = "RecursiveDirectoryUsingDynamicPriorityPolling"
}

const timeIncrements = 1000;
export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost, ModuleResolutionHost {
args: string[] = [];

private readonly output: string[] = [];

private fs: Map<FSEntry> = createMap<FSEntry>();
private time = timeIncrements;
getCanonicalFileName: (s: string) => string;
private toPath: (f: string) => Path;
private timeoutCallbacks = new Callbacks();
Expand Down Expand Up @@ -310,28 +312,31 @@ interface Array<T> {}`
const watchDirectory: HostWatchDirectory = (directory, cb) => this.watchFile(directory, () => cb(directory), PollingInterval.Medium);
this.customRecursiveWatchDirectory = createRecursiveDirectoryWatcher({
directoryExists: path => this.directoryExists(path),
getAccessileSortedChildDirectories: path => this.getDirectories(path),
getAccessibleSortedChildDirectories: path => this.getDirectories(path),
filePathComparer: this.useCaseSensitiveFileNames ? compareStringsCaseSensitive : compareStringsCaseInsensitive,
watchDirectory
watchDirectory,
realpath: s => this.realpath(s)
});
}
else if (tscWatchDirectory === Tsc_WatchDirectory.NonRecursiveWatchDirectory) {
const watchDirectory: HostWatchDirectory = (directory, cb) => this.watchDirectory(directory, fileName => cb(fileName), /*recursive*/ false);
this.customRecursiveWatchDirectory = createRecursiveDirectoryWatcher({
directoryExists: path => this.directoryExists(path),
getAccessileSortedChildDirectories: path => this.getDirectories(path),
getAccessibleSortedChildDirectories: path => this.getDirectories(path),
filePathComparer: this.useCaseSensitiveFileNames ? compareStringsCaseSensitive : compareStringsCaseInsensitive,
watchDirectory
watchDirectory,
realpath: s => this.realpath(s)
});
}
else if (tscWatchDirectory === Tsc_WatchDirectory.DynamicPolling) {
const watchFile = createDynamicPriorityPollingWatchFile(this);
const watchDirectory: HostWatchDirectory = (directory, cb) => watchFile(directory, () => cb(directory), PollingInterval.Medium);
this.customRecursiveWatchDirectory = createRecursiveDirectoryWatcher({
directoryExists: path => this.directoryExists(path),
getAccessileSortedChildDirectories: path => this.getDirectories(path),
getAccessibleSortedChildDirectories: path => this.getDirectories(path),
filePathComparer: this.useCaseSensitiveFileNames ? compareStringsCaseSensitive : compareStringsCaseInsensitive,
watchDirectory
watchDirectory,
realpath: s => this.realpath(s)
});
}
}
Expand All @@ -355,6 +360,11 @@ interface Array<T> {}`
return s;
}

private now() {
this.time += timeIncrements;
return new Date(this.time);
}

reloadFS(fileOrFolderList: ReadonlyArray<FileOrFolder>, options?: Partial<ReloadWatchInvokeOptions>) {
const mapNewLeaves = createMap<true>();
const isNewFs = this.fs.size === 0;
Expand All @@ -381,8 +391,8 @@ interface Array<T> {}`
}
else {
currentEntry.content = fileOrDirectory.content;
currentEntry.modifiedTime = new Date();
this.fs.get(getDirectoryPath(currentEntry.path)).modifiedTime = new Date();
currentEntry.modifiedTime = this.now();
this.fs.get(getDirectoryPath(currentEntry.path)).modifiedTime = this.now();
if (options && options.invokeDirectoryWatcherInsteadOfFileChanged) {
this.invokeDirectoryWatcher(getDirectoryPath(currentEntry.fullPath), currentEntry.fullPath);
}
Expand All @@ -406,7 +416,7 @@ interface Array<T> {}`
}
else {
// Folder update: Nothing to do.
currentEntry.modifiedTime = new Date();
currentEntry.modifiedTime = this.now();
}
}
}
Expand Down Expand Up @@ -512,8 +522,8 @@ interface Array<T> {}`
}

private addFileOrFolderInFolder(folder: Folder, fileOrDirectory: File | Folder | SymLink, ignoreWatch?: boolean) {
folder.entries.push(fileOrDirectory);
folder.modifiedTime = new Date();
insertSorted(folder.entries, fileOrDirectory, (a, b) => compareStringsCaseSensitive(getBaseFileName(a.path), getBaseFileName(b.path)));
folder.modifiedTime = this.now();
this.fs.set(fileOrDirectory.path, fileOrDirectory);

if (ignoreWatch) {
Expand All @@ -528,7 +538,7 @@ interface Array<T> {}`
const baseFolder = this.fs.get(basePath) as Folder;
if (basePath !== fileOrDirectory.path) {
Debug.assert(!!baseFolder);
baseFolder.modifiedTime = new Date();
baseFolder.modifiedTime = this.now();
filterMutate(baseFolder.entries, entry => entry !== fileOrDirectory);
}
this.fs.delete(fileOrDirectory.path);
Expand Down Expand Up @@ -603,7 +613,7 @@ interface Array<T> {}`
return {
path: this.toPath(fullPath),
fullPath,
modifiedTime: new Date()
modifiedTime: this.now()
};
}

Expand All @@ -622,7 +632,7 @@ interface Array<T> {}`

private toFolder(path: string): Folder {
const folder = this.toFsEntry(path) as Folder;
folder.entries = [];
folder.entries = [] as SortedArray<FSEntry>;
return folder;
}

Expand All @@ -642,7 +652,7 @@ interface Array<T> {}`

const realpath = this.realpath(path);
if (path !== realpath) {
return this.getRealFsEntry(isFsEntry, realpath as Path);
return this.getRealFsEntry(isFsEntry, this.toPath(realpath));
}

return undefined;
Expand Down