fix: fallback to file listing if change detection inside a skipped di… (

#1900)

## What this PR does / why we need it:

Fix the optimization done in #1898 for the case a directory in the scope
of change detection was skipped with `.tmskip`.

It's just a fallback to the older directory listing in this case.

## Which issue(s) this PR fixes:
none

## Special notes for your reviewer:

## Does this PR introduce a user-facing change?
```
no because the offending PR is unreleased
```

config/config.go

@@ -62,6 +62,8 @@ type Tree struct {
 	// Node is the configuration of this tree node.
 	Node hcl.Config
 
+	Skipped bool // tells if this node subdirs were skipped
+
 	TerramateFiles []string
 	OtherFiles     []string
 
@@ -154,7 +156,14 @@ func (root *Root) Tree() *Tree { return &root.tree }
 func (root *Root) HostDir() string { return root.tree.RootDir() }
 
 // Lookup a node from the root using a filesystem query path.
-func (root *Root) Lookup(path project.Path) (*Tree, bool) {
+func (root *Root) Lookup(path project.Path) (node *Tree, found bool) {
+	node, _, found = root.tree.lookup(path)
+	return node, found
+}
+
+// Lookup2 is like Lookup but returns skipped as true if the path is not found because
+// a parent directory was skipped.
+func (root *Root) Lookup2(path project.Path) (node *Tree, skipped bool, found bool) {
 	return root.tree.lookup(path)
 }
 
@@ -387,10 +396,13 @@ func (tree *Tree) stacks(cond func(*Tree) bool) List[*Tree] {
 
 // Lookup a node from the tree using a filesystem query path.
 // The abspath is relative to the current tree node.
-func (tree *Tree) lookup(abspath project.Path) (*Tree, bool) {
+func (tree *Tree) lookup(abspath project.Path) (node *Tree, skipped bool, found bool) {
+	if tree.Skipped {
+		return nil, true, false
+	}
 	pathstr := abspath.String()
 	if len(pathstr) == 0 || pathstr[0] != '/' {
-		return nil, false
+		return nil, false, false
 	}
 
 	parts := strings.Split(pathstr, "/")
@@ -402,11 +414,11 @@ func (tree *Tree) lookup(abspath project.Path) (*Tree, bool) {
 		}
 		node, found := cfg.Children[parts[i]]
 		if !found {
-			return nil, false
+			return nil, cfg.Skipped, false
 		}
 		cfg = node
 	}
-	return cfg, true
+	return cfg, false, true
 }
 
 // AsList returns a list with this node and all its children.
@@ -439,7 +451,7 @@ func loadTree(parentTree *Tree, cfgdir string, rootcfg *hcl.Config) (_ *Tree, er
 	for _, fname := range otherFiles {
 		if fname == SkipFilename {
 			logger.Debug().Msg("skip file found: skipping whole subtree")
-			return NewTree(cfgdir), nil
+			return newSkippedTree(cfgdir), nil
 		}
 	}
 
@@ -618,6 +630,12 @@ func NewTree(cfgdir string) *Tree {
 	}
 }
 
+func newSkippedTree(cfgdir string) *Tree {
+	t := NewTree(cfgdir)
+	t.Skipped = true
+	return t
+}
+
 func (tree *Tree) hasExperiment(name string) bool {
 	if tree.Parent != nil {
 		return tree.Parent.hasExperiment(name)

stack/manager.go

@@ -33,7 +33,7 @@ type (
 
 		cache struct {
 			stacks       []Entry
-			changedFiles []string
+			changedFiles map[string][]string // gitBaseRef -> changed files
 		}
 	}
 
@@ -63,17 +63,21 @@ const errListChanged errors.Kind = "listing changed stacks error"
 
 // NewManager creates a new stack manager.
 func NewManager(root *config.Root) *Manager {
-	return &Manager{
+	m := &Manager{
 		root: root,
 	}
+	m.cache.changedFiles = make(map[string][]string)
+	return m
 }
 
 // NewGitAwareManager returns a stack manager that supports change detection.
 func NewGitAwareManager(root *config.Root, git *git.Git) *Manager {
-	return &Manager{
+	m := &Manager{
 		root: root,
 		git:  git,
 	}
+	m.cache.changedFiles = make(map[string][]string)
+	return m
 }
 
 // List walks the basedir directory looking for terraform stacks.
@@ -126,19 +130,21 @@ func (m *Manager) ListChanged(gitBaseRef string) (*Report, error) {
 		return nil, errors.E(errListChanged, err)
 	}
 
-	if m.cache.changedFiles == nil {
-		var err error
+	changedFiles, err := m.changedFiles(gitBaseRef)
+	if err != nil {
+		return nil, errors.E(errListChanged, err)
+	}
 
-		m.cache.changedFiles, err = m.listChangedFiles(m.root.HostDir(), gitBaseRef)
-		if err != nil {
-			return nil, errors.E(errListChanged, err)
-		}
+	if len(changedFiles) == 0 {
+		return &Report{
+			Checks: checks,
+		}, nil
 	}
 
 	stackSet := map[project.Path]Entry{}
 	ignoreSet := map[project.Path]struct{}{}
 
-	for _, path := range m.cache.changedFiles {
+	for _, path := range changedFiles {
 		abspath := filepath.Join(m.root.HostDir(), path)
 		projpath := project.PrjAbsPath(m.root.HostDir(), abspath)
 
@@ -262,7 +268,7 @@ rangeStacks:
 			continue
 		}
 
-		if changed, ok := hasChangedWatchedFiles(stack, m.cache.changedFiles); ok {
+		if changed, ok := hasChangedWatchedFiles(stack, changedFiles); ok {
 			logger.Debug().
 				Stringer("stack", stack).
 				Stringer("watchfile", changed).
@@ -466,12 +472,44 @@ func (m *Manager) AddWantedOf(scopeStacks config.List[*config.SortableStack]) (c
 }
 
 func (m *Manager) filesApply(dir project.Path, apply func(fname string) error) (err error) {
-	tree, ok := m.root.Lookup(dir)
-	if !ok {
-		return errors.E("directory not found: %s", dir)
+	var files []string
+
+	tree, skipped, ok := m.root.Lookup2(dir)
+	if !ok && !skipped {
+		panic(errors.E(errors.ErrInternal, "path is not in the config tree and not .tmskip'ed: %s", dir))
+	}
+
+	if skipped {
+		// WHY: This can only happen if the user is adding a .tmskip in a modules or similar folder.
+		// The user must have a .tmskip in modules for several reasons but the most common
+		// are to speed up Terramate config loading or because they depend on Terraform
+		// modules (from other repos) that contain stack definitions that must not be recognized
+		// in this project.
+		f, err := os.Open(dir.HostPath(m.root.HostDir()))
+		if err != nil {
+			return errors.E(err, "opening directory %q", dir)
+		}
+
+		defer func() {
+			err = errors.L(err, f.Close()).AsError()
+		}()
+
+		entries, err := f.ReadDir(-1)
+		if err != nil {
+			return errors.E(err, "listing files of directory %q", dir)
+		}
+
+		for _, file := range entries {
+			if file.IsDir() {
+				continue
+			}
+			files = append(files, file.Name())
+		}
+	} else {
+		files = tree.OtherFiles
 	}
 
-	for _, fname := range tree.OtherFiles {
+	for _, fname := range files {
 		err := apply(fname)
 		if err != nil {
 			return errors.E(err, "applying operation to file %q", fname)
@@ -508,7 +546,11 @@ func (m *Manager) tfModuleChanged(
 		return false, "", errors.E("\"source\" path %q is not a directory", modAbsPath)
 	}
 
-	for _, changedFile := range m.cache.changedFiles {
+	changedFiles, err := m.changedFiles(gitBaseRef)
+	if err != nil {
+		return false, "", err
+	}
+	for _, changedFile := range changedFiles {
 		changedPath := filepath.Join(m.root.HostDir(), changedFile)
 		if strings.HasPrefix(changedPath, modAbsPath) {
 			return true, fmt.Sprintf("module %q has unmerged changes", mod.Source), nil
@@ -554,6 +596,21 @@ func (m *Manager) tfModuleChanged(
 	return changed, fmt.Sprintf("module %q changed because %s", mod.Source, why), nil
 }
 
+func (m *Manager) changedFiles(gitBaseRef string) ([]string, error) {
+	changedFiles, ok := m.cache.changedFiles[gitBaseRef]
+	if !ok {
+		var err error
+
+		changedFiles, err = m.listChangedFiles(m.root.HostDir(), gitBaseRef)
+		if err != nil {
+			return nil, errors.E(errListChanged, err)
+		}
+
+		m.cache.changedFiles[gitBaseRef] = changedFiles
+	}
+	return changedFiles, nil
+}
+
 func (m *Manager) tgModuleChanged(
 	stack *config.Stack, tgMod *tg.Module, gitBaseRef string, stackSet map[project.Path]Entry, tgModuleMap map[project.Path]*tg.Module,
 ) (changed bool, why string, err error) {
@@ -568,6 +625,11 @@ func (m *Manager) tgModuleChanged(
 		}
 	}
 
+	changedFiles, err := m.changedFiles(gitBaseRef)
+	if err != nil {
+		return false, "", err
+	}
+
 	for _, dep := range tgMod.DependsOn {
 		// if the module is a stack already detected as changed, just mark this as changed and
 		// move on. Fast path.
@@ -578,7 +640,7 @@ func (m *Manager) tgModuleChanged(
 			}
 		}
 
-		for _, changedFile := range m.cache.changedFiles {
+		for _, changedFile := range changedFiles {
 			changedPath := project.PrjAbsPath(m.root.HostDir(), changedFile)
 			if dep == changedPath {
 				return true, fmt.Sprintf("module %q changed because %q changed", tgMod.Path, dep), nil
@@ -599,7 +661,7 @@ func (m *Manager) tgModuleChanged(
 			continue
 		}
 
-		for _, file := range m.cache.changedFiles {
+		for _, file := range changedFiles {
 			if strings.HasPrefix(filepath.Join(m.root.HostDir(), file), depAbsPath) {
 				return true, fmt.Sprintf("module %q changed because %q changed", tgMod.Path, dep), nil
 			}