diff --git a/pkg/dirty_set.go b/pkg/dirty_set.go index e545f3a..e5bd862 100644 --- a/pkg/dirty_set.go +++ b/pkg/dirty_set.go @@ -301,14 +301,49 @@ func propagateFrom(dirtyLabels, actuallyChanged map[string]bool, edges map[strin return result } +// nonLockfileExtensions are extensions a dependency lockfile never uses. +// Lockfile detection matches on name fragments, so without this a script, +// document or migration merely named after a lock forces a full rehash. +// +// Keep this list to executable code, images and prose. Anything that could +// hold structured dependency data must stay off it, however unlikely it +// looks: pip writes requirements_lock.txt, so ".txt" belongs to lockfiles, +// and ".xml" is omitted for the same reason. Wrongly listing an extension +// here silently under-reports impacted targets, which is far worse than the +// redundant rehash a missing entry causes. +var nonLockfileExtensions = map[string]bool{ + ".md": true, ".rst": true, ".html": true, + ".png": true, ".jpg": true, ".jpeg": true, ".svg": true, + ".sh": true, ".bash": true, + ".go": true, ".java": true, ".py": true, ".ts": true, ".js": true, + ".sql": true, +} + +// looksLikeLockfile reports whether lowerBasename, already lowercased, names a +// dependency lockfile. Three shapes occur: a ".lock" extension (Cargo.lock, +// buf.lock), ".lock" as an inner component (multitool.lock.json, +// .terraform.lock.hcl), and a "-lock" or "_lock" name stem (pnpm-lock.yaml, +// package-lock.json). Only the first is unambiguous; the other two also match +// ordinary files that happen to be named after a lock, so they additionally +// require an extension a lockfile could plausibly use. +func looksLikeLockfile(lowerBasename string) bool { + if strings.HasSuffix(lowerBasename, ".lock") { + return true + } + if !strings.Contains(lowerBasename, ".lock.") && + !strings.Contains(lowerBasename, "-lock.") && + !strings.Contains(lowerBasename, "_lock.") { + return false + } + return !nonLockfileExtensions[filepath.Ext(lowerBasename)] +} + func isFallbackTrigger(basename string) bool { if strings.HasSuffix(basename, ".bzl") { return true } lowerBasename := strings.ToLower(basename) - if strings.HasSuffix(lowerBasename, ".lock") || - strings.Contains(lowerBasename, "-lock.") || - strings.Contains(lowerBasename, "_lock.") { + if looksLikeLockfile(lowerBasename) { return true } if basename == ".bazelrc" || strings.HasPrefix(basename, ".bazelrc.") || diff --git a/pkg/dirty_set_test.go b/pkg/dirty_set_test.go index 6d14df3..d9cda5d 100644 --- a/pkg/dirty_set_test.go +++ b/pkg/dirty_set_test.go @@ -724,3 +724,82 @@ func TestPruneDirtySetReadsDependencyHashes(t *testing.T) { t.Error("//app:consumer should be pruned: //pkg:tool is unchanged per DependencyHashes") } } + +func TestIsFallbackTriggerLockfiles(t *testing.T) { + tests := []struct { + basename string + want bool + }{ + // Plain ".lock" extension. + {"Cargo.lock", true}, + {"yarn.lock", true}, + {"poetry.lock", true}, + {"uv.lock", true}, + {"buf.lock", true}, + {"MODULE.bazel.lock", true}, + + // ".lock" as an inner component. + {"multitool.lock.json", true}, + {".terraform.lock.hcl", true}, + + // "-lock" or "_lock" name stem. + {"pnpm-lock.yaml", true}, + {"package-lock.json", true}, + {"requirements_lock.txt", true}, + + // Extensions that could hold dependency data stay conservative, as + // does an unrecognised one. + {"deps-lock.xml", true}, + {"deps-lock.xyz", true}, + + // Ordinary files merely named after a lock must not trigger. + {"merge-pnpm-lock.sh", false}, + {"regen-pnpm-lock.sh", false}, + {"repin-with-lock.sh", false}, + {"2x-lock.png", false}, + {"02_add_and_lock.md", false}, + {"v3__workflow_lock.sql", false}, + + // Nothing lock-like at all. + {"README.md", false}, + {"Main.java", false}, + } + + for _, tt := range tests { + if got := isFallbackTrigger(tt.basename); got != tt.want { + t.Errorf("isFallbackTrigger(%q) = %v, want %v", tt.basename, got, tt.want) + } + } +} + +func TestComputeDirtySetNestedLockExtensionFallsBack(t *testing.T) { + changedFiles := map[string]string{ + "tools/multitool.lock.json": "M", + } + + result := ComputeDirtySet(changedFiles, nil, nil, nil) + + if !result.NeedsFallback { + t.Fatal("expected fallback for multitool.lock.json change") + } + if result.FallbackCode != "unsafe_file_change" { + t.Errorf("FallbackCode = %q, want unsafe_file_change", result.FallbackCode) + } +} + +func TestComputeDirtySetLockNamedScriptDoesNotFallBack(t *testing.T) { + edges := map[string][]string{ + "//pkg:rule_a": {}, + } + allLabels := CollectAllLabels(edges, nil) + + changedFiles := map[string]string{ + "tools/git/merge-pnpm-lock.sh": "M", + } + + result := ComputeDirtySet(changedFiles, edges, allLabels, nil) + + if result.NeedsFallback { + t.Errorf("unexpected fallback for a shell script: %s", result.FallbackReason) + } +}