From 989e9f2568099826130eaacc50b6ab114b993bf0 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Thu, 7 Nov 2019 20:09:06 +0100 Subject: [PATCH] fix: don't use module exclude field for optimization + add test (TBS) Turns out using module exclude fields would need more elaborate logic, in the case of overlapping module sources. So I removed that optimization for now. --- docs/guides/configuration-files.md | 4 ++-- garden-service/src/config/module.ts | 7 ++++++- garden-service/src/config/project.ts | 17 ++++++++++++++--- garden-service/src/watch.ts | 8 +++++--- 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/docs/guides/configuration-files.md b/docs/guides/configuration-files.md index baae4c7397..3c05594bb4 100644 --- a/docs/guides/configuration-files.md +++ b/docs/guides/configuration-files.md @@ -353,7 +353,7 @@ Here we only scan the `modules` directory, but exclude the `modules/tmp` directo If you specify a list with `include`, only those patterns are included. If you then specify one or more `exclude` patterns, those are filtered out of the ones matched by `include`. If you _only_ specify `exclude`, those patterns will be filtered out of all paths in the project directory. -The `modules.exclude` field is also used to limit the number of files and directories Garden watches for changes while running. Use that if you have a large number of files/directories in your project that you do not need to watch, or if you are seeing excessive CPU/RAM usage. +The `modules.exclude` field is also used to limit the number of files and directories Garden watches for changes while running. Use that if you have a large number of files/directories in your project that you do not need to watch, or if you are seeing excessive CPU/RAM usage. The `modules.include` field has no effect on which paths Garden watches for changes. #### Module include/exclude @@ -377,7 +377,7 @@ Here we only include the `Dockerfile` and all the `.py` files under `my-sources/ If you specify a list with `include`, only those files/patterns are included. If you then specify one or more `exclude` files or patterns, those are filtered out of the files matched by `include`. If you _only_ specify `exclude`, those patterns will be filtered out of all files in the module directory. -The `exclude` field on modules is also used to limit the number of files and directories Garden watches for changes while running. Use that if you have a large number of files/directories in your module that you do not need to watch, or if you are seeing excessive CPU/RAM usage. +Note that the module `include` and `exclude` fields have no effect on which paths Garden watches for changes. Use the project `modules.exclude` field (described above) for that purpose. #### .ignore files diff --git a/garden-service/src/config/module.ts b/garden-service/src/config/module.ts index d6dd49cf1b..38f5bcac23 100644 --- a/garden-service/src/config/module.ts +++ b/garden-service/src/config/module.ts @@ -140,7 +140,12 @@ export const baseModuleSpecSchema = joi Note that you can also explicitly _include_ files using the \`include\` field. If you also specify the \`include\` field, the files/patterns specified here are filtered from the files matched by \`include\`. See the - [Configuration Files guide](${includeGuideLink})for details.` + [Configuration Files guide](${includeGuideLink})for details. + + Unlike the \`modules.exclude\` field in the project config, the filters here have _no effect_ on which files + and directories are watched for changes. Use the project \`modules.exclude\` field to affect those, if you have + large directories that should not be watched for changes. + ` ) .example([["tmp/**/*", "*.log"], {}]), repositoryUrl: joiRepositoryUrl().description( diff --git a/garden-service/src/config/project.ts b/garden-service/src/config/project.ts index 995da91bee..e27ac94479 100644 --- a/garden-service/src/config/project.ts +++ b/garden-service/src/config/project.ts @@ -166,12 +166,17 @@ const projectModulesSchema = joi.object().keys({ .array() .items(joi.string().posixPath({ subPathOnly: true })) .description( - dedent`Specify a list of POSIX-style paths or globs that should be scanned for Garden modules. + dedent` + Specify a list of POSIX-style paths or globs that should be scanned for Garden modules. Note that you can also _exclude_ path using the \`exclude\` field or by placing \`.gardenignore\` files in your source tree, which use the same format as \`.gitignore\` files. See the [Configuration Files guide](${includeGuideLink}) for details. + Unlike the \`exclude\` field, the paths/globs specified here have _no effect_ on which files and directories + Garden watches for changes. Use the \`exclude\` field to affect those, if you have large directories that + should not be watched for changes. + Also note that specifying an empty list here means _no paths_ should be included.` ) .example([["modules/**/*"], {}]), @@ -179,11 +184,17 @@ const projectModulesSchema = joi.object().keys({ .array() .items(joi.string().posixPath({ subPathOnly: true })) .description( - deline`Specify a list of POSIX-style paths or glob patterns that should be excluded when scanning for modules. + deline` + Specify a list of POSIX-style paths or glob patterns that should be excluded when scanning for modules. + + The filters here also affect which files and directories are watched for changes. So if you have a large number + of directories in your project that should not be watched, you should specify them here. The \`include\` field + does _not_ affect which files are watched. Note that you can also explicitly _include_ files using the \`include\` field. If you also specify the \`include\` field, the paths/patterns specified here are filtered from the files matched by \`include\`. See the - [Configuration Files guide](${includeGuideLink}) for details.` + [Configuration Files guide](${includeGuideLink}) for details. + ` ) .example([["public/**/*", "tmp/**/*"], {}]), }) diff --git a/garden-service/src/watch.ts b/garden-service/src/watch.ts index 9a0e0f4d37..629fda7fb9 100644 --- a/garden-service/src/watch.ts +++ b/garden-service/src/watch.ts @@ -13,7 +13,7 @@ import { Module } from "./types/module" import { Garden } from "./garden" import { LogEntry } from "./logger/log-entry" import { sleep } from "./util/util" -import { some, flatten } from "lodash" +import { some } from "lodash" import { isConfigFilename, matchPath } from "./util/fs" import Bluebird from "bluebird" import { InternalError } from "./exceptions" @@ -87,7 +87,9 @@ export class Watcher { // See https://github.com/garden-io/garden/issues/1269. // TODO: see if we can extract paths from dotignore files as well (we'd have to deal with negations etc. somehow). const projectExcludes = this.garden.moduleExcludePatterns.map((p) => resolve(this.garden.projectRoot, p)) - const moduleExcludes = flatten(this.modules.map((m) => (m.exclude || []).map((p) => resolve(m.path, p)))) + // TODO: filter paths based on module excludes as well + // (requires more complex logic to handle overlapping module sources). + // const moduleExcludes = flatten(this.modules.map((m) => (m.exclude || []).map((p) => resolve(m.path, p)))) this.watcher = watch(this.paths, { ignoreInitial: true, @@ -97,7 +99,7 @@ export class Watcher { stabilityThreshold: 500, pollInterval: 200, }, - ignored: [...projectExcludes, ...moduleExcludes], + ignored: [...projectExcludes], }) this.watcher