From e0d8c4d30c10e0dbe07196445733852f391d955e Mon Sep 17 00:00:00 2001
From: Noam Dolovich <noam.tzvi.dolovich@gmail.com>
Date: Thu, 25 Apr 2024 21:47:13 +0300
Subject: [PATCH] cue/cmd: avoid overwriting formatted files in cue fmt

Currently, `cue fmt` will always write to cue files, regardless of
whether they are formatted or not. In the case of formatted files,
their contents will remain exactly the same, but their modification
time will change.
This is problematic for tools that rely on the mod time.

To fix this, we buffer the original/formatted bytes. If the result
is exactly the same, the file is not written to.

Fixes #1731, #363

Signed-off-by: Noam Dolovich <noam.tzvi.dolovich@gmail.com>
Change-Id: Iea55c1eb9e99c1dcba301194857a92dad1faa9ad
---
 cmd/cue/cmd/fmt.go                         | 59 +++++++++++++---------
 cmd/cue/cmd/script_test.go                 | 12 +++++
 cmd/cue/cmd/testdata/script/fmt_time.txtar | 10 ++++
 3 files changed, 56 insertions(+), 25 deletions(-)
 create mode 100644 cmd/cue/cmd/testdata/script/fmt_time.txtar

diff --git a/cmd/cue/cmd/fmt.go b/cmd/cue/cmd/fmt.go
index d9200a4dff8..46f7e676060 100644
--- a/cmd/cue/cmd/fmt.go
+++ b/cmd/cue/cmd/fmt.go
@@ -88,16 +88,14 @@ func newFmtCmd(c *Command) *cobra.Command {
 					// When using --check and --diff, we need to buffer the input and output bytes to compare them.
 					var original []byte
 					var formatted bytes.Buffer
-					if doDiff || check {
-						if bs, ok := file.Source.([]byte); ok {
-							original = bs
-						} else {
-							original, err = source.ReadAll(file.Filename, file.Source)
-							exitOnErr(cmd, err, true)
-							file.Source = original
-						}
-						cfg.Out = &formatted
+					if bs, ok := file.Source.([]byte); ok {
+						original = bs
+					} else {
+						original, err = source.ReadAll(file.Filename, file.Source)
+						exitOnErr(cmd, err, true)
+						file.Source = original
 					}
+					cfg.Out = &formatted
 
 					var files []*ast.File
 					d := encoding.NewDecoder(cmd.ctx, file, &cfg)
@@ -130,24 +128,35 @@ func newFmtCmd(c *Command) *cobra.Command {
 						exitOnErr(cmd, err, true)
 					}
 
-					if (doDiff || check) && !bytes.Equal(formatted.Bytes(), original) {
-						foundBadlyFormatted = true
-						var path string
-						if file.Filename != "-" {
-							f := file.Filename
-							path, err = filepath.Rel(cwd, f)
-							if err != nil {
-								path = f
-							}
-						} else {
-							path = "<standard input>"
+					if bytes.Equal(formatted.Bytes(), original) {
+						continue
+					}
+
+					foundBadlyFormatted = true
+					var path string
+					if file.Filename != "-" {
+						f := file.Filename
+						path, err = filepath.Rel(cwd, f)
+						if err != nil {
+							path = f
 						}
+					} else {
+						path = "<standard input>"
+					}
 
-						if doDiff {
-							d := diff.Diff(path+".orig", original, path, formatted.Bytes())
-							fmt.Fprintln(stdout, string(d))
-						} else {
-							fmt.Fprintln(stdout, path)
+					switch {
+					case doDiff:
+						d := diff.Diff(path+".orig", original, path, formatted.Bytes())
+						fmt.Fprintln(stdout, string(d))
+					case check:
+						fmt.Fprintln(stdout, path)
+					case file.Filename == "-":
+						if _, err := fmt.Fprint(stdout, formatted.String()); err != nil {
+							exitOnErr(cmd, err, false)
+						}
+					default:
+						if err := os.WriteFile(file.Filename, formatted.Bytes(), 0644); err != nil {
+							exitOnErr(cmd, err, false)
 						}
 					}
 				}
diff --git a/cmd/cue/cmd/script_test.go b/cmd/cue/cmd/script_test.go
index b2c438cb452..7a4a126ffad 100644
--- a/cmd/cue/cmd/script_test.go
+++ b/cmd/cue/cmd/script_test.go
@@ -116,6 +116,18 @@ func TestScript(t *testing.T) {
 					ts.Check(os.WriteFile(path, []byte(data), 0o666))
 				}
 			},
+			// mod-time prints the modification time of a file to stdout.
+			// The time is displayed as nanoseconds since the Unix epoch.
+			"mod-time": func(ts *testscript.TestScript, neg bool, args []string) {
+				if neg || len(args) != 1 {
+					ts.Fatalf("usage: mod-time PATH")
+				}
+				path := ts.MkAbs(args[0])
+				fi, err := os.Stat(path)
+				ts.Check(err)
+				_, err = fmt.Fprint(ts.Stdout(), fi.ModTime().UnixNano())
+				ts.Check(err)
+			},
 			// get-manifest writes the manifest for a given reference within an OCI
 			// registry to a file in JSON format.
 			"get-manifest": func(ts *testscript.TestScript, neg bool, args []string) {
diff --git a/cmd/cue/cmd/testdata/script/fmt_time.txtar b/cmd/cue/cmd/testdata/script/fmt_time.txtar
new file mode 100644
index 00000000000..03c9bdcef8c
--- /dev/null
+++ b/cmd/cue/cmd/testdata/script/fmt_time.txtar
@@ -0,0 +1,10 @@
+# Verify that cue fmt does not write to files
+# if they are already formatted
+mod-time formatted.cue
+cp stdout mod-time.txt
+exec cue fmt formatted.cue
+mod-time formatted.cue
+cmp stdout mod-time.txt
+
+-- formatted.cue --
+a: 1