Skip to content

Commit 7db4c31

Browse files
authored
refactor(hash-persister): remove verify-seed mode (#16)
## Summary Remove the `--verify-seed` mode from `hash-persister`. This functionality is not needed because incremental hashing will be validated externally by: 1. running `hash-persister` in normal, non-seeded mode to produce the authoritative full hash artifact; 2. running `hash-persister` in seeded mode for the same destination revision; and 3. comparing the two outputs with `hash-differ`. Keeping this validation in the rollout orchestration preserves a clear separation of responsibilities: `hash-persister` produces either full or incrementally seeded artifacts, while CI decides when to run both modes and how to report mismatches. It also lets the authoritative full result remain unchanged if the shadow incremental run fails or diverges. The change removes the `--verify-seed` flag, its dual-run orchestration and verification-only report mode, the now-unused file-copy helper, and the corresponding README section. ## Testing - `bazel test //hash-persister:hash-persister_test //pkg:pkg_test`
2 parents dd2295a + 6b23b87 commit 7db4c31

2 files changed

Lines changed: 0 additions & 111 deletions

File tree

README.md

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -95,23 +95,6 @@ hash-persister \
9595
"${BASE_SHA}"
9696
```
9797

98-
#### Verifying incremental results
99-
100-
`--verify-seed` runs both incremental and full hashing for the destination revision and compares the resulting target hashes:
101-
102-
```sh
103-
hash-persister \
104-
--working-directory . \
105-
--query-backend=query \
106-
--output "hashes-${NEW_SHA}.json" \
107-
--seed-file "hashes-${BASE_SHA}.json" \
108-
--seed-sha "${BASE_SHA}" \
109-
--verify-seed \
110-
"${NEW_SHA}"
111-
```
112-
113-
Verification exits nonzero if the two results differ or if incremental execution falls back to full hashing. When the two computations differ, the output path receives the full result so it remains safe for investigation and downstream use. A fallback stops verification rather than treating two full computations as evidence that incremental mode is correct. Verification is intended for rollout checks and sampling rather than the normal fast path because it deliberately performs both computations.
114-
11598
#### Execution reports
11699

117100
Pass `--execution-report <path>` to write a versioned, machine-readable JSON summary. The report distinguishes the requested mode from the mode actually used, records success or failure and any fallback code, and includes counts for changed files, dirty packages and targets, recomputed targets, reused targets, and total targets. This lets CI systems emit bounded metrics without parsing human-readable logs.

hash-persister/hash-persister.go

Lines changed: 0 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ type hashPersisterFlags struct {
2727
seedableOutput bool
2828
seedFile string
2929
seedSha string
30-
verifySeed bool
3130
reportFile string
3231
}
3332

@@ -39,7 +38,6 @@ type config struct {
3938
SeedableOutput bool
4039
SeedFile string
4140
SeedSha string
42-
VerifySeed bool
4341
ReportFile string
4442
}
4543

@@ -112,10 +110,6 @@ func execute() (returnErr error) {
112110
report.RequestedMode = "incremental"
113111
report.EffectiveMode = "incremental"
114112
}
115-
if cfg.VerifySeed {
116-
report.RequestedMode = "verify"
117-
report.EffectiveMode = "verify"
118-
}
119113
defer func() {
120114
if returnErr != nil {
121115
report.Status = "failed"
@@ -137,10 +131,6 @@ func execute() (returnErr error) {
137131
}
138132
}()
139133

140-
if cfg.VerifySeed && cfg.SeedFile != "" {
141-
return runVerifySeed(cfg)
142-
}
143-
144134
if cfg.SeedFile != "" {
145135
outcome, err := runSeeded(cfg)
146136
applySeededOutcome(report, outcome)
@@ -476,81 +466,13 @@ func mergePersistedEntries[V any](seed map[string]V, dirty map[string]bool, fres
476466
return merged
477467
}
478468

479-
func runVerifySeed(cfg *config) error {
480-
log.Printf("Verify-seed mode: computing seeded AND full hashes for %s", cfg.CommitSha)
481-
482-
seededOutput, err := os.CreateTemp("", "td-verify-seeded-*.json")
483-
if err != nil {
484-
return fmt.Errorf("failed to create temp file for seeded output: %w", err)
485-
}
486-
seededPath := seededOutput.Name()
487-
seededOutput.Close()
488-
defer os.Remove(seededPath)
489-
490-
fullOutput, err := os.CreateTemp("", "td-verify-full-*.json")
491-
if err != nil {
492-
return fmt.Errorf("failed to create temp file for full output: %w", err)
493-
}
494-
fullPath := fullOutput.Name()
495-
fullOutput.Close()
496-
defer os.Remove(fullPath)
497-
498-
seededCfg := *cfg
499-
seededCfg.OutputFile = seededPath
500-
seededCfg.VerifySeed = false
501-
outcome, err := runSeeded(&seededCfg)
502-
if err != nil {
503-
return err
504-
}
505-
if !outcome.UsedSeed {
506-
return fmt.Errorf("verify-seed did not exercise seeded mode: %s", outcome.FallbackDetail)
507-
}
508-
509-
fullCfg := *cfg
510-
fullCfg.OutputFile = fullPath
511-
fullCfg.SeedFile = ""
512-
fullCfg.SeedSha = ""
513-
fullCfg.VerifySeed = false
514-
if _, err := runFull(&fullCfg); err != nil {
515-
return err
516-
}
517-
518-
result, err := pkg.CompareHashFiles(fullPath, seededPath)
519-
if err != nil {
520-
return fmt.Errorf("failed to compare hash files: %w", err)
521-
}
522-
523-
if len(result.Differences) == 0 {
524-
log.Printf("VERIFY-SEED: PASS — seeded output matches full computation")
525-
return copyFile(fullPath, cfg.OutputFile)
526-
} else {
527-
log.Printf("VERIFY-SEED: FAIL — %d differences found:", len(result.Differences))
528-
log.Printf(" Changed: %d, Added: %d, Removed: %d",
529-
result.Summary.TotalChanged, result.Summary.TotalAdded, result.Summary.TotalRemoved)
530-
for i, diff := range result.Differences {
531-
if i >= 20 {
532-
log.Printf(" ... and %d more", len(result.Differences)-20)
533-
break
534-
}
535-
log.Printf(" %s [%s] %s: before=%s after=%s",
536-
diff.Label, diff.Configuration, diff.Status,
537-
diff.BeforeHash, diff.AfterHash)
538-
}
539-
if err := copyFile(fullPath, cfg.OutputFile); err != nil {
540-
return err
541-
}
542-
return fmt.Errorf("verify-seed found %d hash differences", len(result.Differences))
543-
}
544-
}
545-
546469
func parseFlags() (*hashPersisterFlags, error) {
547470
var flags hashPersisterFlags
548471
flags.commonFlags = cli.RegisterCommonFlags()
549472
flag.StringVar(&flags.outputFile, "output", "", "Output file path for persisted hashes (required)")
550473
flag.BoolVar(&flags.seedableOutput, "seedable-output", false, "Include dependency edges and compatibility metadata so the output can seed incremental hashing")
551474
flag.StringVar(&flags.seedFile, "seed-file", "", "Path to a compatible seed hash file for incremental hashing")
552475
flag.StringVar(&flags.seedSha, "seed-sha", "", "Git commit SHA of the seed file (required with --seed-file)")
553-
flag.BoolVar(&flags.verifySeed, "verify-seed", false, "Run both seeded and full computation, compare, exit non-zero on divergence")
554476
flag.StringVar(&flags.reportFile, "execution-report", "", "Optional path for a machine-readable JSON execution report")
555477

556478
flag.Parse()
@@ -562,10 +484,6 @@ func parseFlags() (*hashPersisterFlags, error) {
562484
if flags.seedFile != "" && flags.seedSha == "" {
563485
return nil, fmt.Errorf("--seed-sha is required when --seed-file is specified")
564486
}
565-
if flags.verifySeed && flags.seedFile == "" {
566-
return nil, fmt.Errorf("--verify-seed requires --seed-file")
567-
}
568-
569487
positional := flag.Args()
570488
if len(positional) != 1 {
571489
return nil, fmt.Errorf("expected one positional argument, <git-commit-sha>, but got %d", len(positional))
@@ -647,7 +565,6 @@ func resolveConfig(flags hashPersisterFlags) (*config, error) {
647565
SeedableOutput: seedableOutput,
648566
SeedFile: flags.seedFile,
649567
SeedSha: flags.seedSha,
650-
VerifySeed: flags.verifySeed,
651568
ReportFile: flags.reportFile,
652569
}, nil
653570
}
@@ -729,17 +646,6 @@ func parseGitNameStatus(output []byte) (map[string]string, error) {
729646
return result, nil
730647
}
731648

732-
func copyFile(src, dst string) error {
733-
data, err := os.ReadFile(src)
734-
if err != nil {
735-
return fmt.Errorf("failed to read %s: %w", src, err)
736-
}
737-
if err := os.WriteFile(dst, data, 0o644); err != nil {
738-
return fmt.Errorf("failed to write %s: %w", dst, err)
739-
}
740-
return nil
741-
}
742-
743649
func countHashes(hashes map[string]map[string]string) int {
744650
n := 0
745651
for _, configs := range hashes {

0 commit comments

Comments
 (0)