From 1d376fd7e6775c45575132033eee447449b1653e Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Sun, 26 Apr 2026 17:19:32 +0300 Subject: [PATCH 1/7] Redesign checksum config and add migration tool --- docs/docs/advanced_usage.md | 18 +- docs/docs/changelog.md | 5 + docs/docs/cli.md | 2 + docs/docs/config.md | 47 ++-- docs/static/schema.json | 60 ++++- internal/checksum/checksum.go | 40 +++ internal/checksum/checksum_test.go | 13 + internal/cmd/fix.go | 36 +++ internal/cmd/self.go | 1 + internal/config/config/checksum.go | 86 ++++++- internal/config/config/command.go | 94 ++++--- internal/config/config/command_test.go | 76 ++++++ internal/config/migrate/checksum.go | 126 ++++++++++ internal/config/migrate/checksum_test.go | 116 +++++++++ internal/config/migrate/migrate.go | 298 +++++++++++++++++++++++ internal/executor/executor.go | 20 +- internal/logging/formatter.go | 8 + internal/logging/log.go | 8 + internal/logging/log_test.go | 19 +- lets.yaml | 11 +- tests/command_checksum.bats | 12 + tests/command_checksum/lets.yaml | 17 +- tests/command_persist_checksum.bats | 38 ++- tests/command_persist_checksum/lets.yaml | 12 +- tests/self_fix.bats | 43 ++++ tests/self_fix/lets.old.yaml | 12 + tests/self_fix/mixin.old.yaml | 7 + 27 files changed, 1143 insertions(+), 82 deletions(-) create mode 100644 internal/cmd/fix.go create mode 100644 internal/config/migrate/checksum.go create mode 100644 internal/config/migrate/checksum_test.go create mode 100644 internal/config/migrate/migrate.go create mode 100644 tests/self_fix.bats create mode 100644 tests/self_fix/lets.old.yaml create mode 100644 tests/self_fix/mixin.old.yaml diff --git a/docs/docs/advanced_usage.md b/docs/docs/advanced_usage.md index f811a209..9c255c41 100644 --- a/docs/docs/advanced_usage.md +++ b/docs/docs/advanced_usage.md @@ -91,17 +91,17 @@ But we have one downside - run `npm install` may take some time and we do not wa Checksums allow you to know when some of the files have changed and made a decision based on that. -When you add `checksum` directive to a command - `lets` will calculate checksum from all of the files listed in `checksum` and put `LETS_CHECKSUM` env variable to command env. +When you add `checksum.files` directive to a command - `lets` will calculate checksum from all of the listed files and put `LETS_CHECKSUM` env variable to command env. `LETS_CHECKSUM` will have a checksum value. We then can store this checksum somewhere in the file and check that stored checksum with a checksum from env. -Fortunately, `lets` have an option for that - `persist_checksum`. +Fortunately, `lets` have an option for that - `checksum.persist`. -If `persist_cheksum` used with `checksum` `lets` will store new checksum to `.lets` dir and each time you run a command `lets` will check if stored checksum changed from the one from env. +If `checksum.persist` is used with `checksum.files`, `lets` will store new checksum to `.lets` dir and each time you run a command `lets` will check if stored checksum changed from the one from env. -While using `persist_checksum`, `lets` will add new env variable to command env - `LETS_CHECKUM_CHANGED`. +While using `checksum.persist`, `lets` will add new env variable to command env - `LETS_CHECKSUM_CHANGED`. You can learn more about checksum in [Checksum section](config.md#checksum) @@ -112,8 +112,9 @@ commands: build-deps: description: Install project dependencies checksum: - - package.json - persist_checksum: true + files: + - package.json + persist: true cmd: | if [[ ${LETS_CHECKSUM_CHANGED} == true ]]; then npm install @@ -142,8 +143,9 @@ commands: build-deps: description: Install project dependencies checksum: - - package.json - persist_checksum: true + files: + - package.json + persist: true cmd: | if [[ ${LETS_CHECKSUM_CHANGED} == true ]]; then npm install diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 16552c65..4076e06c 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -3,6 +3,11 @@ id: changelog title: Changelog --- +## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X) + +* `[Added]` Add `checksum.files`, `checksum.sh`, and `checksum.persist` command checksum syntax while keeping the old checksum format compatible. +* `[Added]` Add `lets self fix` config migration command with `--dry-run` preview output for deprecated checksum syntax. + ## [0.0.62](https://github.com/lets-cli/lets/releases/tag/v0.0.62) * `[Changed]` Prerelease tags (e.g. `v0.0.62-rc1`) are no longer published to the Homebrew tap `lets` formula or AUR; they are published to a new opt-in `lets-beta` formula instead (`brew install lets-cli/tap/lets-beta`). diff --git a/docs/docs/cli.md b/docs/docs/cli.md index d807d3d6..8719e906 100644 --- a/docs/docs/cli.md +++ b/docs/docs/cli.md @@ -19,3 +19,5 @@ title: CLI options |`-v, --version`|||version for lets| Upgrade the lets binary with `lets self upgrade`. + +Migrate deprecated config syntax with `lets self fix`. Use `lets self fix --dry-run` to print migrated config content before writing files. diff --git a/docs/docs/config.md b/docs/docs/config.md index 58eedcb1..cdfaa72b 100644 --- a/docs/docs/config.md +++ b/docs/docs/config.md @@ -814,11 +814,11 @@ commands: `key: checksum` -`type: array of string | mapping string => array of string` +`type: object` Checksum used for computing file hashes. It is useful when you depend on some files content changes. -In `checksum` you can specify: +In `checksum.files` you can specify: - a list of file names - a list of file regexp patterns (parsed via go `path/filepath.Glob`) @@ -839,10 +839,11 @@ If checksum is a mapping, e.g: commands: build: checksum: - deps: - - package.json - doc: - - Readme.md + files: + deps: + - package.json + doc: + - Readme.md ``` Resulting env will be: @@ -855,6 +856,18 @@ Checksum is calculated with `sha1`. If you specify patterns, `lets` will try to find all matches and will calculate checksum of that files. +You can use `checksum.sh` instead of `checksum.files` when you need to provide the checksum value from a shell command: + +```yaml +commands: + build: + checksum: + sh: git rev-parse HEAD + cmd: docker build -t myrepo/app:${LETS_CHECKSUM} . +``` + +`checksum.files` and `checksum.sh` are mutually exclusive. + Example: ```yaml @@ -862,22 +875,23 @@ shell: bash commands: app-build: checksum: - - requirements-*.txt + files: + - requirements-*.txt cmd: | docker pull myrepo/app:${LETS_CHECKSUM} docker run --rm myrepo/app${LETS_CHECKSUM} python -m app ``` -### `persist_checksum` +### `checksum.persist` -`key: persist_checksum` +`key: checksum.persist` `type: bool` This feature is useful when you want to know that something has changed between two executions of a command. -`persist_checksum` can be used only if `checksum` declared for command. +`checksum.persist` can be used only if `checksum.files` or `checksum.sh` declared for command. If set to `true`, each run all calculated checksums will be stored to disk. @@ -894,12 +908,13 @@ Example: ```yaml commands: build: - persist_checksum: true checksum: - deps: - - package.json - doc: - - Readme.md + persist: true + files: + deps: + - package.json + doc: + - Readme.md ``` Resulting env will be: @@ -912,6 +927,8 @@ Resulting env will be: * `LETS_CHECKSUM_DOC_CHANGED` - is checksum of doc files changed * `LETS_CHECKSUM_CHANGED` - is checksum of all checksums (deps and doc) changed +`checksum` as a direct list/map and command-level `persist_checksum` are deprecated compatibility syntax. Use `lets self fix` to migrate local configs to `checksum.files` and `checksum.persist`. + ### `ref` `key: ref` diff --git a/docs/static/schema.json b/docs/static/schema.json index 0127f7fd..09338714 100644 --- a/docs/static/schema.json +++ b/docs/static/schema.json @@ -132,10 +132,7 @@ "type": "boolean" }, "checksum": { - "type": "array", - "items": { - "type": "string" - } + "$ref": "#/definitions/command_checksum" }, "env": { "$ref": "#/definitions/env" @@ -237,6 +234,61 @@ } } ] + }, + "checksum_files": { + "oneOf": [ + { + "type": "array", + "items": { + "type": "string" + } + }, + { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "type": "string" + } + } + } + ] + }, + "command_checksum": { + "oneOf": [ + { + "$ref": "#/definitions/checksum_files" + }, + { + "type": "object", + "properties": { + "files": { + "$ref": "#/definitions/checksum_files" + }, + "sh": { + "type": "string", + "description": "Shell command that prints checksum value." + }, + "persist": { + "type": "boolean", + "description": "Persist checksum and expose LETS_CHECKSUM_CHANGED variables." + } + }, + "oneOf": [ + { + "required": [ + "files" + ] + }, + { + "required": [ + "sh" + ] + } + ], + "additionalProperties": false + } + ] } }, "additionalProperties": false diff --git a/internal/checksum/checksum.go b/internal/checksum/checksum.go index 851b2ccb..9d1df4fd 100644 --- a/internal/checksum/checksum.go +++ b/internal/checksum/checksum.go @@ -6,8 +6,10 @@ import ( "encoding/hex" "fmt" "os" + "os/exec" "path/filepath" "sort" + "strings" "github.com/lets-cli/lets/internal/set" "github.com/lets-cli/lets/internal/util" @@ -147,6 +149,44 @@ func CalculateChecksumFromSources(workDir string, checksumSources map[string][]s return checksumMap, nil } +func CalculateChecksumFromConfig( + workDir string, + checksumSources map[string][]string, + shell string, + script string, + env map[string]string, +) (map[string]string, error) { + if script != "" { + result, err := CalculateChecksumFromScript(workDir, shell, script, env) + if err != nil { + return nil, err + } + + return map[string]string{DefaultChecksumKey: result}, nil + } + + return CalculateChecksumFromSources(workDir, checksumSources) +} + +func CalculateChecksumFromScript(workDir string, shell string, script string, env map[string]string) (string, error) { + cmd := exec.Command(shell, "-c", script) + cmd.Dir = workDir + + envList := os.Environ() + for key, value := range env { + envList = append(envList, fmt.Sprintf("%s=%s", key, value)) + } + + cmd.Env = envList + + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("can not get output from checksum.sh script: %s: %w", script, err) + } + + return strings.TrimSpace(string(out)), nil +} + func ReadChecksumFromDisk(checksumsDir, cmdName, checksumName string) (string, error) { _, checksumFilePath := getChecksumPath(checksumsDir, cmdName, checksumName) diff --git a/internal/checksum/checksum_test.go b/internal/checksum/checksum_test.go index d7a221ce..66155ca1 100644 --- a/internal/checksum/checksum_test.go +++ b/internal/checksum/checksum_test.go @@ -136,3 +136,16 @@ func TestCalculateChecksumFromListOrMap(t *testing.T) { ) } } + +func TestCalculateChecksumFromScript(t *testing.T) { + tempDir := t.TempDir() + + got, err := CalculateChecksumFromScript(tempDir, "bash", `printf "checksum-value\n"`, map[string]string{}) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if got != "checksum-value" { + t.Fatalf("unexpected checksum: %s", got) + } +} diff --git a/internal/cmd/fix.go b/internal/cmd/fix.go new file mode 100644 index 00000000..a76c8def --- /dev/null +++ b/internal/cmd/fix.go @@ -0,0 +1,36 @@ +package cmd + +import ( + "os" + + "github.com/lets-cli/lets/internal/config/migrate" + "github.com/spf13/cobra" +) + +func initFixCommand() *cobra.Command { + var dryRun bool + + fixCmd := &cobra.Command{ + Use: "fix", + Short: "Apply lets config migrations", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + configName, err := cmd.Root().Flags().GetString("config") + if err != nil { + return err + } + + if configName == "" { + configName = os.Getenv("LETS_CONFIG") + } + + _, err = migrate.Fix(configName, os.Getenv("LETS_CONFIG_DIR"), dryRun, cmd.OutOrStdout()) + + return err + }, + } + + fixCmd.Flags().BoolVar(&dryRun, "dry-run", false, "print migrated config without writing files") + + return fixCmd +} diff --git a/internal/cmd/self.go b/internal/cmd/self.go index 88bd1841..1fada9be 100644 --- a/internal/cmd/self.go +++ b/internal/cmd/self.go @@ -33,6 +33,7 @@ func initSelfCmd( selfCmd.AddCommand(initConfigCommand(openEditor)) selfCmd.AddCommand(initDocCommand(openURL)) + selfCmd.AddCommand(initFixCommand()) selfCmd.AddCommand(initLspCommand(version)) selfCmd.AddCommand(initSkillsCommand()) selfCmd.AddCommand(initUpgradeCommand(version, appSettings)) diff --git a/internal/config/config/checksum.go b/internal/config/config/checksum.go index 4fb72a12..ef81aa0a 100644 --- a/internal/config/config/checksum.go +++ b/internal/config/config/checksum.go @@ -1,18 +1,21 @@ package config import ( + "errors" + "fmt" "maps" "github.com/lets-cli/lets/internal/checksum" + "gopkg.in/yaml.v3" ) -// Checksum type for all checksum uses (env, command.env, command,checksum). -type Checksum map[string][]string +// ChecksumFiles type for file based checksums. +type ChecksumFiles map[string][]string // UnmarshalYAML implements yaml.Unmarshaler interface. -func (c *Checksum) UnmarshalYAML(unmarshal func(any) error) error { +func (c *ChecksumFiles) UnmarshalYAML(unmarshal func(any) error) error { if *c == nil { - *c = make(Checksum) + *c = make(ChecksumFiles) } var patterns []string @@ -31,3 +34,78 @@ func (c *Checksum) UnmarshalYAML(unmarshal func(any) error) error { return nil } + +// Checksum type for env checksum uses. +type Checksum = ChecksumFiles + +type CommandChecksum struct { + Files ChecksumFiles + Sh string + Persist *bool +} + +func (c *CommandChecksum) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.MappingNode { + var files ChecksumFiles + if err := node.Decode(&files); err == nil { + c.Files = files + + return nil + } + + return errors.New("checksum must be a list, map, or object") + } + + var files ChecksumFiles + if !isCommandChecksumObject(node) { + if err := node.Decode(&files); err != nil { + return err + } + + c.Files = files + + return nil + } + + for i := 0; i < len(node.Content); i += 2 { + key := node.Content[i].Value + value := node.Content[i+1] + + switch key { + case "files": + if err := value.Decode(&c.Files); err != nil { + return err + } + case "sh": + if err := value.Decode(&c.Sh); err != nil { + return err + } + case "persist": + var persist bool + if err := value.Decode(&persist); err != nil { + return err + } + + c.Persist = &persist + default: + return fmt.Errorf("checksum.%s is not supported", key) + } + } + + if len(c.Files) > 0 && c.Sh != "" { + return errors.New("checksum must use only one of 'files' or 'sh'") + } + + return nil +} + +func isCommandChecksumObject(node *yaml.Node) bool { + for i := 0; i < len(node.Content); i += 2 { + switch node.Content[i].Value { + case "files", "sh", "persist": + return true + } + } + + return false +} diff --git a/internal/config/config/command.go b/internal/config/config/command.go index 67d27bb6..7c15623e 100644 --- a/internal/config/config/command.go +++ b/internal/config/config/command.go @@ -29,13 +29,15 @@ type Command struct { // env files from command EnvFiles *EnvFiles // store docopts from options directive - Docopts string - SkipDocopts bool // default false - Options map[string]string - CliOptions map[string]string - Depends *Deps - ChecksumMap map[string]string - PersistChecksum bool + Docopts string + SkipDocopts bool // default false + Options map[string]string + CliOptions map[string]string + Depends *Deps + ChecksumMap map[string]string + PersistChecksum bool + DeprecatedPersistChecksum bool + ChecksumSh string // args from 'lets run --debug' will become [--debug] Args []string @@ -72,8 +74,8 @@ func (c *Command) UnmarshalYAML(unmarshal func(any) error) error { WorkDir string `yaml:"work_dir"` After string Ref string - Checksum *Checksum - PersistChecksum bool `yaml:"persist_checksum"` + Checksum *CommandChecksum + PersistChecksum *bool `yaml:"persist_checksum"` } if err := unmarshal(&cmd); err != nil { @@ -117,14 +119,36 @@ func (c *Command) UnmarshalYAML(unmarshal func(any) error) error { c.After = cmd.After // TODO: checksum must be refactored if cmd.Checksum != nil { - c.ChecksumSources = *cmd.Checksum + c.ChecksumSources = cmd.Checksum.Files + + c.ChecksumSh = cmd.Checksum.Sh + if cmd.Checksum.Persist != nil { + c.PersistChecksum = *cmd.Checksum.Persist + } } - c.PersistChecksum = cmd.PersistChecksum - if len(c.ChecksumSources) == 0 && c.PersistChecksum { + if cmd.PersistChecksum != nil { + c.DeprecatedPersistChecksum = true + + if cmd.Checksum != nil && cmd.Checksum.Persist != nil && *cmd.PersistChecksum != *cmd.Checksum.Persist { + return errors.New("'persist_checksum' conflicts with 'checksum.persist'") + } + + c.PersistChecksum = *cmd.PersistChecksum + } + + if c.ChecksumSh != "" && len(c.ChecksumSources) > 0 { + return errors.New("checksum must use only one of 'files' or 'sh'") + } + + if c.PersistChecksum && cmd.Checksum == nil { return errors.New("'persist_checksum' must be used with 'checksum'") } + if c.PersistChecksum && len(c.ChecksumSources) == 0 && c.ChecksumSh == "" { + return errors.New("'persist_checksum' or 'checksum.persist' must be used with 'checksum.files' or 'checksum.sh'") + } + if cmd.Ref != "" { var refArgs struct { Args *refArgs @@ -173,25 +197,27 @@ func (c *Command) GetEnv(cfg Config, builtinEnv map[string]string) (map[string]s func (c *Command) Clone() *Command { cmd := &Command{ - Name: c.Name, - GroupName: c.GroupName, - Cmds: c.Cmds.Clone(), - After: c.After, - Shell: c.Shell, - WorkDir: c.WorkDir, - Description: c.Description, - Env: c.Env.Clone(), - EnvFiles: c.EnvFiles.Clone(), - Docopts: c.Docopts, - SkipDocopts: c.SkipDocopts, - Options: cloneMap(c.Options), - CliOptions: cloneMap(c.CliOptions), - Depends: c.Depends.Clone(), - ChecksumMap: cloneMap(c.ChecksumMap), - PersistChecksum: c.PersistChecksum, - ChecksumSources: cloneMapSlice(c.ChecksumSources), - persistedChecksums: cloneMap(c.persistedChecksums), - Args: cloneSlice(c.Args), + Name: c.Name, + GroupName: c.GroupName, + Cmds: c.Cmds.Clone(), + After: c.After, + Shell: c.Shell, + WorkDir: c.WorkDir, + Description: c.Description, + Env: c.Env.Clone(), + EnvFiles: c.EnvFiles.Clone(), + Docopts: c.Docopts, + SkipDocopts: c.SkipDocopts, + Options: cloneMap(c.Options), + CliOptions: cloneMap(c.CliOptions), + Depends: c.Depends.Clone(), + ChecksumMap: cloneMap(c.ChecksumMap), + PersistChecksum: c.PersistChecksum, + DeprecatedPersistChecksum: c.DeprecatedPersistChecksum, + ChecksumSh: c.ChecksumSh, + ChecksumSources: cloneMapSlice(c.ChecksumSources), + persistedChecksums: cloneMap(c.persistedChecksums), + Args: cloneSlice(c.Args), } return cmd @@ -227,12 +253,12 @@ func (c *Command) Help() string { return strings.TrimSuffix(buf.String(), "\n") } -func (c *Command) ChecksumCalculator(workDir string) error { - if len(c.ChecksumSources) == 0 { +func (c *Command) ChecksumCalculator(workDir string, shell string, env map[string]string) error { + if len(c.ChecksumSources) == 0 && c.ChecksumSh == "" { return nil } - checksumMap, err := checksum.CalculateChecksumFromSources(workDir, c.ChecksumSources) + checksumMap, err := checksum.CalculateChecksumFromConfig(workDir, c.ChecksumSources, shell, c.ChecksumSh, env) if err != nil { return err } diff --git a/internal/config/config/command_test.go b/internal/config/config/command_test.go index f02865e7..d23a8b0e 100644 --- a/internal/config/config/command_test.go +++ b/internal/config/config/command_test.go @@ -4,6 +4,7 @@ import ( "bytes" "testing" + "github.com/lets-cli/lets/internal/checksum" "github.com/lithammer/dedent" "gopkg.in/yaml.v3" ) @@ -44,3 +45,78 @@ func TestParseCommand(t *testing.T) { } }) } + +func TestParseCommandChecksum(t *testing.T) { + t.Run("old list syntax", func(t *testing.T) { + text := dedent.Dedent(` + checksum: + - foo.txt + persist_checksum: true + cmd: echo ok + `) + command := CommandFixture(t, text) + + if !command.PersistChecksum { + t.Fatal("expected persisted checksum") + } + + got := command.ChecksumSources[checksum.DefaultChecksumKey] + if len(got) != 1 || got[0] != "foo.txt" { + t.Fatalf("unexpected checksum sources: %v", got) + } + }) + + t.Run("new files syntax", func(t *testing.T) { + text := dedent.Dedent(` + checksum: + files: + source: + - foo.txt + persist: true + cmd: echo ok + `) + command := CommandFixture(t, text) + + if !command.PersistChecksum { + t.Fatal("expected persisted checksum") + } + + got := command.ChecksumSources["source"] + if len(got) != 1 || got[0] != "foo.txt" { + t.Fatalf("unexpected checksum sources: %v", got) + } + }) + + t.Run("new files list syntax", func(t *testing.T) { + text := dedent.Dedent(` + checksum: + files: + - foo.txt + cmd: echo ok + `) + command := CommandFixture(t, text) + + got := command.ChecksumSources[checksum.DefaultChecksumKey] + if len(got) != 1 || got[0] != "foo.txt" { + t.Fatalf("unexpected checksum sources: %v", got) + } + }) + + t.Run("new sh syntax", func(t *testing.T) { + text := dedent.Dedent(` + checksum: + sh: echo 1234 + persist: true + cmd: echo ok + `) + command := CommandFixture(t, text) + + if command.ChecksumSh != "echo 1234" { + t.Fatalf("unexpected checksum sh: %s", command.ChecksumSh) + } + + if !command.PersistChecksum { + t.Fatal("expected persisted checksum") + } + }) +} diff --git a/internal/config/migrate/checksum.go b/internal/config/migrate/checksum.go new file mode 100644 index 00000000..7f0f50d3 --- /dev/null +++ b/internal/config/migrate/checksum.go @@ -0,0 +1,126 @@ +package migrate + +import "gopkg.in/yaml.v3" + +type ChecksumMigration struct{} + +func (ChecksumMigration) Name() string { + return "checksum" +} + +func (ChecksumMigration) Apply(root *yaml.Node) (bool, error) { + commands := mappingValue(document(root), "commands") + if commands == nil || commands.Kind != yaml.MappingNode { + return false, nil + } + + changed := false + + for i := 0; i < len(commands.Content); i += 2 { + command := commands.Content[i+1] + if command.Kind != yaml.MappingNode { + continue + } + + commandChanged, err := migrateCommandChecksum(command) + if err != nil { + return false, err + } + + changed = changed || commandChanged + } + + return changed, nil +} + +func migrateCommandChecksum(command *yaml.Node) (bool, error) { + checksumIdx := mappingIndex(command, "checksum") + persistIdx := mappingIndex(command, "persist_checksum") + + if checksumIdx == -1 { + return false, nil + } + + checksumNode := command.Content[checksumIdx+1] + changed := false + + if isNewChecksumNode(checksumNode) { + if persistIdx != -1 && mappingIndex(checksumNode, "persist") == -1 { + appendMapping(checksumNode, scalar("persist"), cloneNode(command.Content[persistIdx+1])) + + changed = true + } + } else if checksumNode.Kind == yaml.SequenceNode || checksumNode.Kind == yaml.MappingNode { + filesNode := cloneNode(checksumNode) + checksumNode.Kind = yaml.MappingNode + checksumNode.Tag = "!!map" + checksumNode.Content = []*yaml.Node{scalar("files"), filesNode} + + if persistIdx != -1 { + appendMapping(checksumNode, scalar("persist"), cloneNode(command.Content[persistIdx+1])) + } + + changed = true + } + + if persistIdx != -1 { + removeMappingIndex(command, persistIdx) + + changed = true + } + + return changed, nil +} + +func isNewChecksumNode(node *yaml.Node) bool { + if node == nil || node.Kind != yaml.MappingNode { + return false + } + + return mappingIndex(node, "files") != -1 || mappingIndex(node, "sh") != -1 || mappingIndex(node, "persist") != -1 +} + +func mappingIndex(node *yaml.Node, key string) int { + if node == nil || node.Kind != yaml.MappingNode { + return -1 + } + + for i := 0; i < len(node.Content); i += 2 { + if node.Content[i].Value == key { + return i + } + } + + return -1 +} + +func appendMapping(node *yaml.Node, key *yaml.Node, value *yaml.Node) { + node.Content = append(node.Content, key, value) +} + +func removeMappingIndex(node *yaml.Node, idx int) { + node.Content = append(node.Content[:idx], node.Content[idx+2:]...) +} + +func scalar(value string) *yaml.Node { + return &yaml.Node{ + Kind: yaml.ScalarNode, + Tag: "!!str", + Value: value, + } +} + +func cloneNode(node *yaml.Node) *yaml.Node { + if node == nil { + return nil + } + + clone := *node + + clone.Content = make([]*yaml.Node, len(node.Content)) + for idx, child := range node.Content { + clone.Content[idx] = cloneNode(child) + } + + return &clone +} diff --git a/internal/config/migrate/checksum_test.go b/internal/config/migrate/checksum_test.go new file mode 100644 index 00000000..8e1275e3 --- /dev/null +++ b/internal/config/migrate/checksum_test.go @@ -0,0 +1,116 @@ +package migrate + +import ( + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +func applyChecksumMigration(t *testing.T, input string) string { + t.Helper() + + root := &yaml.Node{} + if err := yaml.Unmarshal([]byte(input), root); err != nil { + t.Fatalf("decode yaml: %s", err) + } + + changed, err := ChecksumMigration{}.Apply(root) + if err != nil { + t.Fatalf("apply migration: %s", err) + } + + if !changed { + t.Fatal("expected migration to change config") + } + + encoded, err := encodeYAML(root) + if err != nil { + t.Fatalf("encode yaml: %s", err) + } + + return string(encoded) +} + +func TestChecksumMigrationListWithPersistChecksum(t *testing.T) { + got := applyChecksumMigration(t, ` +shell: bash +commands: + build: + persist_checksum: true + checksum: + - go.mod + cmd: go build +`) + + for _, want := range []string{ + "checksum:\n files:\n - go.mod\n persist: true", + "cmd: go build", + } { + if !strings.Contains(got, want) { + t.Fatalf("expected migrated config to contain %q:\n%s", want, got) + } + } + + if strings.Contains(got, "persist_checksum") { + t.Fatalf("expected persist_checksum to be removed:\n%s", got) + } +} + +func TestChecksumMigrationMapWithPersistChecksum(t *testing.T) { + got := applyChecksumMigration(t, ` +shell: bash +commands: + build: + persist_checksum: true + checksum: + deps: + - go.mod + cmd: go build +`) + + want := "checksum:\n files:\n deps:\n - go.mod\n persist: true" + if !strings.Contains(got, want) { + t.Fatalf("expected migrated config to contain %q:\n%s", want, got) + } +} + +func TestChecksumMigrationMovesPersistIntoNewChecksum(t *testing.T) { + got := applyChecksumMigration(t, ` +shell: bash +commands: + build: + persist_checksum: true + checksum: + sh: git rev-parse HEAD + cmd: go build +`) + + want := "checksum:\n sh: git rev-parse HEAD\n persist: true" + if !strings.Contains(got, want) { + t.Fatalf("expected migrated config to contain %q:\n%s", want, got) + } +} + +func TestChecksumMigrationKeepsBlankLineBetweenCommands(t *testing.T) { + got := applyChecksumMigration(t, ` +shell: bash +commands: + build: + persist_checksum: true + checksum: + - go.mod + cmd: go build + + test: + persist_checksum: true + checksum: + - go.sum + cmd: go test ./... +`) + + want := " cmd: go build\n\n test:" + if !strings.Contains(got, want) { + t.Fatalf("expected migrated config to keep command spacing %q:\n%s", want, got) + } +} diff --git a/internal/config/migrate/migrate.go b/internal/config/migrate/migrate.go new file mode 100644 index 00000000..11b82495 --- /dev/null +++ b/internal/config/migrate/migrate.go @@ -0,0 +1,298 @@ +package migrate + +import ( + "bytes" + "fmt" + "io" + "os" + "slices" + "strings" + + "github.com/lets-cli/lets/internal/config" + configpath "github.com/lets-cli/lets/internal/config/path" + "gopkg.in/yaml.v3" +) + +type Migration interface { + Name() string + Apply(root *yaml.Node) (bool, error) +} + +type Result struct { + ChangedFiles []string + RemoteMixins []string + Applied []string + Changed bool + DryRun bool + Previews []string +} + +func DefaultMigrations() []Migration { + return []Migration{ + ChecksumMigration{}, + } +} + +func Fix(configName string, configDir string, dryRun bool, out io.Writer) (Result, error) { + pathInfo, err := config.FindConfig(configName, configDir) + if err != nil { + return Result{}, err + } + + paths, remoteMixins, err := collectConfigPaths(pathInfo.AbsPath, pathInfo.WorkDir) + if err != nil { + return Result{}, err + } + + result := Result{ + DryRun: dryRun, + RemoteMixins: remoteMixins, + } + + for _, path := range paths { + fileChanged, applied, preview, err := fixFile(path, dryRun, DefaultMigrations()) + if err != nil { + return Result{}, err + } + + if !fileChanged { + continue + } + + result.Changed = true + result.ChangedFiles = append(result.ChangedFiles, path) + + result.Applied = append(result.Applied, applied...) + if preview != "" { + result.Previews = append(result.Previews, preview) + } + } + + writeResult(out, result) + + return result, nil +} + +func fixFile(path string, dryRun bool, migrations []Migration) (bool, []string, string, error) { + original, err := os.ReadFile(path) + if err != nil { + return false, nil, "", fmt.Errorf("can not read config %s: %w", path, err) + } + + root, err := decodeYAML(original) + if err != nil { + return false, nil, "", fmt.Errorf("can not parse config %s: %w", path, err) + } + + applied := []string{} + + for _, migration := range migrations { + changed, err := migration.Apply(root) + if err != nil { + return false, nil, "", fmt.Errorf("can not apply migration %s to %s: %w", migration.Name(), path, err) + } + + if changed { + applied = append(applied, migration.Name()) + } + } + + if len(applied) == 0 { + return false, nil, "", nil + } + + updated, err := encodeYAML(root) + if err != nil { + return false, nil, "", fmt.Errorf("can not render config %s: %w", path, err) + } + + if bytes.Equal(original, updated) { + return false, nil, "", nil + } + + preview := "" + + if !dryRun { + if err := os.WriteFile(path, updated, 0o644); err != nil { + return false, nil, "", fmt.Errorf("can not write config %s: %w", path, err) + } + } else { + preview = string(updated) + } + + return true, applied, preview, nil +} + +func decodeYAML(data []byte) (*yaml.Node, error) { + root := &yaml.Node{} + if err := yaml.Unmarshal(data, root); err != nil { + return nil, err + } + + return root, nil +} + +func encodeYAML(root *yaml.Node) ([]byte, error) { + var buf bytes.Buffer + + encoder := yaml.NewEncoder(&buf) + encoder.SetIndent(2) + + if err := encoder.Encode(root); err != nil { + return nil, err + } + + if err := encoder.Close(); err != nil { + return nil, err + } + + return formatCommandSpacing(buf.Bytes()), nil +} + +func formatCommandSpacing(data []byte) []byte { + lines := strings.Split(strings.TrimSuffix(string(data), "\n"), "\n") + if len(lines) == 0 { + return data + } + + formatted := make([]string, 0, len(lines)) + inCommands := false + commandSeen := false + + for _, line := range lines { + if line == "commands:" { + inCommands = true + commandSeen = false + formatted = append(formatted, line) + + continue + } + + if inCommands && line != "" && !strings.HasPrefix(line, " ") { + inCommands = false + } + + if inCommands && isCommandEntryLine(line) { + if commandSeen && len(formatted) > 0 && formatted[len(formatted)-1] != "" { + formatted = append(formatted, "") + } + + commandSeen = true + } + + formatted = append(formatted, line) + } + + return []byte(strings.Join(formatted, "\n") + "\n") +} + +func isCommandEntryLine(line string) bool { + if !strings.HasPrefix(line, " ") || strings.HasPrefix(line, " ") { + return false + } + + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "#") || strings.HasPrefix(trimmed, "-") { + return false + } + + return strings.Contains(trimmed, ":") +} + +func collectConfigPaths(rootPath string, workDir string) ([]string, []string, error) { + paths := []string{rootPath} + seen := map[string]struct{}{rootPath: {}} + + data, err := os.ReadFile(rootPath) + if err != nil { + return nil, nil, err + } + + root, err := decodeYAML(data) + if err != nil { + return nil, nil, err + } + + mixins := mappingValue(document(root), "mixins") + if mixins == nil || mixins.Kind != yaml.SequenceNode { + return paths, nil, nil + } + + remoteMixins := []string{} + + for _, mixin := range mixins.Content { + if mixin.Kind == yaml.ScalarNode { + ignored := strings.HasPrefix(mixin.Value, "-") + mixinPath := strings.TrimPrefix(mixin.Value, "-") + + absPath, err := configpath.GetFullConfigPath(mixinPath, workDir) + if err != nil { + if ignored { + continue + } + + return nil, nil, err + } + + if _, ok := seen[absPath]; !ok { + seen[absPath] = struct{}{} + paths = append(paths, absPath) + } + + continue + } + + if mixin.Kind != yaml.MappingNode { + continue + } + + if url := mappingValue(mixin, "url"); url != nil && url.Value != "" { + remoteMixins = append(remoteMixins, url.Value) + } + } + + return paths, remoteMixins, nil +} + +func document(root *yaml.Node) *yaml.Node { + if root.Kind == yaml.DocumentNode && len(root.Content) > 0 { + return root.Content[0] + } + + return root +} + +func mappingValue(node *yaml.Node, key string) *yaml.Node { + if node == nil || node.Kind != yaml.MappingNode { + return nil + } + + for i := 0; i < len(node.Content); i += 2 { + if node.Content[i].Value == key { + return node.Content[i+1] + } + } + + return nil +} + +func writeResult(out io.Writer, result Result) { + for _, preview := range result.Previews { + fmt.Fprint(out, preview) + } + + if result.Changed && !result.DryRun { + applied := slices.Compact(slices.Sorted(slices.Values(result.Applied))) + for _, migration := range applied { + fmt.Fprintf(out, "Migration '%s' applied successfully\n", migration) + } + } + + for _, remote := range result.RemoteMixins { + fmt.Fprintf(out, "remote mixin not updated: %s\n", remote) + } + + if !result.Changed && len(result.RemoteMixins) == 0 { + fmt.Fprintln(out, "No config migrations needed.") + } +} diff --git a/internal/executor/executor.go b/internal/executor/executor.go index f674b59f..535a8270 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "maps" "os/exec" "github.com/lets-cli/lets/internal/checksum" @@ -164,6 +165,10 @@ func formatOptsUsageError(err error, opts docopt.Opts, cmdName string, rawOption func (e *Executor) initCmd(ctx *Context) error { cmd := ctx.command + if cmd.DeprecatedPersistChecksum { + ctx.logger.Warn("command uses deprecated 'persist_checksum'; use 'checksum.persist' instead") + } + if !cmd.SkipDocopts { ctx.logger.Debug("parse docopt: %s, args: %s", cmd.Docopts, cmd.Args) @@ -180,8 +185,21 @@ func (e *Executor) initCmd(ctx *Context) error { cmd.CliOptions = docopt.OptsToLetsCli(opts) } + checksumShell := e.cfg.Shell + if cmd.Shell != "" { + checksumShell = cmd.Shell + } + + checksumWorkDir := e.cfg.WorkDir + if cmd.WorkDir != "" { + checksumWorkDir = cmd.WorkDir + } + + checksumEnv := e.cfg.CommandBuiltinEnv(cmd, checksumShell, checksumWorkDir) + maps.Copy(checksumEnv, e.cfg.GetEnv()) + // calculate checksum if needed - if err := cmd.ChecksumCalculator(e.cfg.WorkDir); err != nil { + if err := cmd.ChecksumCalculator(e.cfg.WorkDir, checksumShell, checksumEnv); err != nil { return fmt.Errorf("failed to calculate checksum for command '%s': %w", cmd.Name, err) } diff --git a/internal/logging/formatter.go b/internal/logging/formatter.go index fc995437..2d0a5ec2 100644 --- a/internal/logging/formatter.go +++ b/internal/logging/formatter.go @@ -108,6 +108,10 @@ func formatPrefix(entry *log.Entry) string { return color.BlueString("lets:") } + if entry.Level == log.WarnLevel { + return color.YellowString("lets:") + } + return "lets:" } @@ -116,6 +120,10 @@ func formatMessage(entry *log.Entry) string { return color.BlueString(entry.Message) } + if entry.Level == log.WarnLevel { + return color.YellowString(entry.Message) + } + return entry.Message } diff --git a/internal/logging/log.go b/internal/logging/log.go index 7304db4d..1e57770e 100644 --- a/internal/logging/log.go +++ b/internal/logging/log.go @@ -87,6 +87,14 @@ func (l *ExecLogger) Info(format string, a ...any) { l.log.Logf(log.InfoLevel, format, a...) } +func (l *ExecLogger) Warn(format string, a ...any) { + if l.prefix != "" { + format = fmt.Sprintf("%s %s", l.prefix, format) + } + + l.log.Logf(log.WarnLevel, format, a...) +} + func (l *ExecLogger) Debug(format string, a ...any) { if l.prefix != "" { format = fmt.Sprintf("%s %s", l.prefix, format) diff --git a/internal/logging/log_test.go b/internal/logging/log_test.go index 0ef85c2e..21e45456 100644 --- a/internal/logging/log_test.go +++ b/internal/logging/log_test.go @@ -69,6 +69,23 @@ func TestFormatterColorsDebugMessages(t *testing.T) { } } +func TestFormatterColorsWarnMessages(t *testing.T) { + setNoColorForTest(t, false) + + line, err := (&Formatter{}).Format(&log.Entry{ + Level: log.WarnLevel, + Message: "warn message", + }) + if err != nil { + t.Fatalf("Format() error = %v", err) + } + + expected := color.YellowString("lets:") + " " + color.YellowString("warn message") + "\n" + if string(line) != expected { + t.Fatalf("unexpected warn line: %q", string(line)) + } +} + func TestFormatterFormatsLevelsAndFields(t *testing.T) { setNoColorForTest(t, true) @@ -131,7 +148,7 @@ func TestFormatterFormatsLevelsAndFields(t *testing.T) { } if strings.Contains(line, "\x1b[") { - t.Fatalf("expected non-colorized output for non-debug levels, got: %q", line) + t.Fatalf("expected non-colorized output when color is disabled, got: %q", line) } if len(tt.fields) == 0 { diff --git a/lets.yaml b/lets.yaml index 859e77de..62be3a46 100644 --- a/lets.yaml +++ b/lets.yaml @@ -1,13 +1,10 @@ shell: bash - mixins: - lets.build.yaml - -lets.my.yaml - env: CURRENT_UID: sh: echo "`id -u`:`id -g`" - commands: release: description: | @@ -50,6 +47,10 @@ commands: test-bats: description: Run bats tests depends: [build-lets-image] + checksum: + files: + - go.sum + persist: true options: | Usage: lets test-bats [] [--opts=] Example: @@ -125,11 +126,11 @@ commands: cmd: | VERSION=$(git describe) BIN=${LETSOPT_BIN:-lets} - + go build \ -ldflags="-X main.Version=${VERSION:1}-dev -X main.BuildDate=$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ -o ${BIN} ./cmd/lets - + success=$? if [[ $success -eq 0 ]]; then version=$(./${BIN} --version) diff --git a/tests/command_checksum.bats b/tests/command_checksum.bats index c55bca3a..ed7460ec 100644 --- a/tests/command_checksum.bats +++ b/tests/command_checksum.bats @@ -24,6 +24,18 @@ CHECKSUM_FROM_FOO_AND_BAR_CHECKSUMS="b778d48759ad4e6e9a755bd595d23eeaa2f7ff65" assert_line --index 0 ${ALL_CHECKSUM} } +@test "command_checksum: should calculate checksum using new files syntax" { + run lets as-new-list-of-files + assert_success + assert_line --index 0 ${ALL_CHECKSUM} +} + +@test "command_checksum: should calculate checksum using sh" { + run lets as-new-sh + assert_success + assert_line --index 0 custom-checksum +} + @test "command_checksum: should calculate checksum as map of list of files" { run lets as-map-of-list-of-files assert_success diff --git a/tests/command_checksum/lets.yaml b/tests/command_checksum/lets.yaml index 511497e1..df72303a 100644 --- a/tests/command_checksum/lets.yaml +++ b/tests/command_checksum/lets.yaml @@ -16,6 +16,21 @@ commands: - bar_1.txt cmd: echo "${LETS_CHECKSUM}" + as-new-list-of-files: + description: Test checksum + checksum: + files: + - foo_1.txt + - foo_2.txt + - bar_1.txt + cmd: echo "${LETS_CHECKSUM}" + + as-new-sh: + description: Test checksum + checksum: + sh: printf custom-checksum + cmd: echo "${LETS_CHECKSUM}" + as-map-of-list-of-files: description: Test checksum checksum: @@ -49,4 +64,4 @@ commands: - bar_1.txt cmd: | echo LETS_CHECKSUM_ALL="${LETS_CHECKSUM_ALL}" - echo LETS_CHECKSUM="${LETS_CHECKSUM}" \ No newline at end of file + echo LETS_CHECKSUM="${LETS_CHECKSUM}" diff --git a/tests/command_persist_checksum.bats b/tests/command_persist_checksum.bats index fc0074e2..a62cf866 100644 --- a/tests/command_persist_checksum.bats +++ b/tests/command_persist_checksum.bats @@ -33,8 +33,9 @@ TEMP_FILE=foo_test.txt # 3. check checksum persisted assert_success - assert_line --index 0 "LETS_CHECKSUM=${FIRST_CHECKSUM}" - assert_line --index 1 "LETS_CHECKSUM_CHANGED=true" + assert_line --index 0 --partial "deprecated 'persist_checksum'" + assert_line --index 1 "LETS_CHECKSUM=${FIRST_CHECKSUM}" + assert_line --index 2 "LETS_CHECKSUM_CHANGED=true" # it creates .lets [[ -d .lets ]] @@ -50,8 +51,9 @@ TEMP_FILE=foo_test.txt printf "second run: %s\n" "${lines[@]}" assert_success - assert_line --index 0 "LETS_CHECKSUM=${FIRST_CHECKSUM}" - assert_line --index 1 "LETS_CHECKSUM_CHANGED=false" + assert_line --index 0 --partial "deprecated 'persist_checksum'" + assert_line --index 1 "LETS_CHECKSUM=${FIRST_CHECKSUM}" + assert_line --index 2 "LETS_CHECKSUM_CHANGED=false" # third run, there is stored checksum and we creating new file. checksum must be changed now @@ -65,8 +67,9 @@ TEMP_FILE=foo_test.txt printf "third run: %s\n" "${lines[@]}" assert_success - assert_line --index 0 "LETS_CHECKSUM=${CHANGED_CHECKSUM}" - assert_line --index 1 "LETS_CHECKSUM_CHANGED=true" + assert_line --index 0 --partial "deprecated 'persist_checksum'" + assert_line --index 1 "LETS_CHECKSUM=${CHANGED_CHECKSUM}" + assert_line --index 2 "LETS_CHECKSUM_CHANGED=true" } @test "command_persist_checksum: should persist checksum for cmd-as-map" { @@ -124,12 +127,31 @@ TEMP_FILE=foo_test.txt assert_line --index 1 "2 LETS_CHECKSUM_CHANGED=true" } +@test "command_persist_checksum: should check if checksum has changed using new syntax" { + export CMD_NAME=persist-checksum-new + + run lets ${CMD_NAME} + + assert_success + assert_line --index 0 "LETS_CHECKSUM=${FIRST_CHECKSUM}" + assert_line --index 1 "LETS_CHECKSUM_CHANGED=true" + + [[ -f .lets/checksums/${CMD_NAME}/lets_default_checksum ]] + + run lets ${CMD_NAME} + + assert_success + assert_line --index 0 "LETS_CHECKSUM=${FIRST_CHECKSUM}" + assert_line --index 1 "LETS_CHECKSUM_CHANGED=false" +} + @test "command_persist_checksum: should persist checksum only if exit code = 0" { run lets with-error-code-1 [[ $status = 1 ]] - assert_line --index 0 "LETS_CHECKSUM=${FIRST_CHECKSUM}" - assert_line --index 1 "LETS_CHECKSUM_CHANGED=true" + assert_line --index 0 --partial "deprecated 'persist_checksum'" + assert_line --index 1 "LETS_CHECKSUM=${FIRST_CHECKSUM}" + assert_line --index 2 "LETS_CHECKSUM_CHANGED=true" [[ -d .lets ]] [[ ! -d .lets/checksums ]] diff --git a/tests/command_persist_checksum/lets.yaml b/tests/command_persist_checksum/lets.yaml index c07f8f32..98d13bec 100644 --- a/tests/command_persist_checksum/lets.yaml +++ b/tests/command_persist_checksum/lets.yaml @@ -10,6 +10,16 @@ commands: echo LETS_CHECKSUM=${LETS_CHECKSUM} echo LETS_CHECKSUM_CHANGED=${LETS_CHECKSUM_CHANGED} + persist-checksum-new: + description: Test checksum + checksum: + files: + - foo_*.txt + persist: true + cmd: | + echo LETS_CHECKSUM=${LETS_CHECKSUM} + echo LETS_CHECKSUM_CHANGED=${LETS_CHECKSUM_CHANGED} + with-error-code-1: persist_checksum: true checksum: @@ -26,4 +36,4 @@ commands: - foo_*.txt cmd: checksum: echo 1 LETS_CHECKSUM=${LETS_CHECKSUM} - checksum_changed: echo 2 LETS_CHECKSUM_CHANGED=${LETS_CHECKSUM_CHANGED} \ No newline at end of file + checksum_changed: echo 2 LETS_CHECKSUM_CHANGED=${LETS_CHECKSUM_CHANGED} diff --git a/tests/self_fix.bats b/tests/self_fix.bats new file mode 100644 index 00000000..1028bf62 --- /dev/null +++ b/tests/self_fix.bats @@ -0,0 +1,43 @@ +load test_helpers + +setup() { + load "${BATS_UTILS_PATH}/bats-support/load.bash" + load "${BATS_UTILS_PATH}/bats-assert/load.bash" + cd ./tests/self_fix + cp lets.old.yaml lets.yaml + cp mixin.old.yaml mixin.yaml +} + +teardown() { + rm -f lets.yaml mixin.yaml + cleanup +} + +@test "self_fix: dry-run prints migrated config without changing configs" { + run lets self fix --dry-run + + assert_success + assert_line --partial "files:" + assert_line --partial "persist: true" + assert_line --partial "remote mixin not updated: https://example.com/lets.mixin.yaml" + + run grep -q "persist_checksum: true" lets.yaml + assert_success +} + +@test "self_fix: migrates root config and local mixins" { + run lets self fix + + assert_success + assert_line --partial "Migration 'checksum' applied successfully" + assert_line --partial "remote mixin not updated: https://example.com/lets.mixin.yaml" + + run grep -q "persist_checksum" lets.yaml + [[ "$status" = 1 ]] + + run grep -q "persist: true" lets.yaml + assert_success + + run grep -q "files:" mixin.yaml + assert_success +} diff --git a/tests/self_fix/lets.old.yaml b/tests/self_fix/lets.old.yaml new file mode 100644 index 00000000..26a48d22 --- /dev/null +++ b/tests/self_fix/lets.old.yaml @@ -0,0 +1,12 @@ +shell: bash + +mixins: + - mixin.yaml + - url: https://example.com/lets.mixin.yaml + +commands: + build: + persist_checksum: true + checksum: + - go.mod + cmd: echo build diff --git a/tests/self_fix/mixin.old.yaml b/tests/self_fix/mixin.old.yaml new file mode 100644 index 00000000..11e40e35 --- /dev/null +++ b/tests/self_fix/mixin.old.yaml @@ -0,0 +1,7 @@ +commands: + mixin-build: + persist_checksum: true + checksum: + deps: + - go.sum + cmd: echo mixin From 8fa2f8d16227333ea46b6463c4294579a43b97f7 Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Wed, 29 Jul 2026 10:09:52 +0300 Subject: [PATCH 2/7] Fix migration lint spacing --- internal/config/migrate/migrate.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/config/migrate/migrate.go b/internal/config/migrate/migrate.go index 11b82495..51aad469 100644 --- a/internal/config/migrate/migrate.go +++ b/internal/config/migrate/migrate.go @@ -163,6 +163,7 @@ func formatCommandSpacing(data []byte) []byte { if line == "commands:" { inCommands = true commandSeen = false + formatted = append(formatted, line) continue From cc5295611fb8648ed688a809856aa00a3042e323 Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Wed, 29 Jul 2026 10:22:45 +0300 Subject: [PATCH 3/7] Keep CI lets config bootstrap compatible CI installs the latest released lets before running project commands. Keep the repo test-bats command on checksum syntax that the released binary can parse while this branch adds the new checksum.persist syntax. --- lets.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lets.yaml b/lets.yaml index 62be3a46..b709b775 100644 --- a/lets.yaml +++ b/lets.yaml @@ -47,10 +47,9 @@ commands: test-bats: description: Run bats tests depends: [build-lets-image] + persist_checksum: true checksum: - files: - - go.sum - persist: true + - go.sum options: | Usage: lets test-bats [] [--opts=] Example: From 98594862fc8cbeb9298ff09b9143d75a06be3dd3 Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Wed, 29 Jul 2026 10:59:41 +0300 Subject: [PATCH 4/7] Fix checksum execution review issues Document checksum.sh as an intentional Project config execution boundary and suppress the gosec audit at that call site. Also pass the effective command work_dir into checksum calculation while preserving Project config workdir fallback for commands without an explicit work_dir. --- docs/docs/changelog.md | 1 + internal/checksum/checksum.go | 4 +++- internal/config/config/command.go | 11 +++++---- internal/executor/execute_test.go | 37 +++++++++++++++++++++++++++++++ internal/executor/executor.go | 2 +- 5 files changed, 49 insertions(+), 6 deletions(-) diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 4076e06c..9e53bd70 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -7,6 +7,7 @@ title: Changelog * `[Added]` Add `checksum.files`, `checksum.sh`, and `checksum.persist` command checksum syntax while keeping the old checksum format compatible. * `[Added]` Add `lets self fix` config migration command with `--dry-run` preview output for deprecated checksum syntax. +* `[Fixed]` Make checksum calculation respect command-level `work_dir` overrides. ## [0.0.62](https://github.com/lets-cli/lets/releases/tag/v0.0.62) diff --git a/internal/checksum/checksum.go b/internal/checksum/checksum.go index 9d1df4fd..3e2bfbd1 100644 --- a/internal/checksum/checksum.go +++ b/internal/checksum/checksum.go @@ -169,7 +169,9 @@ func CalculateChecksumFromConfig( } func CalculateChecksumFromScript(workDir string, shell string, script string, env map[string]string) (string, error) { - cmd := exec.Command(shell, "-c", script) + // shell and script come from developer-authored Project config. checksum.sh + // intentionally shares the same executable trust boundary as command scripts. + cmd := exec.Command(shell, "-c", script) //nolint:gosec cmd.Dir = workDir envList := os.Environ() diff --git a/internal/config/config/command.go b/internal/config/config/command.go index 7c15623e..e329a435 100644 --- a/internal/config/config/command.go +++ b/internal/config/config/command.go @@ -110,12 +110,15 @@ func (c *Command) UnmarshalYAML(unmarshal func(any) error) error { c.Depends = cmd.Depends - workDir, err := filepath.Abs(cmd.WorkDir) - if err != nil { - return err + if cmd.WorkDir != "" { + workDir, err := filepath.Abs(cmd.WorkDir) + if err != nil { + return err + } + + c.WorkDir = workDir } - c.WorkDir = workDir c.After = cmd.After // TODO: checksum must be refactored if cmd.Checksum != nil { diff --git a/internal/executor/execute_test.go b/internal/executor/execute_test.go index 732bfd9e..975990e7 100644 --- a/internal/executor/execute_test.go +++ b/internal/executor/execute_test.go @@ -397,6 +397,43 @@ func TestChecksumEnvVarsPresentInRunnerInvocation(t *testing.T) { } } +func TestChecksumUsesCommandWorkDir(t *testing.T) { + cfg := newTestCfg(t) + commandDir := filepath.Join(cfg.WorkDir, "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 { + t.Fatalf("write root input: %v", err) + } + if err := os.WriteFile(filepath.Join(commandDir, "input.txt"), []byte("command"), 0o644); err != nil { + t.Fatalf("write command input: %v", err) + } + + want, err := checksum.CalculateChecksum(commandDir, []string{"input.txt"}) + if err != nil { + t.Fatalf("calculate expected checksum: %v", err) + } + + cmd := newCmd("build", "build-script") + cmd.WorkDir = commandDir + cmd.ChecksumSources = map[string][]string{ + checksum.DefaultChecksumKey: {"input.txt"}, + } + + r := newRecordingRunner() + ex := NewExecutor(cfg, r.run) + + if err := ex.Execute(execCtx(cmd)); err != nil { + t.Fatalf("Execute: %v", err) + } + + got := cmd.ChecksumMap[checksum.DefaultChecksumKey] + if got != want { + t.Fatalf("expected checksum from command work_dir %q, got %q", want, got) + } +} + // TestChecksumPersistedAfterSuccess verifies that a successful Execute() persists // the checksum to disk when PersistChecksum is true. func TestChecksumPersistedAfterSuccess(t *testing.T) { diff --git a/internal/executor/executor.go b/internal/executor/executor.go index 535a8270..ca4b2a7c 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -199,7 +199,7 @@ func (e *Executor) initCmd(ctx *Context) error { maps.Copy(checksumEnv, e.cfg.GetEnv()) // calculate checksum if needed - if err := cmd.ChecksumCalculator(e.cfg.WorkDir, checksumShell, checksumEnv); err != nil { + if err := cmd.ChecksumCalculator(checksumWorkDir, checksumShell, checksumEnv); err != nil { return fmt.Errorf("failed to calculate checksum for command '%s': %w", cmd.Name, err) } From 6b226adafa4912df81a8ae12926005160d473fc6 Mon Sep 17 00:00:00 2001 From: Kindritskiy Maksym Date: Wed, 29 Jul 2026 11:53:21 +0300 Subject: [PATCH 5/7] Update docs/docs/config.md Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --- docs/docs/config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/config.md b/docs/docs/config.md index cdfaa72b..0b078353 100644 --- a/docs/docs/config.md +++ b/docs/docs/config.md @@ -891,7 +891,7 @@ commands: This feature is useful when you want to know that something has changed between two executions of a command. -`checksum.persist` can be used only if `checksum.files` or `checksum.sh` declared for command. +`checksum.persist` can be used only if `checksum.files` or `checksum.sh` is declared for the command. If set to `true`, each run all calculated checksums will be stored to disk. From 949a6c643dc5abcd6acfdd1ab0c2a6a804b8975b Mon Sep 17 00:00:00 2001 From: Kindritskiy Maksym Date: Wed, 29 Jul 2026 11:53:31 +0300 Subject: [PATCH 6/7] Update docs/docs/advanced_usage.md Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --- docs/docs/advanced_usage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/advanced_usage.md b/docs/docs/advanced_usage.md index 9c255c41..462252eb 100644 --- a/docs/docs/advanced_usage.md +++ b/docs/docs/advanced_usage.md @@ -97,7 +97,7 @@ When you add `checksum.files` directive to a command - `lets` will calculate che We then can store this checksum somewhere in the file and check that stored checksum with a checksum from env. -Fortunately, `lets` have an option for that - `checksum.persist`. +Fortunately, `lets` has an option for that - `checksum.persist`. If `checksum.persist` is used with `checksum.files`, `lets` will store new checksum to `.lets` dir and each time you run a command `lets` will check if stored checksum changed from the one from env. From c5c95ce4c51b1b67050311a1de36c1efa4384ac8 Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Wed, 29 Jul 2026 12:35:45 +0300 Subject: [PATCH 7/7] Address checksum review test gaps Preserve config file modes when applying migrations and add coverage for invalid checksum configuration validation branches. --- internal/config/config/command_test.go | 66 +++++++++++++++++++++++++ internal/config/migrate/migrate.go | 7 ++- internal/config/migrate/migrate_test.go | 43 ++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 internal/config/migrate/migrate_test.go diff --git a/internal/config/config/command_test.go b/internal/config/config/command_test.go index d23a8b0e..beb305da 100644 --- a/internal/config/config/command_test.go +++ b/internal/config/config/command_test.go @@ -2,6 +2,7 @@ package config import ( "bytes" + "strings" "testing" "github.com/lets-cli/lets/internal/checksum" @@ -119,4 +120,69 @@ func TestParseCommandChecksum(t *testing.T) { t.Fatal("expected persisted checksum") } }) + + t.Run("invalid configurations", func(t *testing.T) { + tests := []struct { + name string + text string + wantErr string + }{ + { + name: "both checksum files and sh set", + text: dedent.Dedent(` + checksum: + files: + - foo.txt + sh: echo 1234 + cmd: echo ok + `), + wantErr: "checksum must use only one of 'files' or 'sh'", + }, + { + name: "persist checksum without checksum", + text: dedent.Dedent(` + persist_checksum: true + cmd: echo ok + `), + wantErr: "'persist_checksum' must be used with 'checksum'", + }, + { + name: "checksum persist without files or sh", + text: dedent.Dedent(` + checksum: + persist: true + cmd: echo ok + `), + wantErr: "'persist_checksum' or 'checksum.persist' must be used with 'checksum.files' or 'checksum.sh'", + }, + { + name: "conflicting persist settings", + text: dedent.Dedent(` + persist_checksum: true + checksum: + files: + - foo.txt + persist: false + cmd: echo ok + `), + wantErr: "'persist_checksum' conflicts with 'checksum.persist'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := bytes.NewBufferString(tt.text) + command := &Command{} + + err := yaml.NewDecoder(buf).Decode(&command) + if err == nil { + t.Fatal("expected command fixture decode error") + } + + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("expected error to contain %q, got %q", tt.wantErr, err.Error()) + } + }) + } + }) } diff --git a/internal/config/migrate/migrate.go b/internal/config/migrate/migrate.go index 51aad469..7f5d72db 100644 --- a/internal/config/migrate/migrate.go +++ b/internal/config/migrate/migrate.go @@ -113,7 +113,12 @@ func fixFile(path string, dryRun bool, migrations []Migration) (bool, []string, preview := "" if !dryRun { - if err := os.WriteFile(path, updated, 0o644); err != nil { + info, err := os.Stat(path) + if err != nil { + return false, nil, "", fmt.Errorf("can not stat config %s: %w", path, err) + } + + if err := os.WriteFile(path, updated, info.Mode()); err != nil { return false, nil, "", fmt.Errorf("can not write config %s: %w", path, err) } } else { diff --git a/internal/config/migrate/migrate_test.go b/internal/config/migrate/migrate_test.go new file mode 100644 index 00000000..8caff8fa --- /dev/null +++ b/internal/config/migrate/migrate_test.go @@ -0,0 +1,43 @@ +package migrate + +import ( + "os" + "path/filepath" + "testing" +) + +func TestFixFilePreservesExistingFileMode(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "lets.yaml") + originalMode := os.FileMode(0o600) + + err := os.WriteFile(path, []byte(` +shell: bash +commands: + build: + persist_checksum: true + checksum: + - go.mod + cmd: go build +`), originalMode) + if err != nil { + t.Fatalf("write fixture: %v", err) + } + + changed, _, _, err := fixFile(path, false, []Migration{ChecksumMigration{}}) + if err != nil { + t.Fatalf("fix file: %v", err) + } + if !changed { + t.Fatal("expected file to change") + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat migrated file: %v", err) + } + + if got := info.Mode().Perm(); got != originalMode { + t.Fatalf("expected mode %s, got %s", originalMode, got) + } +}