Skip to content

Commit

Permalink
fix(core): auto-exclude git and .garden dirs for modules in project root
Browse files Browse the repository at this point in the history
Because we require an `include` or `exclude` to be set on modules if
there are multiple modules in the same directory, this would only have
come up  as an issue if there's a single module in the project root
_or_ if users had used `exclude` fields and not `include` fields to
specify the module context.

Which is likely why this passed by for so long, but certainly this
change is the more sane default behavior.
  • Loading branch information
edvald authored and thsig committed Nov 25, 2020
1 parent 3fdd0e5 commit bf854a5
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 21 deletions.
2 changes: 1 addition & 1 deletion cli/src/add-version-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ async function addVersionFiles() {
const path = config.path
const versionFilePath = resolve(path, GARDEN_VERSIONFILE_NAME)

const vcsHandler = new GitHandler(garden.gardenDirPath, garden.dotIgnoreFiles)
const vcsHandler = new GitHandler(STATIC_DIR, garden.gardenDirPath, garden.dotIgnoreFiles)
const treeVersion = await vcsHandler.getTreeVersion(garden.log, garden.projectName, config)

// tslint:disable-next-line: no-console
Expand Down
2 changes: 1 addition & 1 deletion core/src/commands/get/get-debug-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export async function collectBasicDebugInfo(root: string, gardenDirPath: string,
}

// Find all services paths
const vcs = new GitHandler(root, projectConfig.dotIgnoreFiles || defaultDotIgnoreFiles)
const vcs = new GitHandler(root, gardenDirPath, projectConfig.dotIgnoreFiles || defaultDotIgnoreFiles)
const include = projectConfig.modules && projectConfig.modules.include
const exclude = projectConfig.modules && projectConfig.modules.exclude
const paths = await findConfigPathsInPath({ vcs, dir: root, include, exclude, log })
Expand Down
2 changes: 1 addition & 1 deletion core/src/commands/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class MigrateCommand extends Command<Args, Opts> {
if (args.configPaths && args.configPaths.length > 0) {
configPaths = args.configPaths.map((path) => resolve(root, path))
} else {
const vcs = new GitHandler(resolve(root, DEFAULT_GARDEN_DIR_NAME), [])
const vcs = new GitHandler(root, resolve(root, DEFAULT_GARDEN_DIR_NAME), [])
configPaths = await findConfigPathsInPath({
dir: root,
vcs,
Expand Down
10 changes: 7 additions & 3 deletions core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import { Watcher } from "./watch"
import {
findConfigPathsInPath,
getWorkingCopyId,
fixedExcludes,
fixedProjectExcludes,
detectModuleOverlap,
ModuleOverlap,
defaultConfigFilename,
Expand Down Expand Up @@ -358,10 +358,14 @@ export class Garden {

// We always exclude the garden dir
const gardenDirExcludePattern = `${relative(projectRoot, gardenDirPath)}/**/*`
const moduleExcludePatterns = [...((config.modules || {}).exclude || []), gardenDirExcludePattern, ...fixedExcludes]
const moduleExcludePatterns = [
...((config.modules || {}).exclude || []),
gardenDirExcludePattern,
...fixedProjectExcludes,
]

// Ensure the project root is in a git repo
const vcs = new GitHandler(gardenDirPath, config.dotIgnoreFiles)
const vcs = new GitHandler(projectRoot, gardenDirPath, config.dotIgnoreFiles)
await vcs.getRepoRoot(log, projectRoot)

const garden = new this({
Expand Down
2 changes: 1 addition & 1 deletion core/src/util/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { uuidv4, exec } from "./util"
export const defaultConfigFilename = "garden.yml"
const metadataFilename = "metadata.json"
export const defaultDotIgnoreFiles = [".gardenignore"]
export const fixedExcludes = [".git", ".gitmodules", ".garden/**/*", "debug-info*/**"]
export const fixedProjectExcludes = [".git", ".gitmodules", ".garden/**/*", "debug-info*/**"]

/*
Warning: Don't make any async calls in the loop body when using this function, since this may cause
Expand Down
11 changes: 9 additions & 2 deletions core/src/vcs/vcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { LogEntry } from "../logger/log-entry"
import { treeVersionSchema, moduleVersionSchema } from "../config/common"
import { Warning } from "../db/entities/warning"
import { dedent } from "../util/string"
import { fixedProjectExcludes } from "../util/fs"

export const NEW_MODULE_VERSION = "0000000000"
const fileCountWarningThreshold = 10000
Expand Down Expand Up @@ -66,7 +67,7 @@ export interface VcsFile {
}

export abstract class VcsHandler {
constructor(protected gardenDirPath: string, protected ignoreFiles: string[]) {}
constructor(protected projectRoot: string, protected gardenDirPath: string, protected ignoreFiles: string[]) {}

abstract name: string
abstract async getRepoRoot(log: LogEntry, path: string): Promise<string>
Expand All @@ -78,12 +79,18 @@ export abstract class VcsHandler {
async getTreeVersion(log: LogEntry, projectName: string, moduleConfig: ModuleConfig): Promise<TreeVersion> {
const configPath = moduleConfig.configPath

// Apply project root excludes if the module config is in the project root and `include` isn't set
const exclude =
moduleConfig.path === this.projectRoot && !moduleConfig.include
? [...(moduleConfig.exclude || []), ...fixedProjectExcludes]
: moduleConfig.exclude

let files = await this.getFiles({
log,
path: moduleConfig.path,
pathDescription: "module root",
include: moduleConfig.include,
exclude: moduleConfig.exclude,
exclude,
})

if (files.length > fileCountWarningThreshold) {
Expand Down
12 changes: 6 additions & 6 deletions core/test/unit/src/vcs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe("GitHandler", () => {
log = garden.log
tmpDir = await makeTempGitRepo()
tmpPath = await realpath(tmpDir.path)
handler = new GitHandler(tmpPath, [defaultIgnoreFilename])
handler = new GitHandler(tmpPath, join(tmpPath, ".garden"), [defaultIgnoreFilename])
git = (<any>handler).gitCli(log, tmpPath)
})

Expand Down Expand Up @@ -305,7 +305,7 @@ describe("GitHandler", () => {

const hash = "6e1ab2d7d26c1c66f27fea8c136e13c914e3f137"

const _handler = new GitHandler(tmpPath, [])
const _handler = new GitHandler(tmpPath, join(tmpPath, ".garden"), [])

expect(await _handler.getFiles({ path: tmpPath, log })).to.eql([{ path, hash }])
})
Expand All @@ -325,7 +325,7 @@ describe("GitHandler", () => {
await git("add", pathA)
await git("commit", "-m", "foo")

const _handler = new GitHandler(tmpPath, [defaultIgnoreFilename, ".testignore2"])
const _handler = new GitHandler(tmpPath, join(tmpPath, ".garden"), [defaultIgnoreFilename, ".testignore2"])

const files = (await _handler.getFiles({ path: tmpPath, exclude: [], log })).filter(
(f) => !f.path.includes(defaultIgnoreFilename)
Expand Down Expand Up @@ -545,7 +545,7 @@ describe("GitHandler", () => {
await commit("test commit B", tmpRepoPathB)

const hash = hashRepoUrl(repositoryUrlA)
clonePath = join(tmpPath, "sources", "module", `foo--${hash}`)
clonePath = join(tmpPath, ".garden", "sources", "module", `foo--${hash}`)
})

afterEach(async () => {
Expand Down Expand Up @@ -583,7 +583,7 @@ describe("GitHandler", () => {
})

const hash = hashRepoUrl(repositoryUrlA)
expect(res).to.eql(join(tmpPath, "sources", "project", `foo--${hash}`))
expect(res).to.eql(join(tmpPath, ".garden", "sources", "project", `foo--${hash}`))
})
it("should not error if source already cloned", async () => {
await handler.ensureRemoteSource({
Expand Down Expand Up @@ -642,7 +642,7 @@ describe("GitHandler", () => {
})

const hash = hashRepoUrl(repositoryUrlA)
clonePath = join(tmpPath, "sources", "project", `foo--${hash}`)
clonePath = join(tmpPath, ".garden", "sources", "project", `foo--${hash}`)

expect(await getCommitMsg(clonePath)).to.eql("test commit A")
})
Expand Down
35 changes: 29 additions & 6 deletions core/test/unit/src/vcs/vcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
getVersionString,
writeTreeVersionFile,
readTreeVersionFile,
GetFilesParams,
VcsFile,
} from "../../../../src/vcs/vcs"
import { projectRootA, makeTestGardenA, makeTestGarden, getDataDir, TestGarden } from "../../../helpers"
import { expect } from "chai"
Expand All @@ -24,7 +26,7 @@ import td from "testdouble"
import tmp from "tmp-promise"
import { realpath, readFile, writeFile } from "fs-extra"
import { GARDEN_VERSIONFILE_NAME } from "../../../../src/constants"
import { defaultDotIgnoreFiles } from "../../../../src/util/fs"
import { defaultDotIgnoreFiles, fixedProjectExcludes } from "../../../../src/util/fs"
import { LogEntry } from "../../../../src/logger/log-entry"
import { findByName } from "../../../../src/util/util"

Expand All @@ -36,7 +38,7 @@ class TestVcsHandler extends VcsHandler {
return "/foo"
}

async getFiles() {
async getFiles(_: GetFilesParams): Promise<VcsFile[]> {
return []
}

Expand Down Expand Up @@ -72,7 +74,7 @@ describe("VcsHandler", () => {
}

beforeEach(async () => {
handlerA = new TestVcsHandler(projectRootA, defaultDotIgnoreFiles)
handlerA = new TestVcsHandler(projectRootA, join(projectRootA, ".garden"), defaultDotIgnoreFiles)
gardenA = await makeTestGardenA()
})

Expand Down Expand Up @@ -121,7 +123,7 @@ describe("VcsHandler", () => {
const projectRoot = getDataDir("test-projects", "include-exclude")
const garden = await makeTestGarden(projectRoot)
const moduleConfig = await garden.resolveModule("module-a")
const handler = new GitHandler(garden.gardenDirPath, garden.dotIgnoreFiles)
const handler = new GitHandler(garden.projectRoot, garden.gardenDirPath, garden.dotIgnoreFiles)

const version = await handler.getTreeVersion(gardenA.log, gardenA.projectName, moduleConfig)

Expand All @@ -135,7 +137,7 @@ describe("VcsHandler", () => {
const projectRoot = getDataDir("test-projects", "include-exclude")
const garden = await makeTestGarden(projectRoot)
const moduleConfig = await garden.resolveModule("module-b")
const handler = new GitHandler(garden.gardenDirPath, garden.dotIgnoreFiles)
const handler = new GitHandler(garden.projectRoot, garden.gardenDirPath, garden.dotIgnoreFiles)

const version = await handler.getTreeVersion(garden.log, garden.projectName, moduleConfig)

Expand All @@ -146,7 +148,7 @@ describe("VcsHandler", () => {
const projectRoot = getDataDir("test-projects", "include-exclude")
const garden = await makeTestGarden(projectRoot)
const moduleConfig = await garden.resolveModule("module-c")
const handler = new GitHandler(garden.gardenDirPath, garden.dotIgnoreFiles)
const handler = new GitHandler(garden.projectRoot, garden.gardenDirPath, garden.dotIgnoreFiles)

const version = await handler.getTreeVersion(garden.log, garden.projectName, moduleConfig)

Expand All @@ -169,6 +171,27 @@ describe("VcsHandler", () => {
await writeFile(configPath, orgConfig)
}
})

it("should apply project-level excludes if module's path is same as root and no include is set", async () => {
td.replace(handlerA, "getFiles", async ({ exclude }: GetFilesParams) => {
expect(exclude).to.eql(fixedProjectExcludes)
return []
})
const moduleConfig = await gardenA.resolveModule("module-a")
moduleConfig.path = gardenA.projectRoot
await handlerA.getTreeVersion(gardenA.log, gardenA.projectName, moduleConfig)
})

it("should not apply project-level excludes if module's path is same as root but include is set", async () => {
td.replace(handlerA, "getFiles", async ({ exclude }: GetFilesParams) => {
expect(exclude).to.be.undefined
return []
})
const moduleConfig = await gardenA.resolveModule("module-a")
moduleConfig.path = gardenA.projectRoot
moduleConfig.include = []
await handlerA.getTreeVersion(gardenA.log, gardenA.projectName, moduleConfig)
})
})

describe("resolveTreeVersion", () => {
Expand Down

0 comments on commit bf854a5

Please sign in to comment.