From 198ad60b6824dbb62daa16f98be2861116ee75e2 Mon Sep 17 00:00:00 2001 From: Joe Hillenbrand Date: Fri, 16 Jun 2023 17:22:51 -0700 Subject: [PATCH] fix(fs/walk): return symbolic links (#1359) Symbolic links were not returned. This means that `walk()` is not listing all the contents of a directory. Some use cases for this is are: - Copying a directory recursively. - Finding invalid symbolic links. - Modify link targets without resolving them. Other changes: - walk_test.ts: Rename `walkArray()` to `collectPaths()` because it is more descriptive. - walk_test.ts: Add `collectEntries()` --- fs/walk.ts | 10 ++++- fs/walk_test.ts | 104 +++++++++++++++++++++++++++++++++++------------- 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/fs/walk.ts b/fs/walk.ts index fc7c746e0061e..3d48c8986513d 100644 --- a/fs/walk.ts +++ b/fs/walk.ts @@ -108,7 +108,10 @@ export async function* walk( let { isSymlink, isDirectory } = entry; if (isSymlink) { - if (!followSymlinks) continue; + if (!followSymlinks) { + yield { path, ...entry }; + continue; + } path = await Deno.realPath(path); // Caveat emptor: don't assume |path| is not a symlink. realpath() // resolves symlinks but another process can replace the file system @@ -171,7 +174,10 @@ export function* walkSync( let { isSymlink, isDirectory } = entry; if (isSymlink) { - if (!followSymlinks) continue; + if (!followSymlinks) { + yield { path, ...entry }; + continue; + } path = Deno.realPathSync(path); // Caveat emptor: don't assume |path| is not a symlink. realpath() // resolves symlinks but another process can replace the file system diff --git a/fs/walk_test.ts b/fs/walk_test.ts index 5476a498951f9..2f89186c6b042 100644 --- a/fs/walk_test.ts +++ b/fs/walk_test.ts @@ -4,7 +4,9 @@ import { walk, WalkEntry, WalkError, WalkOptions, walkSync } from "./walk.ts"; import { assert, assertEquals, + assertObjectMatch, assertRejects, + assertStrictEquals, assertThrows, } from "../testing/asserts.ts"; @@ -33,7 +35,35 @@ function normalize({ path }: WalkEntry): string { return path.replace(/\\/g, "/"); } -export async function walkArray( +/** + * Ensure that `walk()` and `walkSync()` return the same entries. + */ +export async function collectEntries( + root: string, + options: WalkOptions = {}, +): Promise { + const entries: WalkEntry[] = []; + for await (const w of walk(root, { ...options })) { + entries.push({ + ...w, + path: normalize(w) + }); + } + const entriesSync = Array.from(walkSync(root, options), (w) => { + return { + ...w, + path: normalize(w) + } + }); + assertEquals(entries, entriesSync); + return entries; +} + +/** + * Collect and compare the paths from `walk()` and `walkSync()` instead of the + * whole WalkEntry as in `collectEntries()`. + */ +export async function collectPaths( root: string, options: WalkOptions = {}, ): Promise { @@ -55,7 +85,6 @@ export async function touch(path: string) { function assertReady(expectedLength: number) { const arr = Array.from(walkSync("."), normalize); - assertEquals(arr.length, expectedLength); } @@ -64,7 +93,7 @@ testWalk( await Deno.mkdir(d + "/empty"); }, async function emptyDir() { - const arr = await walkArray("."); + const arr = await collectPaths("."); assertEquals(arr, [".", "empty"]); }, ); @@ -74,7 +103,7 @@ testWalk( await touch(d + "/x"); }, async function singleFile() { - const arr = await walkArray("."); + const arr = await collectPaths("."); assertEquals(arr, [".", "x"]); }, ); @@ -102,7 +131,7 @@ testWalk( await touch(d + "/a/x"); }, async function nestedSingleFile() { - const arr = await walkArray("."); + const arr = await collectPaths("."); assertEquals(arr, [".", "a", "a/x"]); }, ); @@ -114,9 +143,9 @@ testWalk( }, async function depth() { assertReady(6); - const arr3 = await walkArray(".", { maxDepth: 3 }); + const arr3 = await collectPaths(".", { maxDepth: 3 }); assertEquals(arr3, [".", "a", "a/b", "a/b/c"]); - const arr5 = await walkArray(".", { maxDepth: 5 }); + const arr5 = await collectPaths(".", { maxDepth: 5 }); assertEquals(arr5, [".", "a", "a/b", "a/b/c", "a/b/c/d", "a/b/c/d/x"]); }, ); @@ -129,7 +158,7 @@ testWalk( }, async function includeDirs() { assertReady(4); - const arr = await walkArray(".", { includeDirs: false }); + const arr = await collectPaths(".", { includeDirs: false }); assertEquals(arr, ["a", "b/c"]); }, ); @@ -142,7 +171,7 @@ testWalk( }, async function includeFiles() { assertReady(4); - const arr = await walkArray(".", { includeFiles: false }); + const arr = await collectPaths(".", { includeFiles: false }); assertEquals(arr, [".", "b"]); }, ); @@ -154,7 +183,7 @@ testWalk( }, async function ext() { assertReady(3); - const arr = await walkArray(".", { exts: [".ts"] }); + const arr = await collectPaths(".", { exts: [".ts"] }); assertEquals(arr, ["x.ts"]); }, ); @@ -167,7 +196,7 @@ testWalk( }, async function extAny() { assertReady(4); - const arr = await walkArray(".", { exts: [".rs", ".ts"] }); + const arr = await collectPaths(".", { exts: [".rs", ".ts"] }); assertEquals(arr, ["x.ts", "y.rs"]); }, ); @@ -179,7 +208,7 @@ testWalk( }, async function match() { assertReady(3); - const arr = await walkArray(".", { match: [/x/] }); + const arr = await collectPaths(".", { match: [/x/] }); assertEquals(arr, ["x"]); }, ); @@ -192,7 +221,7 @@ testWalk( }, async function matchAny() { assertReady(4); - const arr = await walkArray(".", { match: [/x/, /y/] }); + const arr = await collectPaths(".", { match: [/x/, /y/] }); assertEquals(arr, ["x", "y"]); }, ); @@ -204,7 +233,7 @@ testWalk( }, async function skip() { assertReady(3); - const arr = await walkArray(".", { skip: [/x/] }); + const arr = await collectPaths(".", { skip: [/x/] }); assertEquals(arr, [".", "y"]); }, ); @@ -217,7 +246,7 @@ testWalk( }, async function skipAny() { assertReady(4); - const arr = await walkArray(".", { skip: [/x/, /y/] }); + const arr = await collectPaths(".", { skip: [/x/, /y/] }); assertEquals(arr, [".", "z"]); }, ); @@ -232,7 +261,7 @@ testWalk( }, async function subDir() { assertReady(6); - const arr = await walkArray("b"); + const arr = await collectPaths("b"); assertEquals(arr, ["b", "b/z"]); }, ); @@ -241,11 +270,32 @@ testWalk( async (_d: string) => {}, async function nonexistentRoot() { await assertRejects(async () => { - await walkArray("nonexistent"); + await collectPaths("nonexistent"); }, Deno.errors.NotFound); + }, ); +testWalk( + async (d: string) => { + await Deno.mkdir(d + "/a"); + await Deno.mkdir(d + "/b"); + await touch(d + "/a/x"); + await Deno.symlink(d + "/b", d + "/a/l2b"); + await Deno.symlink(d + "/b/z", d + "/a/l2z"); + }, + async function symlinks() { + assertReady(6); + const entries = await collectEntries("a"); + assertEquals(entries.length, 4); + assertObjectMatch(entries[0], { path: "a", isSymlink: false, isDirectory: true, isFile: false }); + assertObjectMatch(entries[1], { path: "a/l2z", isSymlink: true, isDirectory: false, isFile: false }); + assertObjectMatch(entries[2], { path: "a/l2b", isSymlink: true, isDirectory: false, isFile: false }); + assertObjectMatch(entries[3], { path: "a/x", isSymlink: false, isDirectory: false, isFile: true }); + }, +); + + testWalk( async (d: string) => { await Deno.mkdir(d + "/a"); @@ -255,13 +305,13 @@ testWalk( await touch(d + "/b/z"); await Deno.symlink(d + "/b", d + "/a/bb"); }, - async function symlink() { - assertReady(6); - const files = await walkArray("a"); - assertEquals(files.length, 3); + async function followSymlinks() { + assertReady(7); + const files = await collectPaths("a"); + assertEquals(files.length, 4); assert(!files.includes("a/bb/z")); - const arr = await walkArray("a", { followSymlinks: true }); + const arr = await collectPaths("a", { followSymlinks: true }); assertEquals(arr.length, 5); assert(arr.some((f): boolean => f.endsWith("/b/z"))); }, @@ -292,8 +342,8 @@ testWalk( await Deno.symlink(d + "/b", d + "/a/bb"); }, async function symlinkPointsToFile() { - assertReady(5); - const files = await walkArray("a", { followSymlinks: true }); + assertReady(6); + const files = await collectPaths("a", { followSymlinks: true }); assertEquals(files.length, 4); assert(files.some((f): boolean => f.endsWith("/b"))); }, @@ -306,7 +356,7 @@ testWalk( }, async function unixSocket(listener: Deno.Listener) { assertReady(2); - const files = await walkArray(".", { followSymlinks: true }); + const files = await collectPaths(".", { followSymlinks: true }); assertEquals(files, [".", "a"]); listener.close(); }, @@ -322,7 +372,7 @@ testWalk( try { await assertRejects( async () => { - await walkArray("a"); + await collectPaths("a"); }, Deno.errors.PermissionDenied, 'for path "a/b"', @@ -364,7 +414,7 @@ testWalk( }, async function fifo() { assertReady(2); - const files = await walkArray(".", { followSymlinks: true }); + const files = await collectPaths(".", { followSymlinks: true }); assertEquals(files, [".", "fifo"]); }, Deno.build.os === "windows",