From d668622b5933f0c84ade350fc9b19f9360bce5af Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Mon, 26 Jul 2021 20:23:50 +0200 Subject: [PATCH] fix(core): warn instead of error when attempting to scan non-directory Fixes #2430 Also adds more specific and helpful error messages when we encounter missing/invalid submodule paths. --- core/src/util/fs.ts | 27 +++++++++++++- core/src/vcs/git.ts | 68 ++++++++++++++++++++++++++++++----- core/test/unit/src/vcs/git.ts | 36 +++++++++++++++++++ 3 files changed, 121 insertions(+), 10 deletions(-) diff --git a/core/src/util/fs.ts b/core/src/util/fs.ts index 355fca03ee..10817dc342 100644 --- a/core/src/util/fs.ts +++ b/core/src/util/fs.ts @@ -11,7 +11,7 @@ import klaw = require("klaw") import glob from "glob" import tmp from "tmp-promise" import _spawn from "cross-spawn" -import { pathExists, readFile, writeFile, lstat, realpath } from "fs-extra" +import { pathExists, readFile, writeFile, lstat, realpath, Stats } from "fs-extra" import { join, basename, win32, posix } from "path" import { platform } from "os" @@ -308,3 +308,28 @@ export async function makeTempDir({ git = false }: { git?: boolean } = {}): Prom return tmpDir } + +/** + * Returns the type of the given fs.Stats object as a string. + * + * @param stats an fs.Stats instance + */ +export function getStatsType(stats: Stats) { + if (stats.isBlockDevice()) { + return "block device" + } else if (stats.isCharacterDevice()) { + return "character device" + } else if (stats.isDirectory()) { + return "directory" + } else if (stats.isFIFO()) { + return "named pipe" + } else if (stats.isFile()) { + return "file" + } else if (stats.isSocket()) { + return "socket" + } else if (stats.isSymbolicLink()) { + return "symbolic link" + } else { + return "unknown" + } +} diff --git a/core/src/vcs/git.ts b/core/src/vcs/git.ts index 6eb8a0f492..7bfd11e03d 100644 --- a/core/src/vcs/git.ts +++ b/core/src/vcs/git.ts @@ -16,7 +16,7 @@ import split2 = require("split2") import { VcsHandler, RemoteSourceParams, VcsFile, GetFilesParams } from "./vcs" import { ConfigurationError, RuntimeError } from "../exceptions" import Bluebird from "bluebird" -import { joinWithPosix, matchPath } from "../util/fs" +import { getStatsType, joinWithPosix, matchPath } from "../util/fs" import { deline } from "../util/string" import { splitLast, exec } from "../util/util" import { LogEntry } from "../logger/log-entry" @@ -25,6 +25,9 @@ import { Profile } from "../util/profiling" import { SortedStreamIntersection } from "../util/streams" import execa = require("execa") import isGlob = require("is-glob") +import chalk = require("chalk") + +const submoduleErrorSuggestion = `Perhaps you need to run ${chalk.underline(`git submodule update --recursive`)}?` export function getCommitIdFromRefList(refList: string[]): string { try { @@ -123,6 +126,29 @@ export class GitHandler extends VcsHandler { log = log.debug(`Scanning ${pathDescription} at ${path}\nIncludes: ${include}\nExcludes:${exclude}`) + try { + const pathStats = await stat(path) + + if (!pathStats.isDirectory()) { + log.warn({ + symbol: "warning", + msg: chalk.gray(`Expected directory at ${path}, but found ${getStatsType(pathStats)}.`), + }) + return [] + } + } catch (err) { + // 128 = File no longer exists + if (err.exitCode === 128 || err.code === "ENOENT") { + log.warn({ + symbol: "warning", + msg: chalk.gray(`Attempted to scan directory at ${path}, but it does not exist.`), + }) + return [] + } else { + throw err + } + } + const git = this.gitCli(log, path) const gitRoot = await this.getRepoRoot(log, path) @@ -306,6 +332,35 @@ export class GitHandler extends VcsHandler { // Note: We apply include/exclude filters after listing files from submodule const submoduleRelPath = relative(path, submodulePath) + // Catch and show helpful message in case the submodule path isn't a valid directory + try { + const pathStats = await stat(path) + + if (!pathStats.isDirectory()) { + const pathType = getStatsType(pathStats) + log.warn({ + symbol: "warning", + msg: chalk.gray( + `Expected submodule directory at ${path}, but found ${pathType}. ${submoduleErrorSuggestion}` + ), + }) + return + } + } catch (err) { + // 128 = File no longer exists + if (err.exitCode === 128 || err.code === "ENOENT") { + log.warn({ + symbol: "warning", + msg: chalk.yellow( + `Found reference to submodule at ${submoduleRelPath}, but the path could not be found. ${submoduleErrorSuggestion}` + ), + }) + return + } else { + throw err + } + } + files.push( ...(await this.getFiles({ log, @@ -518,14 +573,9 @@ export class GitHandler extends VcsHandler { return (await git("rev-parse", "--abbrev-ref", "HEAD"))[0] } catch (err) { if (err.exitCode === 128) { - try { - // If this doesn't throw, then we're in a repo with no commits, or with a detached HEAD. - await git("rev-parse", "--show-toplevel") - return undefined - } catch (notInRepoError) { - // Throw nice error when we detect that we're not in a repo root - throw new RuntimeError(notInRepoRootErrorMessage(path), { path }) - } + // If this doesn't throw, then we're in a repo with no commits, or with a detached HEAD. + await this.getRepoRoot(log, path) + return undefined } else { throw err } diff --git a/core/test/unit/src/vcs/git.ts b/core/test/unit/src/vcs/git.ts index f7fade3e59..19106bbe38 100644 --- a/core/test/unit/src/vcs/git.ts +++ b/core/test/unit/src/vcs/git.ts @@ -435,6 +435,21 @@ describe("GitHandler", () => { expect(files).to.eql([filePath]) }) + it("gracefully aborts if given path doesn't exist", async () => { + const path = resolve(tmpPath, "foo") + + const files = (await handler.getFiles({ path, exclude: [], log })).map((f) => f.path) + expect(files).to.eql([]) + }) + + it("gracefully aborts if given path is not a directory", async () => { + const path = resolve(tmpPath, "foo") + await createFile(path) + + const files = (await handler.getFiles({ path, exclude: [], log })).map((f) => f.path) + expect(files).to.eql([]) + }) + context("path contains a submodule", () => { let submodule: tmp.DirectoryResult let submodulePath: string @@ -545,6 +560,27 @@ describe("GitHandler", () => { expect(paths).to.include(join("sub", initFile)) }) + it("gracefully skips submodule if its path doesn't exist", async () => { + const subPath = join(tmpPath, "sub") + await remove(subPath) + + const files = await handler.getFiles({ path: tmpPath, log }) + const paths = files.map((f) => relative(tmpPath, f.path)) + + expect(paths).to.eql([".gitmodules"]) + }) + + it("gracefully skips submodule if its path doesn't point to a directory", async () => { + const subPath = join(tmpPath, "sub") + await remove(subPath) + await createFile(subPath) + + const files = await handler.getFiles({ path: tmpPath, log }) + const paths = files.map((f) => relative(tmpPath, f.path)) + + expect(paths).to.eql([".gitmodules"]) + }) + context("submodule contains another submodule", () => { let submoduleB: tmp.DirectoryResult let submodulePathB: string