Custom garden dir path #823
Conversation
eysi09
force-pushed
the
custom-garden-dir-path
branch
from
1819ed4 to
8daec3e
Compare
garden-service/src/cli/cli.ts
Outdated
| if (this.fileWritersInitialized) { | ||
| return | ||
| } | ||
| for (const config of FILE_WRITER_CONFIGS) { | ||
| logger.writers.push( | ||
| await FileWriter.factory({ | ||
| level: logger.level, | ||
| root: projectRoot, | ||
| gardenDirPath, |
There was a problem hiding this comment.
This is a change in behaviour, right? Is it intentional?
There was a problem hiding this comment.
Yes you're right, there's a mistake there. However, we also store development and error logs under projectRoot/.garden/logs so the gardenDirPath is also needed there.
| @@ -202,3 +209,7 @@ export async function prepareSystemServices( | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| export function getSystemGardenDirPath(projectGardenDirPath: string) { | |||
There was a problem hiding this comment.
Feels unnecessary to break this out and export it, since it's used only once.
There was a problem hiding this comment.
Ah yes, this was originally used in more places. Forgot to clean up.
garden-service/src/cli/cli.ts
Outdated
| @@ -283,7 +283,7 @@ export class GardenCli { | |||
|
|
|||
| // Register log file writers. We need to do this after the Garden class is initialised because | |||
| // the file writers depend on the project root. | |||
| await this.initFileWriters(logger, garden.projectRoot) | |||
| await this.initFileWriters(logger, garden.gardenDirPath) | |||
There was a problem hiding this comment.
If this is the intended change, the comment needs updating.
Also, we assume in at least a couple of places that the error.log file, for example, gets written in the project root (look for references to ERROR_LOG_FILENAME). We either need to track all those references down and update, or revert this particular change.
There was a problem hiding this comment.
Yeah you're right, this needs fixing, see my comment above.
garden-service/src/plugin-context.ts
Outdated
| @@ -36,6 +38,12 @@ export const pluginContextSchema = Joi.object() | |||
| projectRoot: Joi.string() | |||
| .uri(<any>{ relativeOnly: true }) | |||
| .description("The absolute path of the project root."), | |||
| gardenDirPath: Joi.string() | |||
| .uri(<any>{ relativeOnly: true }) | |||
There was a problem hiding this comment.
The relativeOnly constraint isn't correct.
There was a problem hiding this comment.
Good catch. What about for the projectRoot key above?
There was a problem hiding this comment.
Actually, my bad. I keep mixing these things up, this was fine as it was. But I don't think it actually matters here, this schema is more for documentation since we control the inputs directly.
| }) | ||
|
|
||
| describe("getExtSourcesDirName", () => { | ||
| it("should return the relative path to the remote projects directory", () => { | ||
| const dirName = getRemoteSourcesDirname("project") | ||
| expect(dirName).to.equal(".garden/sources/project") | ||
| expect(dirName).to.equal("sources/project") |
There was a problem hiding this comment.
Unsure if/how it really matters, but we should avoid hardcoding paths like this, and rather use join(...parts) to make sure it works across platforms consistently.
Previously we hard coded the Garden dir path to `projectRoot/.garden`. With this change, it is possible to set a custom Garden dir path when initialising the Garden class. This is useful for system modules whose builds can now be stored at the project level.
eysi09
force-pushed
the
custom-garden-dir-path
branch
from
8daec3e to
a57a611
Compare
With this change, system module builds are stored at the project level. This way we can run several Garden processes at the same time without them all modifying the same system build directory, which can cause unexpected issues.
eysi09
force-pushed
the
custom-garden-dir-path
branch
from
a57a611 to
33019b0
Compare
This PR contains two commits. The first is a refactor that allows setting a custom path for the project level Garden directory (i.e. the
.gardendirectory). The second commit uses that ability when creating the system Garden class.From the commit message:
Previously we hard coded the Garden dir path to
projectRoot/.garden.With this change, it is possible to set a custom Garden dir path when
initialising the Garden class. This is useful for system modules whose
builds can now be stored at the project level
The original motivation behind this is change is that integration tests were failing when they ran in parallel because different instances of Garden were modifying the same system level
.gardendirectory at the same time.@thsig Could you please glance over the
build-dir.tschanges. In particular this line: https://github.com/garden-io/garden/pull/823/files#diff-99aee768efe4263754433634f8cb56ebR130