From 65b1a636a9d7447fa4d6253e6634ce175c6c8ce6 Mon Sep 17 00:00:00 2001 From: creatorHead Date: Wed, 29 Apr 2026 14:00:07 +0530 Subject: [PATCH] fix: LICENSE file no longer updated on every run when no headers changed anyFileUpdated was unconditionally set to true in non-plan mode due to a tautological condition, causing the LICENSE year to be bumped on every run even when all source file headers were already correct. This triggered spurious pre-commit hook failures. Fix: teach addlicense.Run to return whether it actually wrote any files, and use that signal instead of the always-true condition. --- addlicense/main.go | 44 +++++++++++++++++++++++------------------ addlicense/main_test.go | 2 +- cmd/headers.go | 10 +++++----- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/addlicense/main.go b/addlicense/main.go index e064378..fe7cf93 100644 --- a/addlicense/main.go +++ b/addlicense/main.go @@ -28,6 +28,7 @@ import ( "regexp" "runtime" "strings" + "sync/atomic" "text/template" "time" @@ -138,7 +139,7 @@ func main() { logger := log.Default() // real main - err := Run( + _, err := Run( ignorePatterns, spdx, data, @@ -187,33 +188,37 @@ func Run( checkonly bool, patterns []string, logger *log.Logger, -) error { +) (bool, error) { // verify that all ignorePatterns are valid err := validatePatterns(ignorePatternList) if err != nil { - return err + return false, err } ignorePatterns = ignorePatternList tpl, err := fetchTemplate(license.SPDXID, licenseFileOverride, spdx) if err != nil { - return err + return false, err } t, err := template.New("").Parse(tpl) if err != nil { - return err + return false, err } // process at most 1000 files in parallel ch := make(chan *file, 1000) done := make(chan struct{}) var out error + var anyModified atomic.Bool go func() { var wg errgroup.Group for f := range ch { f := f // https://golang.org/doc/faq#closures_and_goroutines wg.Go(func() error { - err := processFile(f, t, license, checkonly, verbose, logger) + modified, err := processFile(f, t, license, checkonly, verbose, logger) + if modified { + anyModified.Store(true) + } return err }) } @@ -223,52 +228,52 @@ func Run( for _, d := range patterns { if err := walk(ch, d, logger); err != nil { - return err + return false, err } } close(ch) <-done - return out + return anyModified.Load(), out } -func processFile(f *file, t *template.Template, license LicenseData, checkonly bool, verbose bool, logger *log.Logger) error { +func processFile(f *file, t *template.Template, license LicenseData, checkonly bool, verbose bool, logger *log.Logger) (bool, error) { if checkonly { // Check if file extension is known lic, err := licenseHeader(f.path, t, license) if err != nil { logger.Printf("%s: %v", f.path, err) - return err + return false, err } if lic == nil { // Unknown fileExtension - return nil + return false, nil } // Check if file has a license hasLicense, err := fileHasLicense(f.path) if err != nil { logger.Printf("%s: %v", f.path, err) - return err + return false, err } if !hasLicense { logger.Printf("%s\n", f.path) - return errors.New("missing license header") + return false, errors.New("missing license header") } // Also check if existing files would need copyright holder updates wouldUpdate, err := wouldUpdateLicenseHolder(f.path, license) if err != nil { logger.Printf("%s: %v", f.path, err) - return err + return false, err } if wouldUpdate { logger.Printf("%s (would update copyright holder)\n", f.path) - return errors.New("copyright holder would be updated") + return false, errors.New("copyright holder would be updated") } } else { // First, try to add a license if missing modified, err := addLicense(f.path, f.mode, t, license) if err != nil { logger.Printf("%s: %v", f.path, err) - return err + return false, err } // If file wasn't modified (already had a license), try to update the holder @@ -276,17 +281,18 @@ func processFile(f *file, t *template.Template, license LicenseData, checkonly b updated, err := updateLicenseHolder(f.path, f.mode, license) if err != nil { logger.Printf("%s: %v", f.path, err) - return err + return false, err } if updated { - logger.Printf("%s (copyright holder updated)", f.path) + return true, nil } } else { logger.Printf("%s (license header added)", f.path) + return true, nil } } - return nil + return false, nil } type file struct { diff --git a/addlicense/main_test.go b/addlicense/main_test.go index 2e1a452..52b6c2c 100644 --- a/addlicense/main_test.go +++ b/addlicense/main_test.go @@ -1001,7 +1001,7 @@ func TestBrokenSymlinkSkipped(t *testing.T) { SPDXID: "Apache-2.0", } - err := Run([]string{}, "off", licenseData, "", false, false, []string{tmp}, logger) + _, err := Run([]string{}, "off", licenseData, "", false, false, []string{tmp}, logger) if err != nil { t.Fatalf("Run returned error on broken symlink: %v", err) } diff --git a/cmd/headers.go b/cmd/headers.go index d5f00f5..eb0bf67 100644 --- a/cmd/headers.go +++ b/cmd/headers.go @@ -145,7 +145,7 @@ config, see the "copywrite init" command.`, if plan && updatedCount > 0 { err = fmt.Errorf("[DRY RUN] %d file(s) would be updated with new copyright years", updatedCount) } - runErr := addlicense.Run(ignoredPatterns, "only", licenseData, "", verbose, plan, []string{"."}, stdcliLogger) + filesAdded, runErr := addlicense.Run(ignoredPatterns, "only", licenseData, "", verbose, plan, []string{"."}, stdcliLogger) if err != nil && runErr != nil { err = fmt.Errorf("%v; %v", err, runErr) } else if err == nil { @@ -153,10 +153,10 @@ config, see the "copywrite init" command.`, } gha.EndGroup() - // STEP 4: Update LICENSE file if any files were modified (either updated or added headers) - // In plan mode: if addlicense found missing headers (returns error), assume files would be modified - // In normal mode: if addlicense succeeded, assume files were modified - if runErr != nil || (!plan && runErr == nil) { + // STEP 4: Update LICENSE file if any files were actually modified (either updated or added headers) + // In plan mode: runErr != nil means headers would be added + // In normal mode: filesAdded tracks whether addlicense actually wrote any files + if runErr != nil || filesAdded { anyFileUpdated = true }