Fix: ability to scan directory symbolic links (#949)

src/modules/candidateWriter.ts

@@ -470,43 +470,44 @@ export default class CandidateWriter extends Module {
       return;
     }
 
-    const targetPath = outputRomFile.getFilePath();
+    const linkPath = outputRomFile.getFilePath();
     let sourcePath = path.resolve(inputRomFile.getFilePath());
     if (this.options.getSymlinkRelative()) {
-      sourcePath = path.relative(path.dirname(targetPath), sourcePath);
+      await CandidateWriter.ensureOutputDirExists(linkPath);
+      sourcePath = await fsPoly.symlinkRelativePath(sourcePath, linkPath);
     }
 
     // If the output file already exists, see if we need to do anything
-    if (await fsPoly.exists(targetPath)) {
+    if (await fsPoly.exists(linkPath)) {
       if (!this.options.getOverwrite() && !this.options.getOverwriteInvalid()) {
-        this.progressBar.logDebug(`${dat.getNameShort()}: ${releaseCandidate.getName()}: ${targetPath}: not overwriting existing file`);
+        this.progressBar.logDebug(`${dat.getNameShort()}: ${releaseCandidate.getName()}: ${linkPath}: not overwriting existing file`);
         return;
       }
 
       if (this.options.getOverwriteInvalid()) {
-        const existingTest = await CandidateWriter.testWrittenSymlink(targetPath, sourcePath);
+        const existingTest = await CandidateWriter.testWrittenSymlink(linkPath, sourcePath);
         if (!existingTest) {
-          this.progressBar.logDebug(`${dat.getNameShort()}: ${releaseCandidate.getName()}: ${targetPath}: not overwriting existing symlink, existing symlink is what was expected`);
+          this.progressBar.logDebug(`${dat.getNameShort()}: ${releaseCandidate.getName()}: ${linkPath}: not overwriting existing symlink, existing symlink is what was expected`);
           return;
         }
       }
 
-      await fsPoly.rm(targetPath, { force: true });
+      await fsPoly.rm(linkPath, { force: true });
     }
 
-    this.progressBar.logInfo(`${dat.getNameShort()}: ${releaseCandidate.getName()}: creating symlink '${sourcePath}' -> '${targetPath}'`);
+    this.progressBar.logInfo(`${dat.getNameShort()}: ${releaseCandidate.getName()}: creating symlink '${sourcePath}' -> '${linkPath}'`);
     try {
-      await CandidateWriter.ensureOutputDirExists(targetPath);
-      await fsPoly.symlink(sourcePath, targetPath);
+      await CandidateWriter.ensureOutputDirExists(linkPath);
+      await fsPoly.symlink(sourcePath, linkPath);
     } catch (error) {
-      this.progressBar.logError(`${dat.getNameShort()}: ${releaseCandidate.getName()}: ${targetPath}: failed to symlink from ${sourcePath}: ${error}`);
+      this.progressBar.logError(`${dat.getNameShort()}: ${releaseCandidate.getName()}: ${linkPath}: failed to symlink from ${sourcePath}: ${error}`);
       return;
     }
 
     if (this.options.shouldTest()) {
-      const writtenTest = await CandidateWriter.testWrittenSymlink(targetPath, sourcePath);
+      const writtenTest = await CandidateWriter.testWrittenSymlink(linkPath, sourcePath);
       if (writtenTest) {
-        this.progressBar.logError(`${dat.getNameShort()}: ${releaseCandidate.getName()} ${targetPath}: written symlink ${writtenTest}`);
+        this.progressBar.logError(`${dat.getNameShort()}: ${releaseCandidate.getName()} ${linkPath}: written symlink ${writtenTest}`);
       }
     }
   }
@@ -520,6 +521,10 @@ export default class CandidateWriter extends Module {
       return `has the source path '${existingSourcePath}', expected '${expectedSourcePath}`;
     }
 
+    if (!await fsPoly.exists(await fsPoly.readlinkResolved(targetPath))) {
+      return `has the source path '${existingSourcePath}' which doesn't exist`;
+    }
+
     return undefined;
   }
 }

src/modules/scanner.ts

