diff --git a/CHANGELOG.md b/CHANGELOG.md index e88b3b7ea..39dfa8a96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,13 @@ Given a version number `MAJOR.MINOR.PATCH`, we increment the: - Backward compatibility in versions `0.0.z` is **not guaranteed** when `z` is increased. - Backward compatibility in versions `0.y.z` is **not guaranteed** when `y` is increased. +## Unreleased + +### Fixed + +- Fix "outdated-code" safeguard giving false positive results for files generated + in subdirectory of stacks. + ## v0.10.5 ### Fixed diff --git a/config/config.go b/config/config.go index 56d867574..2efcaab0e 100644 --- a/config/config.go +++ b/config/config.go @@ -340,6 +340,17 @@ func (tree *Tree) IsStack() bool { return tree.Node.Stack != nil } +// IsInsideStack tells if current tree node is inside a parent stack. +func (tree *Tree) IsInsideStack() bool { + if tree.Parent == nil { + return false + } + if tree.Parent.IsStack() { + return true + } + return tree.Parent.IsInsideStack() +} + // Stack returns the stack object. func (tree *Tree) Stack() (*Stack, error) { if tree.stack == nil { diff --git a/generate/generate.go b/generate/generate.go index ce621efe9..1e4c36aac 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -437,7 +437,7 @@ func handleAsserts(rootdir string, dir string, asserts []config.Assert) error { return errs.AsError() } -// ListGenFiles will list the path of all generated code inside the given dir +// ListStackGenFiles will list the path of all generated code inside the given dir // and all its subdirs that are not stacks. The returned paths are relative to // the given dir, like: // @@ -456,7 +456,7 @@ func handleAsserts(rootdir string, dir string, asserts []config.Assert) error { // // When called with a dir that is a stack this function will list all generated // files that are owned by the stack, since it won't search inside any child stacks. -func ListGenFiles(root *config.Root, dir string) ([]string, error) { +func ListStackGenFiles(root *config.Root, dir string) ([]string, error) { pendingSubDirs := []string{""} genfiles := []string{} @@ -532,8 +532,22 @@ func DetectOutdated(root *config.Root, target *config.Tree, vendorDir project.Pa logger.Debug().Msg("checking outdated code inside stacks") + for _, cfg := range target.Stacks() { + outdated, err := stackContextOutdated(root, cfg, vendorDir) + if err != nil { + errs.Append(err) + continue + } + + // We want results relative to root + dirRelPath := cfg.Dir().String()[1:] + for _, file := range outdated { + outdatedFiles.add(path.Join(dirRelPath, file)) + } + } + for _, cfg := range target.AsList() { - outdated, err := stackOutdated(root, cfg, vendorDir) + outdated, err := rootContextOutdated(root, cfg) if err != nil { errs.Append(err) continue @@ -550,14 +564,41 @@ func DetectOutdated(root *config.Root, target *config.Tree, vendorDir project.Pa return nil, err } + // If the base dir is a stack then there is no need to check orphaned files. + // All files are owned by the parent stack or its children. + if target.IsStack() || target.IsInsideStack() { + logger.Debug().Msg("target directory is stack or inside a stack, no need to check for orphaned files") + + outdated := outdatedFiles.slice() + sort.Strings(outdated) + return outdated, nil + } + + logger.Debug().Msg("checking for orphaned files") + + orphanedFiles, err := ListStackGenFiles(root, target.HostDir()) + if err != nil { + errs.Append(err) + } + + if err := errs.AsError(); err != nil { + return nil, err + } + + // We want results relative to root + dirRelPath := target.Dir().String()[1:] + for _, file := range orphanedFiles { + outdatedFiles.add(path.Join(dirRelPath, file)) + } + outdated := outdatedFiles.slice() sort.Strings(outdated) return outdated, nil } -// stackOutdated will verify if a given directory has outdated code and return a list -// of filenames that are outdated, ordered lexicographically. -func stackOutdated(root *config.Root, cfg *config.Tree, vendorDir project.Path) ([]string, error) { +// stackContextOutdated will verify if a given directory has outdated code +// for blocks with context=stack and return a list of filenames that are outdated. +func stackContextOutdated(root *config.Root, cfg *config.Tree, vendorDir project.Path) ([]string, error) { logger := log.With(). Str("action", "generate.stackOutdated"). Stringer("stack", cfg.Dir()). @@ -565,27 +606,16 @@ func stackOutdated(root *config.Root, cfg *config.Tree, vendorDir project.Path) cfgpath := cfg.HostDir() - var generated []GenFile - if cfg.IsStack() { - stackGenerated, err := loadStackCodeCfgs(root, cfg, vendorDir, nil) - if err != nil { - return nil, err - } - err = validateStackGeneratedFiles(root, cfgpath, stackGenerated) - if err != nil { - return nil, err - } - generated = append(generated, stackGenerated...) + generated, err := loadStackCodeCfgs(root, cfg, vendorDir, nil) + if err != nil { + return nil, err } - - rootGenerated, err := loadRootCodeCfgs(root, cfg) + err = validateStackGeneratedFiles(root, cfgpath, generated) if err != nil { return nil, err } - generated = append(generated, rootGenerated...) - - genfilesOnFs, err := ListGenFiles(root, cfgpath) + genfilesOnFs, err := ListStackGenFiles(root, cfgpath) if err != nil { return nil, errors.E(err, "checking for outdated code") } @@ -602,6 +632,26 @@ func stackOutdated(root *config.Root, cfg *config.Tree, vendorDir project.Path) return outdatedFiles.slice(), nil } +// rootContextOutdated will verify if the given directory has outdated code for context=root blocks +// and return the list of outdated files. +func rootContextOutdated(root *config.Root, cfg *config.Tree) ([]string, error) { + cfgpath := cfg.HostDir() + + generated, err := loadRootCodeCfgs(root, cfg) + if err != nil { + return nil, err + } + + // We start with the assumption that all gen files on the stack + // are outdated and then update the outdated files set as we go. + outdatedFiles := newStringSet() + err = updateOutdatedFiles(root, cfgpath, generated, outdatedFiles) + if err != nil { + return nil, errors.E(err, "handling detected files") + } + return outdatedFiles.slice(), nil +} + func updateOutdatedFiles(root *config.Root, cfgpath string, generated []GenFile, outdatedFiles *stringSet) error { logger := log.With(). Str("action", "generate.updateOutdatedFiles"). @@ -754,7 +804,7 @@ func allStackGeneratedFiles( genfiles []GenFile, ) (map[string]string, error) { allFiles := map[string]string{} - files, err := ListGenFiles(root, dir) + files, err := ListStackGenFiles(root, dir) if err != nil { return nil, err } @@ -1303,7 +1353,7 @@ func cleanupOrphaned(root *config.Root, report Report) Report { logger.Debug().Msg("listing orphaned generated files") - orphanedGenFiles, err := ListGenFiles(root, root.HostDir()) + orphanedGenFiles, err := ListStackGenFiles(root, root.HostDir()) if err != nil { report.CleanupErr = err return report diff --git a/generate/generate_list_test.go b/generate/generate_list_test.go index 0d71d9d77..4f702acf7 100644 --- a/generate/generate_list_test.go +++ b/generate/generate_list_test.go @@ -232,7 +232,7 @@ func TestGeneratedFilesListing(t *testing.T) { listdir = s.RootDir() } - got, err := generate.ListGenFiles(s.Config(), listdir) + got, err := generate.ListStackGenFiles(s.Config(), listdir) assert.NoError(t, err) assertEqualStringList(t, got, tcase.want) }) diff --git a/generate/outdated_detection_test.go b/generate/outdated_detection_test.go index f6ed0fa95..c480c5bc4 100644 --- a/generate/outdated_detection_test.go +++ b/generate/outdated_detection_test.go @@ -26,6 +26,7 @@ func TestOutdatedDetection(t *testing.T) { body fmt.Stringer } step struct { + wd string layout []string vendorDir string files []file @@ -643,6 +644,145 @@ func TestOutdatedDetection(t *testing.T) { }, }, }, + { + name: "detecting orphan files from root dir", + steps: []step{ + { + layout: []string{ + "s:/", + "s:/stack-1", + "s:/stack-2", + }, + files: []file{ + { + path: "config.tm", + body: Doc( + GenerateFile( + Labels("test.txt"), + Str("content", "tm is awesome"), + ), + GenerateHCL( + Labels("test.hcl"), + Content( + Str("content", "tm is awesome"), + ), + ), + ), + }, + }, + want: []string{ + "stack-1/test.hcl", + "stack-1/test.txt", + "stack-2/test.hcl", + "stack-2/test.txt", + "test.hcl", + "test.txt", + }, + }, + { + files: []file{ + { + path: "stack.tm.hcl", + body: Doc(), + }, + }, + want: []string{ + "test.hcl", + }, + }, + }, + }, + { + name: "running from a subdir of a stack do not misdetect as orphan files", + steps: []step{ + { + layout: []string{ + "s:/", + "s:/stack-1", + "s:/stack-2", + }, + files: []file{ + { + path: "config.tm", + body: Doc( + GenerateFile( + Labels("subdir/test.txt"), + Str("content", "tm is awesome"), + ), + GenerateHCL( + Labels("subdir/test.hcl"), + Content( + Str("content", "tm is awesome"), + ), + ), + ), + }, + }, + want: []string{ + "stack-1/subdir/test.hcl", + "stack-1/subdir/test.txt", + "stack-2/subdir/test.hcl", + "stack-2/subdir/test.txt", + "subdir/test.hcl", + "subdir/test.txt", + }, + }, + { + // this step just executes in the subdir to catch outdated code (if any) + wd: "/subdir", + }, + }, + }, + { + name: "running from a subdir still detects orphan files", + steps: []step{ + { + layout: []string{ + "s:/", + "s:/stack-1", + "s:/stack-2", + }, + files: []file{ + { + path: "config.tm", + body: Doc( + GenerateFile( + Labels("subdir/test.txt"), + Str("content", "tm is awesome"), + ), + GenerateHCL( + Labels("subdir/test.hcl"), + Content( + Str("content", "tm is awesome"), + ), + ), + ), + }, + }, + want: []string{ + "stack-1/subdir/test.hcl", + "stack-1/subdir/test.txt", + "stack-2/subdir/test.hcl", + "stack-2/subdir/test.txt", + "subdir/test.hcl", + "subdir/test.txt", + }, + }, + { + // this step just executes in the subdir to catch outdated code (if any) + wd: "/subdir", + files: []file{ + { + path: "/stack.tm.hcl", + body: Doc(), + }, + }, + want: []string{ + "subdir/test.hcl", + }, + }, + }, + }, { name: "changing vendorDir changes all generate blocks calling tm_vendor", steps: []step{ @@ -1326,6 +1466,65 @@ func TestOutdatedDetection(t *testing.T) { }, }, }, + { + name: "detecting outdated code when stack generates in subdir", + steps: []step{ + { + layout: []string{ + "s:/", + "d:/subdir", + "s:/stack-1", + "s:/stack-2", + "d:/stack-1/somedir", + "d:/stack-2/somedir", + }, + files: []file{ + { + path: "config.tm", + body: Doc( + GenerateHCL( + Labels("somedir/test.hcl"), + Content( + Str("content", "tm is awesome"), + ), + ), + ), + }, + }, + want: []string{ + "somedir/test.hcl", + "stack-1/somedir/test.hcl", + "stack-2/somedir/test.hcl", + }, + }, + { + files: []file{ + { + path: "config.tm", + body: Doc( + GenerateFile( + Labels("test.txt"), + Bool("condition", false), + Str("content", "tm is awesome"), + ), + GenerateHCL( + Labels("test.hcl"), + Bool("condition", false), + Content( + Str("content", "tm is awesome"), + ), + ), + ), + }, + }, + want: []string{ + "somedir/test.hcl", + "stack-1/somedir/test.hcl", + "stack-2/somedir/test.hcl", + }, + }, + }, + }, } for _, tc := range tcases { @@ -1352,7 +1551,16 @@ func TestOutdatedDetection(t *testing.T) { vendorDir = project.NewPath(step.vendorDir) } - got, err := generate.DetectOutdated(s.Config(), s.Config().Tree(), vendorDir) + target := s.Config().Tree() + if step.wd != "" { + var ok bool + target, ok = s.Config().Lookup(project.NewPath(step.wd)) + if !ok { + panic("invalid step.wd working directory") + } + } + + got, err := generate.DetectOutdated(s.Config(), target, vendorDir) assert.IsError(t, err, step.wantErr) if err != nil { continue diff --git a/test/sandbox/sandbox.go b/test/sandbox/sandbox.go index eab68cd3f..1327a8a18 100644 --- a/test/sandbox/sandbox.go +++ b/test/sandbox/sandbox.go @@ -437,7 +437,7 @@ func (de DirEntry) CreateFile(name, body string, args ...interface{}) FileEntry func (de DirEntry) ListGenFiles(root *config.Root) []string { de.t.Helper() - files, err := generate.ListGenFiles(root, de.abspath) + files, err := generate.ListStackGenFiles(root, de.abspath) assert.NoError(de.t, err, "listing dir generated files") return files }