Skip to content

Commit

Permalink
fix(vcs): recursively handle submodules when scanning for files
Browse files Browse the repository at this point in the history
This is implemented by explicitly checking for configured submodules,
and recursively scanning each submodule that passes the given ignores
and include/exclude filters.

See the added section in the Configuration Files guide for some notes
on the intended behavior.

Fixes #1097
Closes #1190
  • Loading branch information
edvald committed Sep 18, 2019
1 parent 3592588 commit 06eabda
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 32 deletions.
8 changes: 8 additions & 0 deletions docs/using-garden/configuration-files.md
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,14 @@ name: my-project
dotIgnoreFiles: [.gardenignore]
```

#### Git submodules

If you're using Git submodules in your project, please note the following:

1. You may ignore submodules using .ignore files and include/exclude filters. If a submodule path _itself_ (that is, the path to the submodule directory, not its contents), matches one that is ignored by your .ignore files or exclude filters, or if you specify include filters and the submodule path does not match one of them, the module will not be scanned.
2. Include/exclude filters (both at the project and module level) are applied the same, whether a directory is a submodule or a normal directory.
3. _.ignore files are considered in the context of each git root_. This means that a .ignore file that's outside of a submodule will be completely ignored when scanning that submodule. This is by design, to be consistent with normal Git behavior.

## Next steps

We highly recommend reading the [Variables and Templating guide](./variables-and-templating.md) to understand how you can reference across different providers and modules, as well as to understand how to supply secret values to your configuration.
Expand Down
20 changes: 20 additions & 0 deletions garden-service/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions garden-service/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
"normalize-url": "^4.3.0",
"p-queue": "^6.1.1",
"p-retry": "^4.1.0",
"parse-git-config": "^3.0.0",
"path-is-inside": "^1.0.2",
"pluralize": "^8.0.0",
"proper-url-join": "^2.0.1",
Expand Down Expand Up @@ -152,6 +153,7 @@
"@types/node-emoji": "^1.8.1",
"@types/node-forge": "^0.8.6",
"@types/normalize-path": "^3.0.0",
"@types/parse-git-config": "^3.0.0",
"@types/path-is-inside": "^1.0.0",
"@types/pluralize": "0.0.29",
"@types/prettyjson": "0.0.29",
Expand Down
8 changes: 6 additions & 2 deletions garden-service/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import { platform, arch } from "os"
import { LogEntry } from "./logger/log-entry"
import { EventBus } from "./events"
import { Watcher } from "./watch"
import { findConfigPathsInPath, getConfigFilePath, getWorkingCopyId } from "./util/fs"
import { findConfigPathsInPath, getConfigFilePath, getWorkingCopyId, fixedExcludes } from "./util/fs"
import { Provider, ProviderConfig, getProviderDependencies, defaultProvider } from "./config/provider"
import { ResolveProviderTask } from "./tasks/resolve-provider"
import { ActionHelper } from "./actions"
Expand Down Expand Up @@ -226,7 +226,11 @@ export class Garden {

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

// Ensure the project root is in a git repo
const vcs = new GitHandler(gardenDirPath, config.dotIgnoreFiles)
Expand Down
4 changes: 2 additions & 2 deletions garden-service/src/util/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { LogEntry } from "../logger/log-entry"
const VALID_CONFIG_FILENAMES = ["garden.yml", "garden.yaml"]
const metadataFilename = "metadata.json"
export const defaultDotIgnoreFiles = [".gitignore", ".gardenignore"]
export const fixedExcludes = [".git", ".garden/**/*", "debug-info*/**"]
export const fixedExcludes = [".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 Expand Up @@ -116,7 +116,7 @@ export async function findConfigPathsInPath(
{ vcs: VcsHandler, dir: string, include?: string[], exclude?: string[], log: LogEntry },
) {
// TODO: we could make this lighter/faster using streaming
const files = await vcs.getFiles({ path: dir, include, exclude: [...exclude || [], ...fixedExcludes], log })
const files = await vcs.getFiles({ path: dir, include, exclude: exclude || [], log })
return files
.map(f => f.path)
.filter(f => isConfigFilename(basename(f)))
Expand Down
48 changes: 45 additions & 3 deletions garden-service/src/vcs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import execa from "execa"
import { join, resolve } from "path"
import { join, resolve, relative } from "path"
import { flatten } from "lodash"
import { ensureDir, pathExists, stat, createReadStream } from "fs-extra"
import { PassThrough } from "stream"
Expand All @@ -21,6 +21,7 @@ import { matchPath } from "../util/fs"
import { deline } from "../util/string"
import { splitLast } from "../util/util"
import { LogEntry } from "../logger/log-entry"
import parseGitConfig from "parse-git-config"

export function getCommitIdFromRefList(refList: string[]): string {
try {
Expand Down Expand Up @@ -48,6 +49,11 @@ interface GitCli {
(...args: string[]): Promise<string[]>
}

interface Submodule {
path: string
url: string
}

// TODO Consider moving git commands to separate (and testable) functions
export class GitHandler extends VcsHandler {
name = "git"
Expand Down Expand Up @@ -113,6 +119,9 @@ export class GitHandler extends VcsHandler {
)),
))

// List all submodule paths in the current repo
const submodulePaths = (await this.getSubmodules(gitRoot)).map(s => join(gitRoot, s.path))

// We run ls-files for each ignoreFile and do a manual set-intersection (by counting elements in an object)
// in order to optimize the flow.
const paths: { [path: string]: number } = {}
Expand Down Expand Up @@ -156,7 +165,10 @@ export class GitHandler extends VcsHandler {

// We push to the output array when all ls-files commands "agree" that it should be included,
// and it passes through the include/exclude filters.
if (paths[resolvedPath] === this.ignoreFiles.length && matchPath(filePath, include, exclude)) {
if (
paths[resolvedPath] === this.ignoreFiles.length
&& (matchPath(filePath, include, exclude) || submodulePaths.includes(resolvedPath))
) {
files.push({ path: resolvedPath, hash })
}
}
Expand All @@ -181,8 +193,20 @@ export class GitHandler extends VcsHandler {
}
})

// Resolve submodules
const withSubmodules = flatten(await Bluebird.map(files, async (f) => {
if (submodulePaths.includes(f.path)) {
// This path is a submodule, so we recursively call getFiles for that path again.
// Note: We apply include/exclude filters after listing files from submodule
return (await this.getFiles({ log, path: f.path, exclude: [] }))
.filter(submoduleFile => matchPath(relative(path, submoduleFile.path), include, exclude))
} else {
return [f]
}
}))

// Make sure we have a fresh hash for each file
return Bluebird.map(files, async (f) => {
return Bluebird.map(withSubmodules, async (f) => {
const resolvedPath = resolve(path, f.path)
if (!f.hash || modified.has(resolvedPath)) {
// If we can't compute the hash, i.e. the file is gone, we filter it out below
Expand Down Expand Up @@ -289,4 +313,22 @@ export class GitHandler extends VcsHandler {
createReadStream(path).pipe(stream)
return output
}

private async getSubmodules(gitRoot: string) {
const submodules: Submodule[] = []
const gitmodulesPath = join(gitRoot, ".gitmodules")

if (await pathExists(gitmodulesPath)) {
const parsed = await parseGitConfig({ cwd: gitRoot, path: ".gitmodules" })

for (const [key, spec] of Object.entries(parsed || {}) as any) {
if (!key.startsWith("submodule")) {
continue
}
spec.path && submodules.push(spec)
}
}

return submodules
}
}
Loading

0 comments on commit 06eabda

Please sign in to comment.