From fc5bd21e28fe8beee3e0f544a30f9d9552250901 Mon Sep 17 00:00:00 2001 From: nielash Date: Sun, 10 Aug 2025 21:09:45 -0400 Subject: [PATCH] bisync: fix time.Local data race on tests - fixes #8272 Before this change, the bisync tests were directly setting the time.Local variable to UTC. The reason for overriding the time zone on the tests is to make them deterministic regardless of where in the world the user happens to be. There are some goldenized strings which have the time zone hard-coded and would result in a miscompare failure outside of that time zone. However, mutating the time.Local variable is not the right way to do this, as OP correctly pointed out on #8272. Setting the TZ environment variable from within the code was also not an ideal solution because, while it worked on unix, it did not work on Windows. See https://github.com/golang/go/blob/fbac94a79998d4730a58592f0634fa8a39d8b9fb/src/time/zoneinfo.go#L79-L80 This change fixes the issue by defining a new bisync.LogTZ setting for use when printing timestamps in /cmd/bisync/resolve.go. We override this on the tests instead of time.Local. --- cmd/bisync/bisync_test.go | 2 +- cmd/bisync/listing.go | 6 +++++- cmd/bisync/resolve.go | 12 ++++++------ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/cmd/bisync/bisync_test.go b/cmd/bisync/bisync_test.go index 2cee02f28..b4ed0fe93 100644 --- a/cmd/bisync/bisync_test.go +++ b/cmd/bisync/bisync_test.go @@ -226,6 +226,7 @@ var color = bisync.Color // TestMain drives the tests func TestMain(m *testing.M) { + bisync.LogTZ = time.UTC fstest.TestMain(m) } @@ -277,7 +278,6 @@ func testBisync(t *testing.T, path1, path2 string) { ci.RefreshTimes = true } bisync.Colors = true - time.Local = bisync.TZ ci.FsCacheExpireDuration = fs.Duration(5 * time.Hour) baseDir, err := os.Getwd() diff --git a/cmd/bisync/listing.go b/cmd/bisync/listing.go index 7451a5cbb..1aa1e9a62 100644 --- a/cmd/bisync/listing.go +++ b/cmd/bisync/listing.go @@ -42,10 +42,14 @@ var lineRegex = regexp.MustCompile(`^(\S) +(-?\d+) (\S+) (\S+) (\d{4}-\d\d-\d\dT // timeFormat defines time format used in listings const timeFormat = "2006-01-02T15:04:05.000000000-0700" -// TZ defines time zone used in listings var ( + // TZ defines time zone used in listings TZ = time.UTC tzLocal = false + + // LogTZ defines time zone used in logs (which may be different than that used in listings). + // time.Local by default, but we force UTC on tests to make them deterministic regardless of tester's location. + LogTZ = time.Local ) // fileInfo describes a file diff --git a/cmd/bisync/resolve.go b/cmd/bisync/resolve.go index b3e5d8048..21624580d 100644 --- a/cmd/bisync/resolve.go +++ b/cmd/bisync/resolve.go @@ -380,26 +380,26 @@ func (b *bisyncRun) resolveNewerOlder(t1, t2 time.Time, remote1, remote2 string, } if t1.After(t2) { if prefer == PreferNewer { - fs.Infof(remote1, "Path1 is newer. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t1.Sub(t2)) + fs.Infof(remote1, "Path1 is newer. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t1.Sub(t2)) return 1 } else if prefer == PreferOlder { - fs.Infof(remote1, "Path2 is older. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t1.Sub(t2)) + fs.Infof(remote1, "Path2 is older. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t1.Sub(t2)) return 2 } } else if t1.Before(t2) { if prefer == PreferNewer { - fs.Infof(remote1, "Path2 is newer. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t2.Sub(t1)) + fs.Infof(remote1, "Path2 is newer. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t2.Sub(t1)) return 2 } else if prefer == PreferOlder { - fs.Infof(remote1, "Path1 is older. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t2.Sub(t1)) + fs.Infof(remote1, "Path1 is older. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t2.Sub(t1)) return 1 } } if t1.Equal(t2) { - fs.Infof(remote1, "Winner cannot be determined as times are equal. Path1: %v, Path2: %v, Difference: %s", t1.Local(), t2.Local(), t2.Sub(t1)) + fs.Infof(remote1, "Winner cannot be determined as times are equal. Path1: %v, Path2: %v, Difference: %s", t1.In(LogTZ), t2.In(LogTZ), t2.Sub(t1)) return 0 } - fs.Errorf(remote1, "Winner cannot be determined. Path1: %v, Path2: %v", t1.Local(), t2.Local()) // shouldn't happen unless prefer is of wrong type + fs.Errorf(remote1, "Winner cannot be determined. Path1: %v, Path2: %v", t1.In(LogTZ), t2.In(LogTZ)) // shouldn't happen unless prefer is of wrong type return 0 }