From 4db1daeada5852bc3832043b9c63a6da9f78779b Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 15 Jan 2025 11:20:26 +0000 Subject: [PATCH] Rename CopyIfChanged to WriteIfNeeded and clarify comments --- internal/file/file.go | 14 +++++++------- internal/file/file_test.go | 16 +++++++++------- internal/legacy/legacy.go | 4 ++-- internal/legacy/php_unix.go | 2 +- internal/legacy/php_windows.go | 2 +- 5 files changed, 20 insertions(+), 18 deletions(-) diff --git a/internal/file/file.go b/internal/file/file.go index 44e840a..57d68e3 100644 --- a/internal/file/file.go +++ b/internal/file/file.go @@ -8,9 +8,10 @@ import ( "os" ) -// CopyIfChanged copies source data to a destination filename if it has changed. -func CopyIfChanged(destFilename string, source []byte, perm os.FileMode) error { - matches, err := probablyMatches(destFilename, source) +// WriteIfNeeded writes data to a destination file, only if the file does not exist or if it was partially written. +// To save time, it only checks that the file size is correct, and then matches the end of its contents (up to 32KB). +func WriteIfNeeded(destFilename string, source []byte, perm os.FileMode) error { + matches, err := probablyMatches(destFilename, source, 32*1024) if err != nil || matches { return err } @@ -27,9 +28,8 @@ func Write(path string, content []byte, fileMode fs.FileMode) error { return os.Rename(tmpFile, path) } -// probablyMatches checks, heuristically, if a file matches source data. -// To save time, it only compares the file size and the end of its contents (up to 32KB). -func probablyMatches(filename string, data []byte) (bool, error) { +// probablyMatches checks if a file exists and matches the end of source data (up to checkSize bytes). +func probablyMatches(filename string, data []byte, checkSize int) (bool, error) { f, err := os.Open(filename) if err != nil { if errors.Is(err, fs.ErrNotExist) { @@ -47,7 +47,7 @@ func probablyMatches(filename string, data []byte) (bool, error) { return false, nil } - return matchEndOfFile(f, data, 32*1024) + return matchEndOfFile(f, data, checkSize) } func matchEndOfFile(f *os.File, b []byte, size int) (bool, error) { diff --git a/internal/file/file_test.go b/internal/file/file_test.go index afd2274..4874ad0 100644 --- a/internal/file/file_test.go +++ b/internal/file/file_test.go @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestCopyIfChanged(t *testing.T) { +func TestWriteIfNeeded(t *testing.T) { largeContentLength := 128 * 1024 largeContent := make([]byte, largeContentLength) largeContent[0] = 'f' @@ -50,24 +50,26 @@ func TestCopyIfChanged(t *testing.T) { time.Sleep(time.Millisecond * 5) } - var modTimeBeforeCopy time.Time + var modTimeBefore time.Time stat, err := os.Stat(destFile) if c.initialData == nil { require.True(t, os.IsNotExist(err)) } else { require.NoError(t, err) - modTimeBeforeCopy = stat.ModTime() + modTimeBefore = stat.ModTime() } - err = CopyIfChanged(destFile, c.sourceData, 0o600) + err = WriteIfNeeded(destFile, c.sourceData, 0o600) require.NoError(t, err) - statAfterCopy, err := os.Stat(destFile) + statAfter, err := os.Stat(destFile) require.NoError(t, err) + modTimeAfter := statAfter.ModTime() + if c.expectWrite { - assert.Greater(t, statAfterCopy.ModTime().Truncate(time.Millisecond), modTimeBeforeCopy.Truncate(time.Millisecond)) + assert.Greater(t, modTimeAfter.Truncate(time.Millisecond), modTimeBefore.Truncate(time.Millisecond)) } else { - assert.Equal(t, modTimeBeforeCopy.Truncate(time.Millisecond), statAfterCopy.ModTime().Truncate(time.Millisecond)) + assert.Equal(t, modTimeBefore.Truncate(time.Millisecond), modTimeAfter.Truncate(time.Millisecond)) } data, err := os.ReadFile(destFile) diff --git a/internal/legacy/legacy.go b/internal/legacy/legacy.go index 9c0cdeb..ffe2916 100644 --- a/internal/legacy/legacy.go +++ b/internal/legacy/legacy.go @@ -80,7 +80,7 @@ func (c *CLIWrapper) init() error { g := errgroup.Group{} g.Go(func() error { - if err := file.CopyIfChanged(c.pharPath(cacheDir), phar, 0o644); err != nil { + if err := file.WriteIfNeeded(c.pharPath(cacheDir), phar, 0o644); err != nil { return fmt.Errorf("could not copy phar file: %w", err) } return nil @@ -90,7 +90,7 @@ func (c *CLIWrapper) init() error { if err != nil { return fmt.Errorf("could not load config for checking: %w", err) } - if err := file.CopyIfChanged(filepath.Join(cacheDir, configBasename), configContent, 0o644); err != nil { + if err := file.WriteIfNeeded(filepath.Join(cacheDir, configBasename), configContent, 0o644); err != nil { return fmt.Errorf("could not write config: %w", err) } return nil diff --git a/internal/legacy/php_unix.go b/internal/legacy/php_unix.go index 70d7a7d..1316f18 100644 --- a/internal/legacy/php_unix.go +++ b/internal/legacy/php_unix.go @@ -11,7 +11,7 @@ import ( // copyPHP to destination, if it does not exist func (c *CLIWrapper) copyPHP(cacheDir string) error { - return file.CopyIfChanged(c.phpPath(cacheDir), phpCLI, 0o755) + return file.WriteIfNeeded(c.phpPath(cacheDir), phpCLI, 0o755) } // phpPath returns the path to the temporary PHP-CLI binary diff --git a/internal/legacy/php_windows.go b/internal/legacy/php_windows.go index 5774d81..8ca08f0 100644 --- a/internal/legacy/php_windows.go +++ b/internal/legacy/php_windows.go @@ -42,7 +42,7 @@ func (c *CLIWrapper) copyPHP(cacheDir string) error { return err } - if err := file.CopyIfChanged(filepath.Join(destDir, "extras", "cacert.pem"), caCert, 0o644); err != nil { + if err := file.WriteIfNeeded(filepath.Join(destDir, "extras", "cacert.pem"), caCert, 0o644); err != nil { return err }