@@ -64,7 +64,16 @@ export default abstract class Scanner extends Module {
     try {
       const totalKilobytes = await fsPoly.size(filePath) / 1024;
       const files = await Scanner.FILESIZE_SEMAPHORE.runExclusive(
-        async () => FileFactory.filesFrom(filePath, checksumBitmask),
+        async () => {
+          if (await fsPoly.isSymlink(filePath)) {
+            const realFilePath = await fsPoly.readlinkResolved(filePath);
+            if (!await fsPoly.exists(realFilePath)) {
+              this.progressBar.logWarn(`${filePath}: broken symlink, '${realFilePath}' doesn't exist`);
+              return [];
+            }
+          }
+          return FileFactory.filesFrom(filePath, checksumBitmask);
+        },
         totalKilobytes,
       );
 

src/polyfill/filePoly.ts

@@ -41,7 +41,7 @@ export default class FilePoly {
     );
   }
 
-  static async fileOfSize(pathLike: PathLike, flags: OpenMode, size: number): Promise<FilePoly> {
+  static async fileOfSize(pathLike: string, flags: OpenMode, size: number): Promise<FilePoly> {
     if (await fsPoly.exists(pathLike)) {
       await fsPoly.rm(pathLike, { force: true });
     }

src/polyfill/fsPoly.ts

@@ -8,6 +8,8 @@ import { isNotJunk } from 'junk';
 import nodeDiskInfo from 'node-disk-info';
 import semver from 'semver';
 
+import ArrayPoly from './arrayPoly.js';
+
 export type FsWalkCallback = (increment: number) => void;
 
 export default class FsPoly {
@@ -56,6 +58,16 @@ export default class FsPoly {
     }
   }
 
+  static async dirs(dirPath: string): Promise<string[]> {
+    const readDir = (await util.promisify(fs.readdir)(dirPath))
+      .filter((filePath) => isNotJunk(path.basename(filePath)))
+      .map((filePath) => path.join(dirPath, filePath));
+
+    return (await Promise.all(
+      readDir.map(async (filePath) => (await this.isDirectory(filePath) ? filePath : undefined)),
+    )).filter(ArrayPoly.filterNotNullish);
+  }
+
   static disksSync(): string[] {
     return FsPoly.DRIVES
       .filter((drive) => drive.available > 0)
@@ -71,9 +83,14 @@ export default class FsPoly {
     return util.promisify(fs.exists)(pathLike);
   }
 
-  static async isDirectory(pathLike: PathLike): Promise<boolean> {
+  static async isDirectory(pathLike: string): Promise<boolean> {
     try {
-      return (await util.promisify(fs.lstat)(pathLike)).isDirectory();
+      const lstat = (await util.promisify(fs.lstat)(pathLike));
+      if (lstat.isSymbolicLink()) {
+        const link = await this.readlinkResolved(pathLike);
+        return await this.isDirectory(link);
+      }
+      return lstat.isDirectory();
     } catch {
       return false;
     }
@@ -245,11 +262,26 @@ export default class FsPoly {
     return util.promisify(fs.readlink)(pathLike);
   }
 
+  static async readlinkResolved(link: string): Promise<string> {
+    const source = await this.readlink(link);
+    if (path.isAbsolute(source)) {
+      return source;
+    }
+    return path.join(path.dirname(link), source);
+  }
+
+  static async realpath(pathLike: PathLike): Promise<string> {
+    if (!await this.exists(pathLike)) {
+      throw new Error(`can't get realpath of non-existent path: ${pathLike}`);
+    }
+    return util.promisify(fs.realpath)(pathLike);
+  }
+
   /**
    * fs.rm() was added in: v14.14.0
    * util.promisify(fs.rm)() was added in: v14.14.0
    */
-  static async rm(pathLike: PathLike, options: RmOptions = {}): Promise<void> {
+  static async rm(pathLike: string, options: RmOptions = {}): Promise<void> {
     const optionsWithRetry = {
       maxRetries: 2,
       ...options,
@@ -305,8 +337,21 @@ export default class FsPoly {
     return `${Number.parseFloat((bytes / k ** i).toFixed(decimals))}${sizes[i]}`;
   }
 
-  static async symlink(file: PathLike, link: PathLike): Promise<void> {
-    return util.promisify(fs.symlink)(file, link);
+  static async symlink(target: PathLike, link: PathLike): Promise<void> {
+    return util.promisify(fs.symlink)(target, link);
+  }
+
+  static async symlinkRelativePath(target: string, link: string): Promise<string> {
+    // NOTE(cemmer): macOS can be funny with files or links in system folders such as
+    // `/var/folders/*/...` whose real path is actually `/private/var/folders/*/...`, and
+    // path.resolve() won't resolve these fully, so we need the OS to resolve them in order to
+    // generate valid relative paths
+    const realTarget = await this.realpath(target);
+    const realLink = path.join(
+      await this.realpath(path.dirname(link)),
+      path.basename(link),
+    );
+    return path.relative(path.dirname(realLink), realTarget);
   }
 
   static async touch(filePath: string): Promise<void> {
@@ -340,6 +385,7 @@ export default class FsPoly {
       callback(files.length);
     }
 
+    // TODO(cemmer): `Promise.all()` this?
     for (const file of files) {
       const fullPath = path.join(pathLike.toString(), file);
       if (await this.isDirectory(fullPath)) {

src/types/files/file.ts

@@ -183,13 +183,6 @@ export default class File implements FileProps {
     return this.symlinkSource;
   }
 
-  getSymlinkSourceResolved(): string | undefined {
-    if (!this.symlinkSource) {
-      return undefined;
-    }
-    return path.resolve(path.dirname(this.getFilePath()), this.symlinkSource);
-  }
-
   getFileHeader(): ROMHeader | undefined {
     return this.fileHeader;
   }

test/modules/romScanner.test.ts

@@ -57,17 +57,64 @@ describe('multiple files', () => {
     const scannedRealFiles = (await createRomScanner(['test/fixtures/roms']).scan())
       .sort((a, b) => a.getFilePath().localeCompare(b.getFilePath()));
 
+    // Given some symlinked files
     const tempDir = await fsPoly.mkdtemp(Constants.GLOBAL_TEMP_DIR);
     try {
       const romFiles = await fsPoly.walk('test/fixtures/roms');
-      await Promise.all(romFiles.map(async (romFile) => {
+      await Promise.all(romFiles.map(async (romFile, idx) => {
         const tempLink = path.join(tempDir, romFile);
         await fsPoly.mkdir(path.dirname(tempLink), { recursive: true });
-        await fsPoly.symlink(path.resolve(romFile), tempLink);
+        if (idx % 2 === 0) {
+          // symlink some files with absolute paths
+          await fsPoly.symlink(path.resolve(romFile), tempLink);
+        } else {
+          // symlink some files with relative paths
+          await fsPoly.symlink(await fsPoly.symlinkRelativePath(romFile, tempLink), tempLink);
+        }
       }));
+
+      // When scanning symlinked files
+      const scannedSymlinks = (await createRomScanner([tempDir]).scan())
+        .sort((a, b) => a.getFilePath().localeCompare(b.getFilePath()));
+
+      // Then the files scan successfully
+      expect(scannedSymlinks).toHaveLength(scannedRealFiles.length);
+      for (const [idx, scannedSymlink] of scannedSymlinks.entries()) {
+        expect(scannedSymlink.getSize()).toEqual(scannedRealFiles[idx].getSize());
+        expect(scannedSymlink.getCrc32()).toEqual(scannedRealFiles[idx].getCrc32());
+      }
+    } finally {
+      await fsPoly.rm(tempDir, { recursive: true });
+    }
+  });
+
+  it('should scan symlinked directories', async () => {
+    const realRomDir = path.join('test', 'fixtures', 'roms');
+    const romDirs = await fsPoly.dirs(realRomDir);
+
+    const scannedRealFiles = (await createRomScanner(romDirs).scan())
+      .sort((a, b) => a.getFilePath().localeCompare(b.getFilePath()));
+
+    // Given some symlinked dirs
+    const tempDir = await fsPoly.mkdtemp(Constants.GLOBAL_TEMP_DIR);
+    try {
+      await Promise.all(romDirs.map(async (romDir, idx) => {
+        const tempLink = path.join(tempDir, romDir);
+        await fsPoly.mkdir(path.dirname(tempLink), { recursive: true });
+        if (idx % 2 === 0) {
+          // symlink some files with absolute paths
+          await fsPoly.symlink(path.resolve(romDir), tempLink);
+        } else {
+          // symlink some files with relative paths
+          await fsPoly.symlink(await fsPoly.symlinkRelativePath(romDir, tempLink), tempLink);
+        }
+      }));
+
+      // When scanning symlink dirs
       const scannedSymlinks = (await createRomScanner([tempDir]).scan())
         .sort((a, b) => a.getFilePath().localeCompare(b.getFilePath()));
 
+      // Then the dirs scan successfully
       expect(scannedSymlinks).toHaveLength(scannedRealFiles.length);
       for (const [idx, scannedSymlink] of scannedSymlinks.entries()) {
         expect(scannedSymlink.getSize()).toEqual(scannedRealFiles[idx].getSize());

test/polyfill/fsPoly.test.ts

@@ -106,6 +106,14 @@ describe('makeLegal', () => {
   });
 });
 
+describe('readlink', () => {
+  // TODO(cemmer)
+});
+
+describe('readlinkResolved', () => {
+  // TODO(cemmer)
+});
+
 describe('rm', () => {
   it('should throw on missing file', async () => {
     const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'temp'));

test/types/files/archives/archiveEntry.test.ts

@@ -15,6 +15,11 @@ import Options from '../../../../src/types/options.js';
 import IPSPatch from '../../../../src/types/patches/ipsPatch.js';
 import ProgressBarFake from '../../../console/progressBarFake.js';
 
+describe('archiveOf', () => {
+  // TODO(cemmer): what does it do with a file that doesn't exist - I think the TAR filestream
+  //  reading might hang forever, specifically
+});
+
 describe('getEntryPath', () => {
   test.each([
     'something.rom',
@@ -26,6 +31,63 @@ describe('getEntryPath', () => {
   });
 });
 
+describe('getSize', () => {
+  describe.each([
+    ['./test/fixtures/roms/7z/fizzbuzz.7z', 9],
+    ['./test/fixtures/roms/rar/fizzbuzz.rar', 9],
+    ['./test/fixtures/roms/tar/fizzbuzz.tar.gz', 9],
+    ['./test/fixtures/roms/zip/fizzbuzz.zip', 9],
+    ['./test/fixtures/roms/7z/foobar.7z', 7],
+    ['./test/fixtures/roms/rar/foobar.rar', 7],
+    ['./test/fixtures/roms/tar/foobar.tar.gz', 7],
+    ['./test/fixtures/roms/zip/foobar.zip', 7],
+    ['./test/fixtures/roms/7z/loremipsum.7z', 11],
+    ['./test/fixtures/roms/rar/loremipsum.rar', 11],
+    ['./test/fixtures/roms/tar/loremipsum.tar.gz', 11],
+    ['./test/fixtures/roms/zip/loremipsum.zip', 11],
+  ])('%s', (filePath, expectedSize) => {
+    it('should get the file\'s size', async () => {
+      const archiveEntries = await FileFactory.filesFrom(filePath);
+      expect(archiveEntries).toHaveLength(1);
+      const archiveEntry = archiveEntries[0];
+
+      expect(archiveEntry.getSize()).toEqual(expectedSize);
+    });
+
+    it('should get the absolute symlink\'s target size', async () => {
+      const tempDir = await fsPoly.mkdtemp(Constants.GLOBAL_TEMP_DIR);
+      try {
+        const tempLink = path.join(tempDir, path.basename(filePath));
+        await fsPoly.symlink(path.resolve(filePath), tempLink);
+
+        const archiveEntries = await FileFactory.filesFrom(tempLink);
+        expect(archiveEntries).toHaveLength(1);
+        const archiveEntry = archiveEntries[0];
+
+        expect(archiveEntry.getSize()).toEqual(expectedSize);
+      } finally {
+        await fsPoly.rm(tempDir, { recursive: true });
+      }
+    });
+
+    it('should get the relative symlink\'s target size', async () => {
+      const tempDir = await fsPoly.mkdtemp(Constants.GLOBAL_TEMP_DIR);
+      try {
+        const tempLink = path.join(tempDir, path.basename(filePath));
+        await fsPoly.symlink(await fsPoly.symlinkRelativePath(filePath, tempLink), tempLink);
+
+        const archiveEntries = await FileFactory.filesFrom(tempLink);
+        expect(archiveEntries).toHaveLength(1);
+        const archiveEntry = archiveEntries[0];
+
+        expect(archiveEntry.getSize()).toEqual(expectedSize);
+      } finally {
+        await fsPoly.rm(tempDir, { recursive: true });
+      }
+    });
+  });
+});
+
 describe('getCrc32', () => {
   test.each([
     ['./test/fixtures/roms/7z/fizzbuzz.7z', '370517b5'],