From 44e2b26a8748552a249ab9cce17cbde64736e5ef Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Jun 2023 13:10:58 +0200 Subject: [PATCH 1/3] pkg/epoch: replace some fmt.Sprintfs with strconv Teeny-tiny optimizations: BenchmarkSprintf-10 37735996 32.31 ns/op 0 B/op 0 allocs/op BenchmarkItoa-10 591945836 2.031 ns/op 0 B/op 0 allocs/op BenchmarkFormatUint-10 593701444 2.014 ns/op 0 B/op 0 allocs/op Signed-off-by: Sebastiaan van Stijn --- pkg/epoch/epoch.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/epoch/epoch.go b/pkg/epoch/epoch.go index 124e8edb5..14379a68e 100644 --- a/pkg/epoch/epoch.go +++ b/pkg/epoch/epoch.go @@ -60,10 +60,10 @@ func SourceDateEpochOrNow() time.Time { // SetSourceDateEpoch sets the SOURCE_DATE_EPOCH env var. func SetSourceDateEpoch(tm time.Time) { - os.Setenv(SourceDateEpochEnv, fmt.Sprintf("%d", tm.Unix())) + _ = os.Setenv(SourceDateEpochEnv, strconv.Itoa(int(tm.Unix()))) } // UnsetSourceDateEpoch unsets the SOURCE_DATE_EPOCH env var. func UnsetSourceDateEpoch() { - os.Unsetenv(SourceDateEpochEnv) + _ = os.Unsetenv(SourceDateEpochEnv) } From 9924e56f4278d111d08fe9bddf36bc34dd291e1b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Jun 2023 13:53:44 +0200 Subject: [PATCH 2/3] pkg/epoch: fix tests on macOS These tests were failing on my macOS; could be the precision issue (like on Windows), or just because they're "too fast". === RUN TestSourceDateEpoch/WithoutSourceDateEpoch epoch_test.go:51: Error Trace: /Users/thajeztah/go/src/github.com/containerd/containerd/pkg/epoch/epoch_test.go:51 Error: Should be true Test: TestSourceDateEpoch/WithoutSourceDateEpoch Messages: now: 2023-06-23 11:47:09.93118 +0000 UTC, v: 2023-06-23 11:47:09.93118 +0000 UTC This patch: - updates the rightAfter utility to allow the timestamps to be "equal" - updates the asserts to provide some details about the timestamps - uses UTC for the value we're comparing to, to match the timestamps that are generated. Signed-off-by: Sebastiaan van Stijn --- pkg/epoch/epoch_test.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/epoch/epoch_test.go b/pkg/epoch/epoch_test.go index f25c77eb2..27535b78d 100644 --- a/pkg/epoch/epoch_test.go +++ b/pkg/epoch/epoch_test.go @@ -26,11 +26,15 @@ import ( ) func rightAfter(t1, t2 time.Time) bool { + if t2.Equal(t1) { + return true + } + threshold := 10 * time.Millisecond if runtime.GOOS == "windows" { // Low timer resolution on Windows - return (t2.After(t1) && t2.Before(t1.Add(100*time.Millisecond))) || t2.Equal(t1) + threshold *= 10 } - return t2.After(t1) && t2.Before(t1.Add(10*time.Millisecond)) + return t2.After(t1) && t2.Before(t1.Add(threshold)) } func TestSourceDateEpoch(t *testing.T) { @@ -46,9 +50,9 @@ func TestSourceDateEpoch(t *testing.T) { require.NoError(t, err) require.Nil(t, vp) - now := time.Now() + now := time.Now().UTC() v := SourceDateEpochOrNow() - require.True(t, rightAfter(now, v)) + require.True(t, rightAfter(now, v), "now: %s, v: %s", now, v) }) t.Run("WithEmptySourceDateEpoch", func(t *testing.T) { @@ -58,9 +62,9 @@ func TestSourceDateEpoch(t *testing.T) { require.NoError(t, err) require.Nil(t, vp) - now := time.Now() + now := time.Now().UTC() v := SourceDateEpochOrNow() - require.True(t, rightAfter(now, v)) + require.True(t, rightAfter(now, v), "now: %s, v: %s", now, v) }) t.Run("WithSourceDateEpoch", func(t *testing.T) { @@ -85,8 +89,8 @@ func TestSourceDateEpoch(t *testing.T) { require.ErrorContains(t, err, "invalid SOURCE_DATE_EPOCH value") require.Nil(t, vp) - now := time.Now() + now := time.Now().UTC() v := SourceDateEpochOrNow() - require.True(t, rightAfter(now, v)) + require.True(t, rightAfter(now, v), "now: %s, v: %s", now, v) }) } From 8760b8717459a8cdac11e71fbfaf43af3452c45d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 23 Jun 2023 14:38:36 +0200 Subject: [PATCH 3/3] pkg/epoch: extract parsing SOURCE_DATE_EPOCH to a function This introduces a ParseSourceDateEpoch function, which can be used to parse "SOURCE_DATE_EPOCH" values for situations where those values are not passed through an env-var (or the env-var has been read through other means). Signed-off-by: Sebastiaan van Stijn --- pkg/epoch/epoch.go | 21 +++++++++++++++++---- pkg/epoch/epoch_test.go | 22 +++++++++++++++++++--- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/pkg/epoch/epoch.go b/pkg/epoch/epoch.go index 14379a68e..1e4e06c20 100644 --- a/pkg/epoch/epoch.go +++ b/pkg/epoch/epoch.go @@ -37,12 +37,11 @@ func SourceDateEpoch() (*time.Time, error) { if !ok || v == "" { return nil, nil // not an error } - i64, err := strconv.ParseInt(v, 10, 64) + t, err := ParseSourceDateEpoch(v) if err != nil { - return nil, fmt.Errorf("invalid %s value %q: %w", SourceDateEpochEnv, v, err) + return nil, fmt.Errorf("invalid %s value: %w", SourceDateEpochEnv, err) } - unix := time.Unix(i64, 0).UTC() - return &unix, nil + return t, nil } // SourceDateEpochOrNow returns the SOURCE_DATE_EPOCH time if available, @@ -58,6 +57,20 @@ func SourceDateEpochOrNow() time.Time { return time.Now().UTC() } +// ParseSourceDateEpoch parses the given source date epoch, as *time.Time. +// It returns an error if sourceDateEpoch is empty or not well-formatted. +func ParseSourceDateEpoch(sourceDateEpoch string) (*time.Time, error) { + if sourceDateEpoch == "" { + return nil, fmt.Errorf("value is empty") + } + i64, err := strconv.ParseInt(sourceDateEpoch, 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid value: %w", err) + } + unix := time.Unix(i64, 0).UTC() + return &unix, nil +} + // SetSourceDateEpoch sets the SOURCE_DATE_EPOCH env var. func SetSourceDateEpoch(tm time.Time) { _ = os.Setenv(SourceDateEpochEnv, strconv.Itoa(int(tm.Unix()))) diff --git a/pkg/epoch/epoch_test.go b/pkg/epoch/epoch_test.go index 27535b78d..5b3cdb8d1 100644 --- a/pkg/epoch/epoch_test.go +++ b/pkg/epoch/epoch_test.go @@ -19,6 +19,7 @@ package epoch import ( "os" "runtime" + "strconv" "testing" "time" @@ -56,19 +57,25 @@ func TestSourceDateEpoch(t *testing.T) { }) t.Run("WithEmptySourceDateEpoch", func(t *testing.T) { - t.Setenv(SourceDateEpochEnv, "") + const emptyValue = "" + t.Setenv(SourceDateEpochEnv, emptyValue) vp, err := SourceDateEpoch() require.NoError(t, err) require.Nil(t, vp) + vp, err = ParseSourceDateEpoch(emptyValue) + require.Error(t, err, "value is empty") + require.Nil(t, vp) + now := time.Now().UTC() v := SourceDateEpochOrNow() require.True(t, rightAfter(now, v), "now: %s, v: %s", now, v) }) t.Run("WithSourceDateEpoch", func(t *testing.T) { - sourceDateEpoch, err := time.Parse(time.RFC3339, "2022-01-23T12:34:56Z") + const rfc3339Str = "2022-01-23T12:34:56Z" + sourceDateEpoch, err := time.Parse(time.RFC3339, rfc3339Str) require.NoError(t, err) SetSourceDateEpoch(sourceDateEpoch) @@ -76,6 +83,10 @@ func TestSourceDateEpoch(t *testing.T) { vp, err := SourceDateEpoch() require.NoError(t, err) + require.True(t, vp.Equal(sourceDateEpoch.UTC())) + + vp, err = ParseSourceDateEpoch(strconv.Itoa(int(sourceDateEpoch.Unix()))) + require.NoError(t, err) require.True(t, vp.Equal(sourceDateEpoch)) v := SourceDateEpochOrNow() @@ -83,12 +94,17 @@ func TestSourceDateEpoch(t *testing.T) { }) t.Run("WithInvalidSourceDateEpoch", func(t *testing.T) { - t.Setenv(SourceDateEpochEnv, "foo") + const invalidValue = "foo" + t.Setenv(SourceDateEpochEnv, invalidValue) vp, err := SourceDateEpoch() require.ErrorContains(t, err, "invalid SOURCE_DATE_EPOCH value") require.Nil(t, vp) + vp, err = ParseSourceDateEpoch(invalidValue) + require.ErrorContains(t, err, "invalid value:") + require.Nil(t, vp) + now := time.Now().UTC() v := SourceDateEpochOrNow() require.True(t, rightAfter(now, v), "now: %s, v: %s", now, v)