diff --git a/CONTEXT.md b/CONTEXT.md index b26ffc89..7bdd4982 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -48,7 +48,9 @@ This is a single-context repo. | **Init script** | A top-level script run once per lets invocation before the first Project command executes. | Before script | | **Before script** | A top-level script prepended to each Project command invocation, including dependencies. | Init script | | **After script** | A command-scoped script run after a Project command execution attempt. | Cleanup hook | -| **Work dir** | The directory where a Project command runs after config and command resolution. | Repo root | +| **Root dir** | The directory lets was invoked from. Project commands run here unless they set `work_dir`. | Project root, config dir | +| **Config dir** | The directory holding the Project config file. Only local mixin paths resolve against it. | Root dir, work dir | +| **Work dir** | The directory where a Project command runs after config and command resolution: the Root dir, or the command's `work_dir`. | Repo root | | **Download progress indicator** | A user-visible status shown while lets retrieves a Remote config or Remote mixin. | Progress bar | | **Help surface** | The rendered CLI help for root and Project commands. | Docs page | | **LSP surface** | The editor-facing language-server features exposed by `lets self lsp`. | CLI help | diff --git a/docs/adr/0004-root-dir-is-invocation-dir.md b/docs/adr/0004-root-dir-is-invocation-dir.md new file mode 100644 index 00000000..7be1c2f7 --- /dev/null +++ b/docs/adr/0004-root-dir-is-invocation-dir.md @@ -0,0 +1,105 @@ +# ADR-0004 — The Root dir is the invocation dir + +**Date:** 2026-08-10 +**Status:** Accepted + +## Context + +`lets` had never specified which directory a **Project command** runs in. The behavior +that shipped was an accident of implementation, and it changed silently in `0.0.63`. + +Up to `0.0.62`, `Command.WorkDir` was assigned unconditionally from +`filepath.Abs(cmd.WorkDir)`. For the overwhelming majority of commands — those without a +`work_dir` — that argument was `""`, and `filepath.Abs("")` returns the process cwd. The +executor then preferred `Command.WorkDir` over `Config.WorkDir` whenever it was non-empty, +which it now always was. So commands ran in the invocation dir, and `Config.WorkDir` — the +config file's directory, computed since 2020 — was dead code for its entire life. + +`0.0.63` added an `if cmd.WorkDir != ""` guard while fixing checksum handling for +`work_dir`. That was correct in isolation, but it un-shadowed `Config.WorkDir` and moved +every command's working directory to the config file's directory. + +Neither version was internally consistent. `Config.WorkDir` was read by some directives +and not others, so a single command definition resolved paths against two directories: + +| | `0.0.62` | `0.0.63` | +| --- | --- | --- | +| `cmd` cwd | invocation dir | config dir | +| `env.sh` cwd | invocation dir | invocation dir | +| `checksum` | config dir | config dir / `work_dir` | +| `env_file` | config dir | config dir | +| `work_dir` base | invocation dir | invocation dir | + +On `0.0.62`, `checksum: [data.txt]` and `cmd: cat data.txt` in the same command could +refer to different files. On `0.0.63`, `cmd` and `env.sh` disagreed instead. The root +cause of both was one field, `Config.WorkDir`, carrying two meanings: "where the config +is" and "where commands run". + +**Remote configs** (ADR-0003) already special-cased this by pinning their root to the +invocation dir, since a cached config has no meaningful local directory. That special case +was evidence that the invocation dir was the right general answer. + +## Decision + +Split the conflated field into two, and define resolution in terms of when it happens. + +- **Root dir** — the directory `lets` was invoked from. Commands run here by default. +- **Config dir** — the directory holding the config file. +- **Work dir** — a command's actual directory: the Root dir, or its `work_dir` if set. + +Two rules: + +1. **Config assembly** resolves against the config file that declares it. Local `mixins` + paths are the only thing in this category. +2. **Everything a command reads or runs** resolves against that command's Work dir. This + covers `cmd`, `checksum` paths, `env_file` paths at both global and command scope, and + `env.sh` scripts. A relative `work_dir` resolves against the Root dir. + +`--config` / `-c`, `--config-dir` and `LETS_CONFIG_DIR` select which config is loaded and +never change the Root dir. + +`.lets/` is created in the Root dir, so a persisted checksum stays paired with the files it +was computed from. + +Remote configs stop being a special case: their root is the invocation dir under the +general rule. Because their Config dir is a cache directory holding only the downloaded +YAML, a remote config declaring a local `mixins` path is rejected with an explicit error. + +## Consequences + +- `lets foo` does what typing `foo` at the prompt would do, and a command is readable + without knowing where its config lives. +- `lets -c ../other/lets.yaml build` borrows another project's commands and runs them on + the invoking project's files, which is the only useful reading of `-c`. +- A filename appearing in two directives of one command now means one file. +- Breaking relative to `0.0.63`: commands run in the invocation dir again. +- Breaking relative to every previous version: `checksum` and `env_file` follow the Work + dir, and `.lets/` follows the Root dir. +- A relative `work_dir` now depends on where the user stands, so `work_dir: docs` works + from the project root and fails from a subdirectory. Commands that must always target the + project use `cd "${LETS_CONFIG_DIR}/…"` inside `cmd`. `work_dir` does not expand + environment variables. +- `LETS_CONFIG_DIR` for a remote config reports the cache directory rather than the cwd. + `$PWD` is the way to reach the project. + +## Alternatives considered + +**Keep `0.0.63` — Root dir is the Config dir.** Every path in a config would mean the same +thing regardless of where `lets` ran, and `lets x` would be identical from any directory. +Rejected because it makes "act on where I am" inexpressible: nothing exposes the invocation +dir, so it would have required a new `LETS_INVOCATION_DIR`. The chosen model needs no new +API — `$LETS_CONFIG_DIR` already provides the inverse escape hatch and always has. + +**Make everything cwd-relative, including mixins.** Fully uniform, and the honest version +of pre-`0.0.63` behavior. Rejected because `mixins: [./common.yaml]` would break whenever +`lets` ran from a subdirectory, making configs with mixins unloadable from anywhere but +their own directory. + +**Leave `checksum` and `env_file` resolving against the Config dir.** Closest to a literal +revert of `0.0.63`. Rejected because it preserves the original defect — one command +definition resolving the same filename against two directories. + +**Put `.lets/` next to the config rather than in the Root dir.** Avoids stray `.lets/` +directories when running from subdirectories. Rejected because a persisted checksum stored +next to the config but computed from an arbitrary directory would describe different files +on different runs, making change detection unreliable. diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 3d233418..bb31efe4 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -5,6 +5,13 @@ title: Changelog ## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X) +* `[Changed]` **Breaking.** Commands run in the directory `lets` was invoked from, whatever config was loaded and wherever that config lives. In `0.0.63` they ran in the config file's directory instead, which changed behaviour for `lets -c some/dir/lets.yaml` and for running `lets` from a subdirectory of a project. See [Where commands run](/docs/where_commands_run). +* `[Changed]` **Breaking.** Everything a command reads or runs now resolves against a single directory — the command's working dir, which is the root dir unless the command sets `work_dir`. This covers `cmd`, `checksum` file paths, `env_file` paths and `env.sh` scripts. Previously these disagreed: `checksum` and `env_file` resolved against the config directory while `env.sh` ran in the invocation directory, so the same filename in one command definition could mean two different directories. +* `[Changed]` **Breaking.** `.lets/` is created in the root dir rather than next to the config file, so persisted checksums stay paired with the files they were computed from. +* `[Changed]` A remote config that declares a local `mixins` path now fails with an explicit error instead of silently resolving it against the invocation directory. Remote configs can only mix in URLs. +* `[Changed]` `LETS_CONFIG_DIR` at command runtime is the config file's real directory for remote configs too (the local cache directory); previously it reported the invocation directory. Use `$PWD` for the root dir. +* `[Fixed]` `work_dir` no longer resolves inconsistently with the rest of the command: relative paths resolve against the root dir, and `work_dir` now also moves `checksum`, `env_file` and `env.sh` resolution. +* `[Fixed]` A remote config whose `RemoteSource` was only recorded after parsing meant remote-specific mixin handling never applied during load. * `[Changed]` Group and delay Dependabot version updates, enable updates for docs and examples, and validate those projects in pull request CI. * `[Fixed]` Restore the documentation and Python example builds after dependency updates. diff --git a/docs/docs/config.md b/docs/docs/config.md index 0b078353..bc0f6afc 100644 --- a/docs/docs/config.md +++ b/docs/docs/config.md @@ -4,6 +4,7 @@ title: Config reference --- - [Agent Skills](#agent-skills) +- [Where commands run](where_commands_run.md) - [Top-level directives:](#top-level-directives) - [Version](#version) - [Shell](#shell) @@ -41,6 +42,17 @@ Agent Skills are not configured in `lets.yaml`. They are installed and managed w Use [`lets self skills`](agent_skills.md) to show, install, update, or remove the bundled `lets` agent skill. +## Where commands run + +Commands run in the directory you ran `lets` from, not in the directory the config lives +in. Everything a command reads or runs — `cmd`, [`checksum`](#checksum) paths, +[`env_file`](#env_file) paths and `env.sh` — resolves against that one directory, or +against [`work_dir`](#work_dir) if the command sets one. Local [`mixins`](#mixins) paths +are the exception: they resolve against the config file that declares them. + +See **[Where commands run](where_commands_run.md)** for the full rules, every way of +pointing `lets` at a config, and the reasoning. + ## Top-level directives: ### Version @@ -134,7 +146,7 @@ env_file: Rules: - `-filename` is a short form of `required: false` -- files are resolved relative to the config directory +- files are resolved relative to the [root dir](where_commands_run.md) — the directory you ran `lets` from - file names are expanded after global `env` is resolved, so `env_file` can depend on global `env` - values loaded from `env_file` have higher precedence than values from `env` - missing files fail by default @@ -356,7 +368,8 @@ lets -c https://example.com/lets.yaml build Lets will download the config and cache it in `~/.config/lets/remote-configs`. Use `--no-cache` to force lets to re-download the remote config instead of using the cached copy. -Commands from a remote config run from the directory where `lets` was invoked unless the command specifies `work_dir`. +Commands from a remote config run in the [root dir](where_commands_run.md), exactly like commands from a local one. +A remote config can only mix in other URLs — a local `mixins` path is an error, since the config has no local directory to resolve it against. When stderr is an interactive terminal, lets shows download progress for remote config downloads. Cache hits do not show progress. @@ -518,7 +531,12 @@ Usage: lets hello `type: string` -Specify work directory to run in. Path must be relative to project root. Be default command's workdir is project root (where lets.yaml located). +Specify the directory to run the command in. A relative path resolves against the +[root dir](where_commands_run.md) — the directory you ran `lets` from. Absolute paths are +used as-is. By default a command runs in the root dir itself. + +`work_dir` moves everything the command touches, not just `cmd`: [`checksum`](#checksum) +file paths, [`env_file`](#env_file) paths and `env.sh` scripts all resolve against it too. Example: @@ -530,6 +548,17 @@ commands: cmd: npm start ``` +Since the path is relative to where you ran `lets`, `lets run-docs` works from the +project root and fails from a subdirectory. `work_dir` does not expand env variables, +so anchor the command itself when it should always target the same place regardless of +where it is run from: + +```yaml +commands: + run-docs: + cmd: cd "${LETS_CONFIG_DIR}/docs" && npm start +``` + ### `shell` `key: shell` @@ -786,7 +815,7 @@ Rules: - command `env` is resolved first - command `env_file` file names are expanded using builtin lets vars, merged global env, and resolved command `env` - values loaded from command `env_file` override values from command `env` -- paths are resolved relative to the config directory, not `work_dir` +- paths are resolved relative to the command's working dir, so they follow `work_dir` Example: diff --git a/docs/docs/env.md b/docs/docs/env.md index 494dd8db..0c796f2e 100644 --- a/docs/docs/env.md +++ b/docs/docs/env.md @@ -17,9 +17,9 @@ title: Environment * `LETS_COMMAND_NAME` - string name of launched command * `LETS_COMMAND_ARGS` - positional arguments for launched command, e.g. for `lets run --debug --config=test.ini` it will contain `--debug --config=test.ini` -* `LETS_COMMAND_WORK_DIR` - absolute path to `work_dir` specified in command. -* `LETS_CONFIG` - absolute path to lets config file. -* `LETS_CONFIG_DIR` - absolute path to lets config file firectory. +* `LETS_COMMAND_WORK_DIR` - absolute path to the directory the command runs in: the root dir, or the command's `work_dir` if it sets one. +* `LETS_CONFIG` - absolute path to lets config file. For a remote config this is the URL it was loaded from. +* `LETS_CONFIG_DIR` - absolute path to the directory holding the config file. Use it to target the project rather than the directory you ran `lets` from. For a remote config this is the local cache directory. * `LETS_OS` - current operating system name from Go runtime, for example `linux`, `darwin`, `windows` * `LETS_ARCH` - current architecture name from Go runtime, for example `amd64`, `arm64`, `386` * `LETS_SHELL` - shell from config or command. diff --git a/docs/docs/where_commands_run.md b/docs/docs/where_commands_run.md new file mode 100644 index 00000000..8c69ac53 --- /dev/null +++ b/docs/docs/where_commands_run.md @@ -0,0 +1,171 @@ +--- +id: where_commands_run +title: Where commands run +--- + +## Short answer + +**`lets` runs your commands in the directory you ran `lets` from.** Not in the directory +`lets.yaml` lives in. + +```yaml +shell: bash +commands: + where: + cmd: pwd +``` + +```console +$ cd ~/myproject && lets where +/home/you/myproject + +$ cd ~/myproject/src && lets where # lets.yaml is still up in ~/myproject +/home/you/myproject/src +``` + +If a command should always act on the project instead of on your current directory, +send it there yourself with `$LETS_CONFIG_DIR`: + +```yaml +commands: + build: + cmd: cd "${LETS_CONFIG_DIR}" && go build ./... +``` + +## The three directories + +`lets` distinguishes three directories. Most confusion comes from expecting one of them +to be another. + +| | What it is | How to reach it | +| --- | --- | --- | +| **Root dir** | The directory you ran `lets` from. Commands run here by default. | `$PWD`, `$(pwd)` | +| **Config dir** | The directory holding `lets.yaml`. | `$LETS_CONFIG_DIR` | +| **Work dir** | The directory a specific command runs in: the root dir, or [`work_dir`](config.md#work_dir) if the command sets one. | `$LETS_COMMAND_WORK_DIR` | + +The root dir and the config dir are the same directory whenever you run `lets` from the +project root, which is the common case. They only differ when you run `lets` from a +subdirectory, or point `-c` at a config somewhere else. + +## What resolves against what + +There are exactly two rules. + +> **Config assembly** resolves against the config file that declares it. +> **Everything a command reads or runs** resolves against that command's work dir. + +| Directive | Resolves against | +| --- | --- | +| [`mixins`](config.md#mixins) (local paths) | the config file that declares it | +| [`cmd`](config.md#cmd) working directory | the command's work dir | +| [`checksum`](config.md#checksum) file paths and globs | the command's work dir | +| [`env_file`](config.md#env_file) paths, global and command | the command's work dir | +| `env.sh` scripts | the command's work dir | +| [`work_dir`](config.md#work_dir), when relative | the root dir | + +`mixins` is the only exception, and it has to be: a mixin is an include. If mixin paths +moved with your shell, a config would only be loadable from its own directory. + +## Every way of pointing lets at a config + +The root dir never depends on how `lets` found the config: + +| you run | root dir | +| --- | --- | +| `cd proj && lets x` | `proj` | +| `cd proj && lets -c lets.yaml x` | `proj` | +| `cd proj && lets -c sub/lets.yaml x` | `proj` | +| `cd proj && lets -c /abs/path/lets.yaml x` | `proj` | +| `cd proj/deep && lets x` — config found up the tree | `proj/deep` | +| `cd proj/deep && lets -c ../lets.yaml x` | `proj/deep` | +| `cd proj && lets -c https://example.com/lets.yaml x` | `proj` | +| `cd proj && LETS_CONFIG_DIR=sub lets x` | `proj` | + +`--config` / `-c`, `--config-dir` and `LETS_CONFIG_DIR` choose **which config is +loaded**. None of them changes **where commands run**. + +## Remote configs + +A [remote config](config.md#remote-configs) behaves exactly like a local one: commands +run in the directory you invoked `lets` from, which is the only sensible root, since the +config itself lives in a cache directory on your machine that holds nothing but the +downloaded YAML. + +Two consequences: + +- `LETS_CONFIG_DIR` points at that cache directory, so it is not useful for reaching your + project. Use `$PWD` instead. +- A remote config can only mix in other URLs. A local `mixins` path is an error, because + there is no local directory to resolve it against. + +## Where `.lets` goes + +The `.lets/` directory — persisted checksums and cached mixins — is created in the **root +dir**, so running `lets` from a subdirectory creates it there. + +This keeps a persisted checksum next to the files it was computed from. If `.lets` sat +next to the config while checksums were computed from wherever you happened to stand, the +same stored checksum would describe different files on different runs, and change +detection would flip back and forth. + +## Recipes + +| You want | Write | +| --- | --- | +| Act on the directory the user is in | nothing — that is the default | +| Always act on the project, wherever `lets` is run from | `cmd: cd "${LETS_CONFIG_DIR}" && …` | +| Always act on a fixed subdirectory of the project | `cmd: cd "${LETS_CONFIG_DIR}/docs" && …` | +| Act on a subdirectory relative to where the user is | `work_dir: docs` | +| Know where the user actually is | `$PWD` | + +`work_dir` takes a plain path and does not expand environment variables, so +`work_dir: ${LETS_CONFIG_DIR}/docs` will not work. Use `cd` inside `cmd` for that. + +## Why it works this way + +**Commands run where you are, because a config describes commands — it does not relocate +them.** `lets foo` should do what typing `foo` at your prompt would do. That makes a +command predictable from its own text: a relative path in `cmd` means what it looks like +it means, and you do not have to know where the config file happens to live to read it. + +It also keeps `-c` honest. `lets -c ../other-project/lets.yaml build` borrows another +project's commands and runs them on **your** data. If `-c` also moved the root dir, +loading someone else's config would silently retarget everything, and there would be no +way to express "use these commands here". + +**One work dir per command, for everything the command touches.** Before `0.0.63`, +`checksum` and `env_file` resolved against the config dir while `cmd` ran somewhere else, +so this command could hash one `data.txt` and read a different one: + +```yaml +commands: + build: + checksum: [data.txt] + cmd: cat data.txt +``` + +A single filename appearing twice in one command definition has to mean one file. Making +every command-scoped directive follow the same directory is what buys that, and it is +also what makes `work_dir` mean something coherent: it moves the whole command, not just +its script. + +**Mixins are the exception because they are resolved at a different time.** Mixins are +read while the config is being assembled, before any command exists and before there is a +command work dir to speak of. A mixin path is part of the config's own structure, like an +`import`, so it belongs to the file that wrote it. + +## History + +This behavior was accidental for a long time before it was specified. + +- Up to and including `0.0.62`, commands ran in the invocation dir — but by accident, via + a `filepath.Abs("")` that silently defaulted to the process cwd. `checksum` and + `env_file` resolved against the config dir at the same time, so the two disagreed. +- `0.0.63` fixed an unrelated `work_dir` bug and removed that accident, which moved the + root dir to the config dir. `env.sh` kept running in the invocation dir, so the + directives disagreed in a new way. +- The current release restores the invocation dir as the root and defines the resolution + rules explicitly, so every command-scoped directive agrees. + +See [ADR-0004](https://github.com/lets-cli/lets/blob/master/docs/adr/0004-root-dir-is-invocation-dir.md) +for the decision record, and the [changelog](changelog.md) for the exact breaking changes. diff --git a/docs/sidebars.js b/docs/sidebars.js index eaae095b..177aec02 100644 --- a/docs/sidebars.js +++ b/docs/sidebars.js @@ -37,6 +37,7 @@ module.exports = { }, ], }, + "where_commands_run", "config", "settings", "agent_skills", diff --git a/internal/config/config/command.go b/internal/config/config/command.go index e329a435..755811c5 100644 --- a/internal/config/config/command.go +++ b/internal/config/config/command.go @@ -6,8 +6,7 @@ import ( "errors" "fmt" "maps" - "path/filepath" - "strings" + "strings" "github.com/lets-cli/lets/internal/checksum" ) @@ -21,7 +20,8 @@ type Command struct { After string // overrides global shell for this particular command Shell string - // overrides global workdir (where lets.yaml is located) for this particular command + // work_dir as authored: overrides Config.RootDir for this command. + // Relative paths resolve against the root — see Config.CommandWorkDir. WorkDir string Description string // env from command @@ -110,14 +110,8 @@ func (c *Command) UnmarshalYAML(unmarshal func(any) error) error { c.Depends = cmd.Depends - if cmd.WorkDir != "" { - workDir, err := filepath.Abs(cmd.WorkDir) - if err != nil { - return err - } - - c.WorkDir = workDir - } + // kept as authored; resolved against the root by Config.CommandWorkDir at use + c.WorkDir = cmd.WorkDir c.After = cmd.After // TODO: checksum must be refactored @@ -169,7 +163,9 @@ func (c *Command) UnmarshalYAML(unmarshal func(any) error) error { return nil } -func (c *Command) GetEnv(cfg Config, builtinEnv map[string]string) (map[string]string, error) { +// GetEnv resolves a command's env. workDir is the command's working directory — +// env.sh runs there and env_file paths resolve against it. +func (c *Command) GetEnv(cfg Config, workDir string, builtinEnv map[string]string) (map[string]string, error) { baseEnv := cloneMap(builtinEnv) if baseEnv == nil { baseEnv = make(map[string]string) @@ -177,8 +173,13 @@ func (c *Command) GetEnv(cfg Config, builtinEnv map[string]string) (map[string]s maps.Copy(baseEnv, cfg.GetEnv()) + shell := cfg.Shell + if c.Shell != "" { + shell = c.Shell + } + envs := c.Env.Clone() - if err := envs.Execute(cfg, baseEnv); err != nil { + if err := envs.Execute(shell, workDir, baseEnv); err != nil { return nil, err } @@ -187,7 +188,7 @@ func (c *Command) GetEnv(cfg Config, builtinEnv map[string]string) (map[string]s envFiles := c.EnvFiles.Clone() - envFileEnv, err := envFiles.Load(cfg, filenameEnv) + envFileEnv, err := envFiles.Load(workDir, filenameEnv) if err != nil { return nil, fmt.Errorf("failed to resolve env_file for command '%s': %w", c.Name, err) } diff --git a/internal/config/config/config.go b/internal/config/config/config.go index 90d0509b..35053170 100644 --- a/internal/config/config/config.go +++ b/internal/config/config/config.go @@ -31,8 +31,12 @@ var keywords = set.NewSet[string]( // Config is a struct for loaded config file. type Config struct { - // absolute path to work dir - where config is placed - WorkDir string + // ConfigDir is the absolute path to the directory holding the config file. + // Only config assembly (mixin paths) resolves against it. + ConfigDir string + // RootDir is the absolute path commands run in by default. Everything a + // command reads or runs resolves against it, unless the command sets work_dir. + RootDir string // absolute path for lets config file FilePath string Commands Commands @@ -296,7 +300,16 @@ func (c *Config) readMixin(mixin *Mixin) error { } } } else { - mixinAbsPath, err := path.GetFullConfigPath(mixin.FileName, c.WorkDir) + // A remote config's ConfigDir is its cache dir, which only ever holds the + // downloaded yaml — a local mixin path there can never resolve. + if c.RemoteSource != "" { + return fmt.Errorf( + "remote config '%s' declares local mixin '%s': remote configs can only mix in URLs", + c.RemoteSource, mixin.FileName, + ) + } + + mixinAbsPath, err := path.GetFullConfigPath(mixin.FileName, c.ConfigDir) if err != nil { if mixin.Ignored && errors.Is(err, path.ErrFileNotExists) { return nil @@ -311,8 +324,8 @@ func (c *Config) readMixin(mixin *Mixin) error { return fmt.Errorf("failed to read mixin config %s: %w", mixin.FileName, err) } - // TODO(maybe bug): probably not filename but mixinAbsPath - mixinCfg := NewMixinConfig(c, mixin.FileName) + // abs path, so a nested mixin resolves against the dir of the file that declares it + mixinCfg := NewMixinConfig(c, mixinAbsPath) if err := yaml.NewDecoder(file).Decode(mixinCfg); err != nil { return fmt.Errorf("can not parse mixin config %s:\n%w", mixin.FileName, err) } @@ -360,14 +373,14 @@ func (c *Config) GetEnv() map[string]string { // SetupEnv must be called once. It is not intended to be called // multiple times hence does not have mutex. func (c *Config) SetupEnv() error { - if err := c.Env.Execute(*c, nil); err != nil { + if err := c.Env.Execute(c.Shell, c.RootDir, nil); err != nil { return err } filenameEnv := c.BuiltinEnv(c.Shell) maps.Copy(filenameEnv, c.Env.Dump()) - envFileEnv, err := c.EnvFiles.Load(*c, filenameEnv) + envFileEnv, err := c.EnvFiles.Load(c.RootDir, filenameEnv) if err != nil { return fmt.Errorf("failed to resolve global env_file: %w", err) } @@ -389,9 +402,24 @@ func (c *Config) SetupEnv() error { return nil } -func NewConfig(workDir string, configAbsPath string, dotLetsDir string) *Config { +// CommandWorkDir returns the absolute directory a command runs in. Everything the +// command reads or runs — cmd, checksum globs, env_file paths — resolves against it. +func (c *Config) CommandWorkDir(cmd *Command) string { + if cmd == nil || cmd.WorkDir == "" { + return c.RootDir + } + + if filepath.IsAbs(cmd.WorkDir) { + return cmd.WorkDir + } + + return filepath.Join(c.RootDir, cmd.WorkDir) +} + +func NewConfig(rootDir string, configAbsPath string, dotLetsDir string) *Config { return &Config{ - WorkDir: workDir, + RootDir: rootDir, + ConfigDir: filepath.Dir(configAbsPath), FilePath: configAbsPath, DotLetsDir: dotLetsDir, ChecksumsDir: filepath.Join(dotLetsDir, "checksums"), @@ -400,8 +428,10 @@ func NewConfig(workDir string, configAbsPath string, dotLetsDir string) *Config } func NewMixinConfig(cfg *Config, configAbsPath string) *Config { - mixin := NewConfig(cfg.WorkDir, configAbsPath, cfg.DotLetsDir) + mixin := NewConfig(cfg.RootDir, configAbsPath, cfg.DotLetsDir) mixin.isMixin = true + // a mixin of a remote config is itself remote — local mixin paths stay rejected down the chain + mixin.RemoteSource = cfg.RemoteSource mixin.SetDownloadOptions(cfg.context(), cfg.progressBar, cfg.noCache) return mixin diff --git a/internal/config/config/env.go b/internal/config/config/env.go index cbbd6f9c..1cf026d2 100644 --- a/internal/config/config/env.go +++ b/internal/config/config/env.go @@ -211,8 +211,9 @@ func convertEnvMapToList(envMap map[string]string) []string { } // eval env value and trim result string. -func executeScript(shell string, script string, envMap map[string]string) (string, error) { +func executeScript(shell string, dir string, script string, envMap map[string]string) (string, error) { cmd := exec.Command(shell, "-c", script) + cmd.Dir = dir envList := os.Environ() // Append resolved env last so it overrides process env keys (Go 1.21+ cmd.Env dedup: last value wins). envList = append(envList, convertEnvMapToList(envMap)...) @@ -228,9 +229,10 @@ func executeScript(shell string, script string, envMap map[string]string) (strin return strings.TrimSpace(res), nil } -// Execute executes env entries for sh scrips and calculate checksums +// Execute resolves sh and checksum env entries. baseDir is the directory sh +// scripts run in and checksum globs resolve against. // It is lazy and caches data on first call. -func (e *Envs) Execute(cfg Config, baseEnv map[string]string) error { +func (e *Envs) Execute(shell string, baseDir string, baseEnv map[string]string) error { if e == nil { return nil } @@ -247,7 +249,7 @@ func (e *Envs) Execute(cfg Config, baseEnv map[string]string) error { for _, key := range e.Keys { env := e.Mapping[key] if env.Sh != "" { - result, err := executeScript(cfg.Shell, env.Sh, resolvedEnv) + result, err := executeScript(shell, baseDir, env.Sh, resolvedEnv) if err != nil { return err } @@ -255,7 +257,7 @@ func (e *Envs) Execute(cfg Config, baseEnv map[string]string) error { env.Value = result e.Mapping[key] = env } else if len(env.Checksum) > 0 { - result, err := checksum.CalculateChecksum(cfg.WorkDir, env.Checksum[checksum.DefaultChecksumKey]) + result, err := checksum.CalculateChecksum(baseDir, env.Checksum[checksum.DefaultChecksumKey]) if err != nil { return err } diff --git a/internal/config/config/env_execute_test.go b/internal/config/config/env_execute_test.go index d37e82ff..7485fb8a 100644 --- a/internal/config/config/env_execute_test.go +++ b/internal/config/config/env_execute_test.go @@ -5,7 +5,7 @@ import "testing" func TestEnvsExecute(t *testing.T) { cfg := Config{ Shell: "bash", - WorkDir: ".", + RootDir: ".", } t.Run("resolves env entries sequentially", func(t *testing.T) { @@ -13,7 +13,7 @@ func TestEnvsExecute(t *testing.T) { envs.Set("ENGINE", Env{Name: "ENGINE", Value: "docker"}) envs.Set("COMPOSE", Env{Name: "COMPOSE", Sh: `echo "${ENGINE}-compose"`}) - err := envs.Execute(cfg, nil) + err := envs.Execute(cfg.Shell, cfg.RootDir, nil) if err != nil { t.Fatalf("unexpected execute error: %s", err) } @@ -27,7 +27,7 @@ func TestEnvsExecute(t *testing.T) { envs := &Envs{} envs.Set("COMPOSE", Env{Name: "COMPOSE", Sh: `echo "${ENGINE}-compose"`}) - err := envs.Execute(cfg, map[string]string{"ENGINE": "docker"}) + err := envs.Execute(cfg.Shell, cfg.RootDir, map[string]string{"ENGINE": "docker"}) if err != nil { t.Fatalf("unexpected execute error: %s", err) } @@ -44,7 +44,7 @@ func TestEnvsExecute(t *testing.T) { envs.Set("ENGINE", Env{Name: "ENGINE", Value: "docker"}) envs.Set("COMPOSE", Env{Name: "COMPOSE", Sh: `echo "${ENGINE}-compose"`}) - err := envs.Execute(cfg, nil) + err := envs.Execute(cfg.Shell, cfg.RootDir, nil) if err != nil { t.Fatalf("unexpected execute error: %s", err) } @@ -58,12 +58,12 @@ func TestEnvsExecute(t *testing.T) { envs := &Envs{} envs.Set("COMPOSE", Env{Name: "COMPOSE", Sh: `echo "${ENGINE}-compose"`}) - err := envs.Execute(cfg, map[string]string{"ENGINE": "docker"}) + err := envs.Execute(cfg.Shell, cfg.RootDir, map[string]string{"ENGINE": "docker"}) if err != nil { t.Fatalf("unexpected execute error: %s", err) } - err = envs.Execute(cfg, map[string]string{"ENGINE": "podman"}) + err = envs.Execute(cfg.Shell, cfg.RootDir, map[string]string{"ENGINE": "podman"}) if err != nil { t.Fatalf("unexpected execute error: %s", err) } diff --git a/internal/config/config/env_file.go b/internal/config/config/env_file.go index 32372745..43016c6d 100644 --- a/internal/config/config/env_file.go +++ b/internal/config/config/env_file.go @@ -116,7 +116,8 @@ func (e *EnvFiles) Append(other *EnvFiles) { e.Items = append(e.Items, other.Items...) } -func (e *EnvFiles) Load(cfg Config, envMap map[string]string) (map[string]string, error) { +// Load reads the env files. baseDir is the directory relative paths resolve against. +func (e *EnvFiles) Load(baseDir string, envMap map[string]string) (map[string]string, error) { if e == nil { return map[string]string{}, nil } @@ -134,7 +135,7 @@ func (e *EnvFiles) Load(cfg Config, envMap map[string]string) (map[string]string } if !filepath.IsAbs(filename) { - filename = filepath.Join(cfg.WorkDir, filename) + filename = filepath.Join(baseDir, filename) } if !util.FileExists(filename) { diff --git a/internal/config/config/env_file_test.go b/internal/config/config/env_file_test.go index 5ab61e5c..fe576060 100644 --- a/internal/config/config/env_file_test.go +++ b/internal/config/config/env_file_test.go @@ -166,7 +166,7 @@ func TestEnvFilesLoad(t *testing.T) { writeFixtureFile(t, workDir, ".env.second", "VALUE=second\nSECOND=two\n") writeFixtureFile(t, workDir, ".env.invalid", "NOT VALID\n") - cfg := Config{WorkDir: workDir} + cfg := Config{RootDir: workDir} t.Run("later files override earlier files", func(t *testing.T) { envFiles := &EnvFiles{ @@ -176,7 +176,7 @@ func TestEnvFilesLoad(t *testing.T) { }, } - got, err := envFiles.Load(cfg, nil) + got, err := envFiles.Load(cfg.RootDir, nil) if err != nil { t.Fatalf("unexpected load error: %s", err) } @@ -194,7 +194,7 @@ func TestEnvFilesLoad(t *testing.T) { }, } - got, err := envFiles.Load(cfg, nil) + got, err := envFiles.Load(cfg.RootDir, nil) if err != nil { t.Fatalf("unexpected load error: %s", err) } @@ -209,7 +209,7 @@ func TestEnvFilesLoad(t *testing.T) { Items: []EnvFile{{Name: ".env.missing", Required: true}}, } - _, err := envFiles.Load(cfg, nil) + _, err := envFiles.Load(cfg.RootDir, nil) if err == nil { t.Fatal("expected load error") } @@ -224,7 +224,7 @@ func TestEnvFilesLoad(t *testing.T) { Items: []EnvFile{{Name: ".env.invalid", Required: true}}, } - _, err := envFiles.Load(cfg, nil) + _, err := envFiles.Load(cfg.RootDir, nil) if err == nil { t.Fatal("expected load error") } @@ -302,7 +302,7 @@ func TestCommandGetEnvWithEnvFile(t *testing.T) { } cmd := cfg.Commands["echo"] - got, err := cmd.GetEnv(*cfg, cfg.CommandBuiltinEnv(cmd, cfg.Shell, cfg.WorkDir)) + got, err := cmd.GetEnv(*cfg, cfg.CommandWorkDir(cmd), cfg.CommandBuiltinEnv(cmd, cfg.Shell, cfg.RootDir)) if err != nil { t.Fatalf("unexpected command env error: %s", err) } @@ -336,13 +336,13 @@ func TestCommandGetEnvDoesNotReuseBuiltinEnvCache(t *testing.T) { cmd := cfg.Commands["echo"] cmd.Args = []string{"one"} - gotOne, err := cmd.GetEnv(*cfg, cfg.CommandBuiltinEnv(cmd, cfg.Shell, cfg.WorkDir)) + gotOne, err := cmd.GetEnv(*cfg, cfg.CommandWorkDir(cmd), cfg.CommandBuiltinEnv(cmd, cfg.Shell, cfg.RootDir)) if err != nil { t.Fatalf("unexpected command env error: %s", err) } cmd.Args = []string{"two"} - gotTwo, err := cmd.GetEnv(*cfg, cfg.CommandBuiltinEnv(cmd, cfg.Shell, cfg.WorkDir)) + gotTwo, err := cmd.GetEnv(*cfg, cfg.CommandWorkDir(cmd), cfg.CommandBuiltinEnv(cmd, cfg.Shell, cfg.RootDir)) if err != nil { t.Fatalf("unexpected command env error: %s", err) } diff --git a/internal/config/config/runtime_env.go b/internal/config/config/runtime_env.go index bad9c5a8..54f3f8ff 100644 --- a/internal/config/config/runtime_env.go +++ b/internal/config/config/runtime_env.go @@ -8,11 +8,12 @@ import ( func (c *Config) BuiltinEnv(shell string) map[string]string { letsConfig := filepath.Base(c.FilePath) - letsConfigDir := filepath.Dir(c.FilePath) + // ConfigDir, not RootDir: for a remote config this is the cache dir holding the + // downloaded yaml. The project root is simply the cwd now, so $PWD covers it. + letsConfigDir := c.ConfigDir if c.RemoteSource != "" { letsConfig = c.RemoteSource - letsConfigDir = c.WorkDir } return map[string]string{ diff --git a/internal/config/find.go b/internal/config/find.go index 973626ff..11061001 100644 --- a/internal/config/find.go +++ b/internal/config/find.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "os" "path/filepath" "github.com/lets-cli/lets/internal/config/path" @@ -15,7 +16,12 @@ const defaultConfigFile = "lets.yaml" type PathInfo struct { Filename string AbsPath string - WorkDir string + // ConfigDir is the directory holding the config file. Config assembly + // (mixin paths) resolves against it. + ConfigDir string + // RootDir is where commands run by default. Everything a command reads or + // runs resolves against it, unless the command sets work_dir. + RootDir string // .lets abs path DotLetsDir string } @@ -25,20 +31,20 @@ type PathInfo struct { // - if specified configName - try to load only that file // - if specified configDir - try to look for a config only in that dir - don't do recursion // - if not specified any of params above - try to find config recursively. -func FindConfig(configName string, configDir string) (PathInfo, error) { - configDirSpecifiedByUser := configDir != "" +func FindConfig(configName string, configDirFlag string) (PathInfo, error) { + configDirSpecifiedByUser := configDirFlag != "" if configName == "" { configName = defaultConfigFile } - // work dir is where to start looking for lets.yaml - workDir, err := getWorkDir(configName, configDir) + // searchDir is where to start looking for lets.yaml + searchDir, err := getSearchDir(configName, configDirFlag) if err != nil { return PathInfo{}, err } - log.Debugf("found %s config file in %s directory", configName, workDir) + log.Debugf("found %s config file in %s directory", configName, searchDir) configAbsPath := "" @@ -47,23 +53,31 @@ func FindConfig(configName string, configDir string) (PathInfo, error) { configAbsPath = configName } else { if configDirSpecifiedByUser { - configAbsPath, err = path.GetFullConfigPath(configName, workDir) + configAbsPath, err = path.GetFullConfigPath(configName, searchDir) if err != nil { return PathInfo{}, err } } else { // try to find abs config path up in parent dir tree - configAbsPath, err = path.GetFullConfigPathRecursive(configName, workDir) + configAbsPath, err = path.GetFullConfigPathRecursive(configName, searchDir) if err != nil { return PathInfo{}, err } } } - // just to be sure that work dir is correct - workDir = filepath.Dir(configAbsPath) + configDir := filepath.Dir(configAbsPath) - dotLetsDir, err := workdir.GetDotLetsDir(workDir) + // The root is the cwd, not the config dir: a config describes commands, it does + // not relocate them. --config-dir / LETS_CONFIG_DIR only steer discovery. + rootDir, err := os.Getwd() + if err != nil { + return PathInfo{}, fmt.Errorf("failed to get working directory: %w", err) + } + + // .lets follows the root, so persisted checksums stay paired with the files + // they were computed from. + dotLetsDir, err := workdir.GetDotLetsDir(rootDir) if err != nil { return PathInfo{}, fmt.Errorf("can not get .lets absolute path: %w", err) } @@ -74,7 +88,8 @@ func FindConfig(configName string, configDir string) (PathInfo, error) { pathInfo := PathInfo{ AbsPath: configAbsPath, - WorkDir: workDir, + ConfigDir: configDir, + RootDir: rootDir, Filename: configName, DotLetsDir: dotLetsDir, } diff --git a/internal/config/load.go b/internal/config/load.go index 1e3ba568..f8f07e2a 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -56,11 +56,11 @@ func LoadWithContext(ctx context.Context, configName string, configDir string, v return nil, err } - return loadConfigFromFile(ctx, configPath.AbsPath, configPath.WorkDir, configPath.DotLetsDir, configPath.Filename, version, opts) + return loadConfigFromFile(ctx, configPath.AbsPath, configPath.RootDir, configPath.DotLetsDir, configPath.Filename, "", version, opts) } -// LoadRemote downloads (or loads from cache) a remote lets.yaml at url and -// returns a Config with the working directory set to the caller's CWD. +// LoadRemote downloads (or loads from cache) a remote lets.yaml at url. +// Its root is the caller's cwd, same as for a local config. func LoadRemote(ctx context.Context, url string, noCache bool, version string, options ...LoadOption) (*config.Config, error) { opts := newLoadOptions(options) if noCache { @@ -86,21 +86,21 @@ func LoadRemote(ctx context.Context, url string, noCache bool, version string, o return nil, fmt.Errorf("can not create .lets dir: %w", err) } - c, err := loadConfigFromFile(ctx, cachedPath, cwd, dotLetsDir, url, version, opts) + c, err := loadConfigFromFile(ctx, cachedPath, cwd, dotLetsDir, url, url, version, opts) if err != nil { return nil, fmt.Errorf("%w (use --no-cache to re-download)", err) } - c.RemoteSource = url - return c, nil } // loadConfigFromFile is shared by Load and LoadRemote: opens the file at absPath, // decodes YAML, validates, and sets up env. displayName appears in parse error messages. +// remoteSource is the URL the config came from, empty for local configs; it must be set +// before decoding, since mixin resolution during decode depends on it. func loadConfigFromFile( ctx context.Context, - absPath, workDir, dotLetsDir, displayName, version string, + absPath, rootDir, dotLetsDir, displayName, remoteSource, version string, opts loadOptions, ) (*config.Config, error) { f, err := os.Open(absPath) @@ -109,7 +109,8 @@ func loadConfigFromFile( } defer f.Close() - c := config.NewConfig(workDir, absPath, dotLetsDir) + c := config.NewConfig(rootDir, absPath, dotLetsDir) + c.RemoteSource = remoteSource c.SetDownloadOptions(ctx, opts.progress, opts.noCache) if err := yaml.NewDecoder(f).Decode(c); err != nil { diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 92c7cdf0..520e7c39 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -310,3 +310,104 @@ func TestLoadRemote(t *testing.T) { } }) } + +// The root dir is where lets was invoked, never where the config file sits. +func TestRootDirIsInvocationDir(t *testing.T) { + ctx := context.Background() + + t.Run("local config in a child dir does not move the root", func(t *testing.T) { + root := t.TempDir() + configDir := filepath.Join(root, "sub") + if err := os.MkdirAll(configDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + writeFile(t, filepath.Join(configDir, "lets.yaml"), "shell: bash\ncommands:\n hi:\n cmd: echo hi\n") + + t.Chdir(root) + + cfg, err := LoadWithContext(ctx, "sub/lets.yaml", "", "0.0.0-test") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + assertSameDir(t, "RootDir", cfg.RootDir, root) + assertSameDir(t, "ConfigDir", cfg.ConfigDir, configDir) + }) + + t.Run("remote config roots at the cwd, not the cache dir", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte("shell: bash\ncommands:\n hi:\n cmd: echo hi\n")) + })) + defer srv.Close() + + root := t.TempDir() + home := t.TempDir() + t.Setenv("HOME", home) + t.Chdir(root) + + cfg, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + assertSameDir(t, "RootDir", cfg.RootDir, root) + assertSameDir(t, "ConfigDir", cfg.ConfigDir, filepath.Join(home, ".config", "lets", "remote-configs")) + + // what a command actually receives, not just the field behind it + builtin := cfg.BuiltinEnv(cfg.Shell) + assertSameDir(t, "LETS_CONFIG_DIR", builtin["LETS_CONFIG_DIR"], cfg.ConfigDir) + + if builtin["LETS_CONFIG"] != srv.URL { + t.Fatalf("expected LETS_CONFIG=%q, got %q", srv.URL, builtin["LETS_CONFIG"]) + } + }) +} + +// A remote config's ConfigDir is its cache dir, which only holds the downloaded +// yaml, so a local mixin path there can never resolve. +func TestRemoteConfigRejectsLocalMixin(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte("shell: bash\nmixins:\n - local.yaml\ncommands:\n hi:\n cmd: echo hi\n")) + })) + defer srv.Close() + + root := t.TempDir() + t.Setenv("HOME", t.TempDir()) + t.Chdir(root) + + // present next to the cwd, to prove it is not picked up from there either + writeFile(t, filepath.Join(root, "local.yaml"), "commands:\n local:\n cmd: echo local\n") + + _, err := LoadRemote(context.Background(), srv.URL, false, "0.0.0-test") + if err == nil { + t.Fatal("expected local mixin in a remote config to fail") + } + if !strings.Contains(err.Error(), "can only mix in URLs") { + t.Fatalf("unexpected error: %v", err) + } +} + +func writeFile(t *testing.T, path string, content string) { + t.Helper() + + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +// macOS resolves t.TempDir() under /var, a symlink to /private/var, while +// os.Getwd reports the resolved path. +func assertSameDir(t *testing.T, name string, got string, want string) { + t.Helper() + + resolved, err := filepath.EvalSymlinks(want) + if err != nil { + t.Fatalf("resolve %s: %v", want, err) + } + + if got != resolved && got != want { + t.Fatalf("expected %s=%q, got %q", name, resolved, got) + } +} diff --git a/internal/config/migrate/migrate.go b/internal/config/migrate/migrate.go index 7f5d72db..1ad5cc42 100644 --- a/internal/config/migrate/migrate.go +++ b/internal/config/migrate/migrate.go @@ -39,7 +39,7 @@ func Fix(configName string, configDir string, dryRun bool, out io.Writer) (Resul return Result{}, err } - paths, remoteMixins, err := collectConfigPaths(pathInfo.AbsPath, pathInfo.WorkDir) + paths, remoteMixins, err := collectConfigPaths(pathInfo.AbsPath, pathInfo.ConfigDir) if err != nil { return Result{}, err } diff --git a/internal/config/workdir.go b/internal/config/workdir.go index cb2c4b18..00d6a7d0 100644 --- a/internal/config/workdir.go +++ b/internal/config/workdir.go @@ -5,16 +5,17 @@ import ( "os" ) -// workDir is where lets.yaml found or rootDir points to. -func getWorkDir(filename string, rootDir string) (string, error) { - workDir, err := os.Getwd() +// getSearchDir is where lets starts looking for the config file: the process +// cwd, or rootDir when the user pinned one via --config-dir / LETS_CONFIG_DIR. +func getSearchDir(filename string, rootDir string) (string, error) { + searchDir, err := os.Getwd() if err != nil { return "", fmt.Errorf("failed to get workdir for config %s: %w", filename, err) } if rootDir != "" { - workDir = rootDir + searchDir = rootDir } - return workDir, nil + return searchDir, nil } diff --git a/internal/executor/execute_test.go b/internal/executor/execute_test.go index 975990e7..3140a767 100644 --- a/internal/executor/execute_test.go +++ b/internal/executor/execute_test.go @@ -84,7 +84,7 @@ func newTestCfg(t *testing.T) *config.Config { t.Fatalf("newTestCfg: create .lets dir: %v", err) } return &config.Config{ - WorkDir: dir, + RootDir: dir, FilePath: filepath.Join(dir, "lets.yaml"), Shell: "sh", Commands: config.Commands{}, @@ -399,11 +399,11 @@ func TestChecksumEnvVarsPresentInRunnerInvocation(t *testing.T) { func TestChecksumUsesCommandWorkDir(t *testing.T) { cfg := newTestCfg(t) - commandDir := filepath.Join(cfg.WorkDir, "command-dir") + commandDir := filepath.Join(cfg.RootDir, "command-dir") if err := os.Mkdir(commandDir, 0o755); err != nil { t.Fatalf("create command dir: %v", err) } - if err := os.WriteFile(filepath.Join(cfg.WorkDir, "input.txt"), []byte("root"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(cfg.RootDir, "input.txt"), []byte("root"), 0o644); err != nil { t.Fatalf("write root input: %v", err) } if err := os.WriteFile(filepath.Join(commandDir, "input.txt"), []byte("command"), 0o644); err != nil { diff --git a/internal/executor/executor.go b/internal/executor/executor.go index ca4b2a7c..7382a04a 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -190,10 +190,7 @@ func (e *Executor) initCmd(ctx *Context) error { checksumShell = cmd.Shell } - checksumWorkDir := e.cfg.WorkDir - if cmd.WorkDir != "" { - checksumWorkDir = cmd.WorkDir - } + checksumWorkDir := e.cfg.CommandWorkDir(cmd) checksumEnv := e.cfg.CommandBuiltinEnv(cmd, checksumShell, checksumWorkDir) maps.Copy(checksumEnv, e.cfg.GetEnv()) diff --git a/internal/executor/runner.go b/internal/executor/runner.go index e970e6f6..63fcc578 100644 --- a/internal/executor/runner.go +++ b/internal/executor/runner.go @@ -45,10 +45,7 @@ func (r *shellRunner) run(command *config.Command, cmdScript string) error { osCmd.Stderr = r.out osCmd.Stdin = os.Stdin - osCmd.Dir = r.cfg.WorkDir - if command.WorkDir != "" { - osCmd.Dir = command.WorkDir - } + osCmd.Dir = r.cfg.CommandWorkDir(command) if err := r.setupEnv(osCmd, command, shell); err != nil { return err @@ -74,7 +71,7 @@ func (r *shellRunner) setupEnv(osCmd *exec.Cmd, command *config.Command, shell s ) } - cmdEnv, err := command.GetEnv(*r.cfg, defaultEnv) + cmdEnv, err := command.GetEnv(*r.cfg, osCmd.Dir, defaultEnv) if err != nil { return err } diff --git a/tests/command_checksum.bats b/tests/command_checksum.bats index ed7460ec..b4823bac 100644 --- a/tests/command_checksum.bats +++ b/tests/command_checksum.bats @@ -7,6 +7,8 @@ setup() { } ALL_CHECKSUM="be48892c650a32df361202a3662f31e5eac2b83c" +# same file names, different contents, under ./subdir +SUBDIR_CHECKSUM="7506688b525201813110ff3598eed02b016b1775" FOO_CHECKSUM="833330f14e30e3ce1907f1e126e1ea4db1ec349f" BAR_CHECKSUM="7917368d518c031517855672acf2ef82b9cb6836" @@ -60,9 +62,11 @@ CHECKSUM_FROM_FOO_AND_BAR_CHECKSUMS="b778d48759ad4e6e9a755bd595d23eeaa2f7ff65" } -@test "command_checksum: should calculate checksum from sub-dir" { +@test "command_checksum: checksum files resolve against the dir lets was invoked from" { + # subdir holds its own foo_*/bar_* files, so running there must checksum those, + # not the ones next to lets.yaml — checksum follows the command's work dir cd ./subdir run lets as-list-of-files assert_success - assert_line --index 0 ${ALL_CHECKSUM} + assert_line --index 0 ${SUBDIR_CHECKSUM} } diff --git a/tests/command_checksum/subdir/bar_1.txt b/tests/command_checksum/subdir/bar_1.txt new file mode 100644 index 00000000..4d34fb4b --- /dev/null +++ b/tests/command_checksum/subdir/bar_1.txt @@ -0,0 +1 @@ +subdir-bar-1 diff --git a/tests/command_checksum/subdir/foo_1.txt b/tests/command_checksum/subdir/foo_1.txt new file mode 100644 index 00000000..e7264f96 --- /dev/null +++ b/tests/command_checksum/subdir/foo_1.txt @@ -0,0 +1 @@ +subdir-foo-1 diff --git a/tests/command_checksum/subdir/foo_2.txt b/tests/command_checksum/subdir/foo_2.txt new file mode 100644 index 00000000..aa153cd2 --- /dev/null +++ b/tests/command_checksum/subdir/foo_2.txt @@ -0,0 +1 @@ +subdir-foo-2 diff --git a/tests/command_work_dir.bats b/tests/command_work_dir.bats index b9c11c7a..7a8de23d 100644 --- a/tests/command_work_dir.bats +++ b/tests/command_work_dir.bats @@ -4,10 +4,41 @@ setup() { load "${BATS_UTILS_PATH}/bats-support/load.bash" load "${BATS_UTILS_PATH}/bats-assert/load.bash" cd ./tests/command_work_dir + find . -type d -name ".lets" -exec rm -rf {} + } +PROJECT_CHECKSUM="8d856bf15117bf927d7a12caf3fb427f1cdd600f" +ROOT_CHECKSUM="10bb3cf83b3ba687e54b9d15ab8d15e282ccd6f0" + @test "command_work_dir: should run command in work_dir" { run lets print-file assert_success assert_line --index 0 "hi there" } + +@test "command_work_dir: checksum, env_file and env.sh all follow work_dir" { + # ./input.txt and ./.env exist in both dirs with different contents, so each + # line below would differ if that directive resolved against the root instead + run lets everything-follows-work-dir + assert_success + assert_line --index 0 "cmd=project" + assert_line --index 1 "file=project-content" + assert_line --index 2 "env_file=project" + assert_line --index 3 "env_sh=project" + assert_line --index 4 "checksum=${PROJECT_CHECKSUM}" +} + +@test "command_work_dir: without work_dir everything resolves against the root" { + run lets everything-follows-root + assert_success + assert_line --index 0 "cmd=command_work_dir" + assert_line --index 1 "file=other-content" + assert_line --index 2 "env_file=root" + assert_line --index 3 "env_sh=command_work_dir" + assert_line --index 4 "checksum=${ROOT_CHECKSUM}" +} + +@test "command_work_dir: checksum differs between work_dir and root" { + # guards the two checksums above against both collapsing to the same value + [[ "${PROJECT_CHECKSUM}" != "${ROOT_CHECKSUM}" ]] +} diff --git a/tests/command_work_dir/.env.wd b/tests/command_work_dir/.env.wd new file mode 100644 index 00000000..476d5002 --- /dev/null +++ b/tests/command_work_dir/.env.wd @@ -0,0 +1 @@ +WHO=root diff --git a/tests/command_work_dir/input.txt b/tests/command_work_dir/input.txt new file mode 100644 index 00000000..17098dda --- /dev/null +++ b/tests/command_work_dir/input.txt @@ -0,0 +1 @@ +other-content diff --git a/tests/command_work_dir/lets.yaml b/tests/command_work_dir/lets.yaml index f84de9cf..661d2163 100644 --- a/tests/command_work_dir/lets.yaml +++ b/tests/command_work_dir/lets.yaml @@ -4,3 +4,35 @@ commands: print-file: work_dir: project cmd: cat text.txt + + # work_dir moves everything the command touches, not just cmd. + # Both dirs hold an input.txt and a .env with different contents, so each + # assertion below fails if the directive resolved against the root instead. + everything-follows-work-dir: + work_dir: project + checksum: + files: [input.txt] + env_file: .env.wd + env: + ENV_SH_DIR: + sh: basename "$(pwd)" + cmd: | + echo "cmd=$(basename "$(pwd)")" + echo "file=$(cat input.txt)" + echo "env_file=${WHO}" + echo "env_sh=${ENV_SH_DIR}" + echo "checksum=${LETS_CHECKSUM}" + + everything-follows-root: + checksum: + files: [input.txt] + env_file: .env.wd + env: + ENV_SH_DIR: + sh: basename "$(pwd)" + cmd: | + echo "cmd=$(basename "$(pwd)")" + echo "file=$(cat input.txt)" + echo "env_file=${WHO}" + echo "env_sh=${ENV_SH_DIR}" + echo "checksum=${LETS_CHECKSUM}" diff --git a/tests/command_work_dir/project/.env.wd b/tests/command_work_dir/project/.env.wd new file mode 100644 index 00000000..ac4b4738 --- /dev/null +++ b/tests/command_work_dir/project/.env.wd @@ -0,0 +1 @@ +WHO=project diff --git a/tests/command_work_dir/project/input.txt b/tests/command_work_dir/project/input.txt new file mode 100644 index 00000000..f5a3f563 --- /dev/null +++ b/tests/command_work_dir/project/input.txt @@ -0,0 +1 @@ +project-content diff --git a/tests/find_config.bats b/tests/find_config.bats index 1e3440c3..7d7df66c 100644 --- a/tests/find_config.bats +++ b/tests/find_config.bats @@ -15,13 +15,15 @@ setup() { assert_line --index 0 "foo" } -@test "find_config: .lets must be created in the same dir where lets.yaml placed" { +@test "find_config: .lets must be created in the dir lets was invoked from" { + # .lets follows the root dir, so persisted checksums stay paired with the + # files they were computed from cd a/b run lets foo assert_success - [[ ! -d .lets ]] - [[ -d ../../.lets ]] + [[ -d .lets ]] + [[ ! -d ../../.lets ]] } @test "find_config: LETS_CONFIG changes which config file to read" { diff --git a/tests/mixins.bats b/tests/mixins.bats index 986cda67..ddc2b8b7 100644 --- a/tests/mixins.bats +++ b/tests/mixins.bats @@ -9,3 +9,23 @@ setup() { assert_success assert_line --index 0 "Hello" } + +@test "mixins: mixin path in a subdir is loaded" { + run lets hello-from-subdir-mixin + assert_success + assert_line --index 0 "Hello from sub/outer.yaml" +} + +@test "mixins: mixin paths resolve against the config dir, not the root dir" { + # run from sub/, so the root dir is sub/ — the mixin paths in ../lets.yaml + # ('sub/outer.yaml', 'lets.mix.yaml') only resolve if they are taken + # relative to the config file rather than to where lets was invoked + cd sub + run lets -c ../lets.yaml hello-from-subdir-mixin + assert_success + assert_line --index 0 "Hello from sub/outer.yaml" + + run lets -c ../lets.yaml hello-from-minix + assert_success + assert_line --index 0 "Hello" +} diff --git a/tests/mixins/lets.yaml b/tests/mixins/lets.yaml index 3921feee..b245482d 100644 --- a/tests/mixins/lets.yaml +++ b/tests/mixins/lets.yaml @@ -1,5 +1,6 @@ shell: bash mixins: + - sub/outer.yaml - lets.mix.yaml - -lets.no.yaml diff --git a/tests/mixins/sub/outer.yaml b/tests/mixins/sub/outer.yaml new file mode 100644 index 00000000..9e7930a7 --- /dev/null +++ b/tests/mixins/sub/outer.yaml @@ -0,0 +1,3 @@ +commands: + hello-from-subdir-mixin: + cmd: echo "Hello from sub/outer.yaml" diff --git a/tests/root_dir.bats b/tests/root_dir.bats new file mode 100644 index 00000000..051876c8 --- /dev/null +++ b/tests/root_dir.bats @@ -0,0 +1,63 @@ +load test_helpers + +setup() { + load "${BATS_UTILS_PATH}/bats-support/load.bash" + load "${BATS_UTILS_PATH}/bats-assert/load.bash" + cd ./tests/root_dir + find . -type d -name ".lets" -exec rm -rf {} + +} + +# The root dir is the dir lets was invoked from, never the dir the config lives in. +# Each test below pins one way of pointing lets at a config. + +@test "root_dir: config in cwd, no -c" { + run lets pwd + assert_success + assert_line --index 0 "" +} + +@test "root_dir: -c naming a config in cwd" { + run lets -c lets.yaml pwd + assert_success + assert_line --index 0 "" +} + +@test "root_dir: -c pointing into a child dir does not move the root" { + run lets -c sub/lets.yaml pwd + assert_success + assert_line --index 0 "" +} + +@test "root_dir: config found recursively up the tree does not move the root" { + cd deep/nested + run lets pwd + assert_success + assert_line --index 0 "/deep/nested" +} + +@test "root_dir: -c pointing at a parent config does not move the root" { + cd sub + run lets -c ../lets.yaml pwd + assert_success + assert_line --index 0 "/sub" +} + +@test "root_dir: LETS_CONFIG_DIR steers discovery but not the root" { + LETS_CONFIG_DIR=sub run lets pwd + assert_success + assert_line --index 0 "" +} + +@test "root_dir: work_dir resolves against the root" { + run lets pwd-with-work-dir + assert_success + assert_line --index 0 "/wd" +} + +@test "root_dir: work_dir resolves against the root, not the config dir" { + # invoked from deep/, so work_dir 'wd' means deep/wd — which does not exist + cd deep + run lets -c ../lets.yaml pwd-with-work-dir + assert_failure + assert_output --partial "deep/wd" +} diff --git a/tests/root_dir/deep/nested/.gitkeep b/tests/root_dir/deep/nested/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/tests/root_dir/lets.yaml b/tests/root_dir/lets.yaml new file mode 100644 index 00000000..12195bae --- /dev/null +++ b/tests/root_dir/lets.yaml @@ -0,0 +1,11 @@ +shell: bash + +commands: + pwd: + description: print the dir the command runs in, relative to the fixture root + cmd: pwd | sed "s|.*/tests/root_dir||" + + pwd-with-work-dir: + description: work_dir resolves against the dir lets was invoked from + work_dir: wd + cmd: pwd | sed "s|.*/tests/root_dir||" diff --git a/tests/root_dir/sub/lets.yaml b/tests/root_dir/sub/lets.yaml new file mode 100644 index 00000000..12195bae --- /dev/null +++ b/tests/root_dir/sub/lets.yaml @@ -0,0 +1,11 @@ +shell: bash + +commands: + pwd: + description: print the dir the command runs in, relative to the fixture root + cmd: pwd | sed "s|.*/tests/root_dir||" + + pwd-with-work-dir: + description: work_dir resolves against the dir lets was invoked from + work_dir: wd + cmd: pwd | sed "s|.*/tests/root_dir||" diff --git a/tests/root_dir/wd/.gitkeep b/tests/root_dir/wd/.gitkeep new file mode 100644 index 00000000..e69de29b