From 8e0ea4a56700afc652b13d7b7e52cc1c3b4233da Mon Sep 17 00:00:00 2001 From: Iain McGinniss <309153+iainmcgin@users.noreply.github.com> Date: Mon, 16 Mar 2026 23:43:21 +0000 Subject: [PATCH] Skip writing unchanged generated files to preserve mtimes buf generate previously wrote output files unconditionally, touching mtimes even when content was identical to what was already on disk. For mtime-based build systems (cargo, make, ninja), a no-op generate would cascade into unnecessary downstream rebuilds. This change replaces the unconditional storage.Copy in the directory output path with a compare-then-write loop. Files whose content matches what is already on disk are skipped, preserving their mtimes. Read errors during comparison (e.g. file does not exist, permissions) fall through to write, preserving existing behavior. Scope is limited to directory output; zip/jar output continues to write unconditionally. Interaction with --clean is already correct: clean deletes first, so comparison finds nothing and writes everything. Closes #3126 --- CHANGELOG.md | 1 + .../command/generate/generate_test.go | 20 +++ .../bufprotopluginos/response_writer.go | 41 +++++- .../bufprotopluginos/response_writer_test.go | 131 ++++++++++++++++++ 4 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 private/bufpkg/bufprotoplugin/bufprotopluginos/response_writer_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9845b4f1f7..c5cb029070 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Add support for `--rbs_out` as a `protoc_builtin` plugin (requires protoc v34.0+). - Add relevant links from CEL LSP hover documentation to either or +- `buf generate` now skips writing output files when the content matches what's already on disk, preserving modification times for mtime-based build systems like cargo and make. ## [v1.66.1] - 2026-03-09 diff --git a/cmd/buf/internal/command/generate/generate_test.go b/cmd/buf/internal/command/generate/generate_test.go index 2165d94324..5a8cc15538 100644 --- a/cmd/buf/internal/command/generate/generate_test.go +++ b/cmd/buf/internal/command/generate/generate_test.go @@ -25,6 +25,7 @@ import ( "path/filepath" "strings" "testing" + "time" "buf.build/go/app/appcmd" "buf.build/go/app/appcmd/appcmdtesting" @@ -371,6 +372,25 @@ func TestOutputFlag(t *testing.T) { } } +func TestSkipWriteWhenUnchanged(t *testing.T) { + t.Parallel() + tempDirPath := t.TempDir() + template := filepath.Join("testdata", "simple", "buf.gen.yaml") + input := filepath.Join("testdata", "simple") + outFile := filepath.Join(tempDirPath, "java", "a", "v1", "A.java") + + testRunSuccess(t, "--output", tempDirPath, "--template", template, input) + + past := time.Now().Add(-time.Hour) + require.NoError(t, os.Chtimes(outFile, past, past)) + + testRunSuccess(t, "--output", tempDirPath, "--template", template, input) + + info, err := os.Stat(outFile) + require.NoError(t, err) + require.Equal(t, past.Truncate(time.Second), info.ModTime().Truncate(time.Second)) +} + func TestProtoFileRefIncludePackageFiles(t *testing.T) { t.Parallel() tempDirPath := t.TempDir() diff --git a/private/bufpkg/bufprotoplugin/bufprotopluginos/response_writer.go b/private/bufpkg/bufprotoplugin/bufprotopluginos/response_writer.go index 579841f7ed..ec4ff29360 100644 --- a/private/bufpkg/bufprotoplugin/bufprotopluginos/response_writer.go +++ b/private/bufpkg/bufprotoplugin/bufprotopluginos/response_writer.go @@ -15,6 +15,7 @@ package bufprotopluginos import ( + "bytes" "context" "errors" "fmt" @@ -29,6 +30,7 @@ import ( "github.com/bufbuild/buf/private/pkg/storage/storagearchive" "github.com/bufbuild/buf/private/pkg/storage/storagemem" "github.com/bufbuild/buf/private/pkg/storage/storageos" + "github.com/bufbuild/buf/private/pkg/thread" "google.golang.org/protobuf/types/pluginpb" ) @@ -284,14 +286,45 @@ func (w *responseWriter) writeDirectory( if err != nil { return err } - if _, err := storage.Copy(ctx, readWriteBucket, osReadWriteBucket); err != nil { - return err - } - return nil + return w.copySkipUnchanged(ctx, readWriteBucket, osReadWriteBucket) }) return nil } +// copySkipUnchanged copies all paths from the source bucket to the destination, +// skipping any path whose content already matches what is on disk. This preserves +// mtimes for unchanged generated files so that mtime-based build systems do not +// rebuild unnecessarily. +func (w *responseWriter) copySkipUnchanged( + ctx context.Context, + from storage.ReadBucket, + to storage.ReadWriteBucket, +) error { + paths, err := storage.AllPaths(ctx, from, "") + if err != nil { + return err + } + jobs := make([]func(context.Context) error, len(paths)) + for i, path := range paths { + jobs[i] = func(ctx context.Context) error { + newData, err := storage.ReadPath(ctx, from, path) + if err != nil { + return err + } + existingData, err := storage.ReadPath(ctx, to, path) + if err == nil && bytes.Equal(existingData, newData) { + w.logger.DebugContext(ctx, "skipping unchanged generated file", slog.String("path", path)) + return nil + } + // Not-exist, read error, or content differs: fall through to write. + // We intentionally swallow read errors here; this comparison is an + // optimization and must not cause generate to fail. + return storage.PutPath(ctx, to, path, newData) + } + } + return thread.Parallelize(ctx, jobs) +} + type responseWriterOptions struct { createOutDirIfNotExists bool } diff --git a/private/bufpkg/bufprotoplugin/bufprotopluginos/response_writer_test.go b/private/bufpkg/bufprotoplugin/bufprotopluginos/response_writer_test.go new file mode 100644 index 0000000000..6126c9ac2a --- /dev/null +++ b/private/bufpkg/bufprotoplugin/bufprotopluginos/response_writer_test.go @@ -0,0 +1,131 @@ +// Copyright 2020-2026 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bufprotopluginos + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/bufbuild/buf/private/pkg/slogtestext" + "github.com/bufbuild/buf/private/pkg/storage/storageos" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/pluginpb" +) + +func TestResponseWriterSkipsUnchangedFile(t *testing.T) { + t.Parallel() + outDir := t.TempDir() + content := "package foo\n" + filePath := filepath.Join(outDir, "foo.go") + require.NoError(t, os.WriteFile(filePath, []byte(content), 0600)) + past := time.Now().Add(-time.Hour) + require.NoError(t, os.Chtimes(filePath, past, past)) + + runResponseWriter(t, outDir, newResponseFile("foo.go", content)) + + info, err := os.Stat(filePath) + require.NoError(t, err) + require.Equal(t, past.Truncate(time.Second), info.ModTime().Truncate(time.Second)) +} + +func TestResponseWriterWritesChangedFile(t *testing.T) { + t.Parallel() + outDir := t.TempDir() + filePath := filepath.Join(outDir, "foo.go") + require.NoError(t, os.WriteFile(filePath, []byte("package old\n"), 0600)) + past := time.Now().Add(-time.Hour) + require.NoError(t, os.Chtimes(filePath, past, past)) + + newContent := "package new\n" + runResponseWriter(t, outDir, newResponseFile("foo.go", newContent)) + + data, err := os.ReadFile(filePath) + require.NoError(t, err) + require.Equal(t, newContent, string(data)) + info, err := os.Stat(filePath) + require.NoError(t, err) + require.Greater(t, info.ModTime(), past) +} + +func TestResponseWriterWritesNewFile(t *testing.T) { + t.Parallel() + outDir := t.TempDir() + content := "package foo\n" + + runResponseWriter(t, outDir, newResponseFile("foo.go", content)) + + data, err := os.ReadFile(filepath.Join(outDir, "foo.go")) + require.NoError(t, err) + require.Equal(t, content, string(data)) +} + +func TestResponseWriterMixedFiles(t *testing.T) { + t.Parallel() + outDir := t.TempDir() + unchangedContent := "package unchanged\n" + unchangedPath := filepath.Join(outDir, "unchanged.go") + changedPath := filepath.Join(outDir, "changed.go") + newPath := filepath.Join(outDir, "new.go") + require.NoError(t, os.WriteFile(unchangedPath, []byte(unchangedContent), 0600)) + require.NoError(t, os.WriteFile(changedPath, []byte("package old\n"), 0600)) + past := time.Now().Add(-time.Hour) + require.NoError(t, os.Chtimes(unchangedPath, past, past)) + require.NoError(t, os.Chtimes(changedPath, past, past)) + + runResponseWriter(t, outDir, + newResponseFile("unchanged.go", unchangedContent), + newResponseFile("changed.go", "package changed\n"), + newResponseFile("new.go", "package new\n"), + ) + + unchangedInfo, err := os.Stat(unchangedPath) + require.NoError(t, err) + require.Equal(t, past.Truncate(time.Second), unchangedInfo.ModTime().Truncate(time.Second)) + + changedData, err := os.ReadFile(changedPath) + require.NoError(t, err) + require.Equal(t, "package changed\n", string(changedData)) + changedInfo, err := os.Stat(changedPath) + require.NoError(t, err) + require.Greater(t, changedInfo.ModTime(), past) + + newData, err := os.ReadFile(newPath) + require.NoError(t, err) + require.Equal(t, "package new\n", string(newData)) +} + +func runResponseWriter(t *testing.T, outDir string, files ...*pluginpb.CodeGeneratorResponse_File) { + t.Helper() + writer := NewResponseWriter( + slogtestext.NewLogger(t), + storageos.NewProvider(), + ResponseWriterWithCreateOutDirIfNotExists(), + ) + require.NoError(t, writer.AddResponse( + t.Context(), + &pluginpb.CodeGeneratorResponse{File: files}, + outDir, + )) + require.NoError(t, writer.Close()) +} + +func newResponseFile(name, content string) *pluginpb.CodeGeneratorResponse_File { + return &pluginpb.CodeGeneratorResponse_File{ + Name: &name, + Content: &content, + } +}