Don't unecessarily change the package.lock and package.yaml files (#54)

Makefile

@@ -2,6 +2,7 @@ BUILD_DIR := build
 
 GO_PACKAGE := github.com/toitlang/tpkg
 
+# When rebuilding gold files, the TOITC_PATH must be changed to be absolute.
 TOITC_PATH ?= toit.compile
 TOITLSP_PATH ?= toit.lsp
 TOITVM_PATH ?= toit.run

pkg/tpkg/lock_file.go

@@ -17,7 +17,7 @@ package tpkg
 
 import (
 	"fmt"
-	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -76,7 +76,7 @@ func (pe PackageEntry) Validate(ui UI) error {
 // ReadLockFile reads the lock-file at the given path.
 func ReadLockFile(path string) (*LockFile, error) {
 	// TODO(florian): should we validate the file here?
-	b, err := ioutil.ReadFile(path)
+	b, err := os.ReadFile(path)
 	if err != nil {
 		return nil, err
 	}
@@ -91,11 +91,16 @@ func ReadLockFile(path string) (*LockFile, error) {
 }
 
 func (lf *LockFile) WriteToFile() error {
+	// Write the YAML to memory first, and then compare it with any
+	// existing file.
+	// We don't want to touch files if they don't change.
+
 	b, err := yaml.Marshal(lf)
 	if err != nil {
 		return err
 	}
-	return ioutil.WriteFile(lf.path, b, 0644)
+
+	return writeFileIfChanged(lf.path, b)
 }
 
 // PrintLockFile prints the contents of the lock file for the current project.

pkg/tpkg/spec.go

@@ -16,9 +16,9 @@
 package tpkg
 
 import (
+	"bytes"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -111,7 +111,7 @@ func (s *Spec) Validate(ui UI) error {
 
 func (s *Spec) ParseFile(filename string, ui UI) error {
 	s.path = filename
-	b, err := ioutil.ReadFile(filename)
+	b, err := os.ReadFile(filename)
 	if err != nil {
 		return err
 	}
@@ -134,18 +134,15 @@ func (s *Spec) WriteYAML(writer io.Writer) error {
 }
 
 func (s *Spec) WriteToFile() error {
-	file, err := os.Create(s.path)
+	// Write the YAML to memory first, and then compare it with any
+	// existing file.
+	// We don't want to touch files if they don't change.
+	var b bytes.Buffer
+	err := s.WriteYAML(&b)
 	if err != nil {
 		return err
 	}
-	defer func() {
-		e := file.Close()
-		if e != nil {
-			err = e
-		}
-	}()
-
-	return s.WriteYAML(file)
+	return writeFileIfChanged(s.path, b.Bytes())
 }
 
 // BuildLockFile generates a lock file using the given solution.

pkg/tpkg/ui.go

@@ -59,6 +59,19 @@ func (ui fmtUI) ReportInfo(format string, a ...interface{}) {
 	fmt.Printf("Info: "+format+"\n", a...)
 }
 
+// nullUI implements a UI that does nothing.
+type nullUI struct{}
+
+func (ui nullUI) ReportError(format string, a ...interface{}) error {
+	return ErrAlreadyReported
+}
+
+func (ui nullUI) ReportWarning(format string, a ...interface{}) {
+}
+
+func (ui nullUI) ReportInfo(format string, a ...interface{}) {
+}
+
 var (
 	// ErrAlreadyReported can be used to signal that an error has
 	// been reported, and that no further action needs to be taken.

pkg/tpkg/util.go

@@ -16,6 +16,7 @@
 package tpkg
 
 import (
+	"bytes"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -74,3 +75,28 @@ func sdkConstraintToMinSDK(sdk string) (*version.Version, error) {
 	}
 	return minSDK, nil
 }
+
+func writeFileIfChanged(path string, content []byte) error {
+	// Check whether the file already exists and has the same content.
+	// We don't want to touch files if they don't change.
+	if _, err := os.Stat(path); err == nil {
+		oldContent, err := os.ReadFile(path)
+		if err == nil && bytes.Equal(oldContent, content) {
+			return nil
+		}
+	}
+
+	file, err := os.Create(path)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		e := file.Close()
+		if e != nil {
+			err = e
+		}
+	}()
+
+	_, err = file.Write(content)
+	return err
+}

tests/assets/pkg/InstallNoMTimeChange/gold/test.gold

@@ -0,0 +1,9 @@
+// Add registry so we can find packages.
+===================
+pkg registry add --local test-reg <TEST>/registry_git_pkgs
+Exit Code: 0
+===================
+// Just 'install' doesn't add the missing dependencies.
+===================
+pkg install
+Exit Code: 0

tests/assets/pkg/InstallNoMTimeChange/main.toit

@@ -0,0 +1,6 @@
+// Copyright (C) 2024 Toitware ApS. All rights reserved.
+
+import pkg1
+
+main:
+  print pkg1.identify

tests/assets/pkg/InstallNoMTimeChange/package.lock

@@ -0,0 +1,7 @@
+prefixes:
+  pkg1: pkg1
+
+packages:
+  pkg1:
+    url: '<[*TEST_GIT_DIR_ESCAPE*]>/git_pkgs/pkg1'
+    version: 1.0.0

tests/pkg_test.go

@@ -290,7 +290,7 @@ func copyRec(t *tedi.T, testDir string, sourceDir string, targetDir string) {
 			}
 			return unzip(p, filepath.Dir(target))
 		}
-		data, err := ioutil.ReadFile(p)
+		data, err := os.ReadFile(p)
 		if err != nil {
 			return err
 		}
@@ -300,7 +300,7 @@ func copyRec(t *tedi.T, testDir string, sourceDir string, targetDir string) {
 		data = bytes.ReplaceAll(data, []byte(testDirGitPattern), []byte(testDirGitURL))
 		escapedTestDirGitURL := string(compiler.ToURIPath(testDirGitURL))
 		data = bytes.ReplaceAll(data, []byte(testDirGitEscapePattern), []byte(escapedTestDirGitURL))
-		return ioutil.WriteFile(target, data, info.Mode().Perm())
+		return os.WriteFile(target, data, info.Mode().Perm())
 	})
 	require.NoError(t, err)
 }
@@ -515,7 +515,7 @@ func (pt PkgTest) updateGold(name string, newGold string) {
 	}
 	require.NoError(pt.t, err)
 	goldPath := filepath.Join(goldDir, name+".gold")
-	oldBytes, err := ioutil.ReadFile(goldPath)
+	oldBytes, err := os.ReadFile(goldPath)
 	if err == nil {
 		oldGold := string(oldBytes)
 		if string(oldBytes) != newGold {
@@ -524,7 +524,7 @@ func (pt PkgTest) updateGold(name string, newGold string) {
 	} else if os.IsNotExist(err) {
 		fmt.Printf("Creating new gold %s with content:\n%s", goldPath, newGold)
 	}
-	err = ioutil.WriteFile(goldPath, []byte(newGold), 0644)
+	err = os.WriteFile(goldPath, []byte(newGold), 0644)
 	require.NoError(pt.t, err)
 }
 
@@ -534,7 +534,7 @@ func (pt PkgTest) checkGold(name string, actual string) {
 		return
 	}
 	goldPath := filepath.Join(pt.dir, "gold", name+".gold")
-	contentBytes, err := ioutil.ReadFile(goldPath)
+	contentBytes, err := os.ReadFile(goldPath)
 	require.NoError(pt.t, err)
 	gold := string(contentBytes)
 	// On windows the gold files come with '\r\n'...
@@ -648,12 +648,12 @@ func test_toitPkg(t *tedi.T) {
 			Branch: plumbing.NewTagReferenceName("1.0.0"),
 		})
 		require.NoError(t, err)
-		data, err := ioutil.ReadFile(filepath.Join(gitDir, "a"))
+		data, err := os.ReadFile(filepath.Join(gitDir, "a"))
 		require.NoError(t, err)
 		dataStr := strings.ReplaceAll(string(data), "\r\n", "\n")
 		// Notice that we implicitly check the correct `<[*TEST_DIR*>]` replacement.
 		assert.Equal(t, dirInFiles+"/a 1.0.0\n", dataStr)
-		data, err = ioutil.ReadFile(filepath.Join(gitDir, "b"))
+		data, err = os.ReadFile(filepath.Join(gitDir, "b"))
 		require.NoError(t, err)
 		dataStr = strings.ReplaceAll(string(data), "\r\n", "\n")
 		assert.Equal(t, dirInFiles+"/b 1.0.0\n", dataStr)
@@ -666,13 +666,13 @@ func test_toitPkg(t *tedi.T) {
 			Branch: plumbing.NewTagReferenceName("2.0.0"),
 		})
 		require.NoError(t, err)
-		data, err = ioutil.ReadFile(filepath.Join(gitDir, "a"))
+		data, err = os.ReadFile(filepath.Join(gitDir, "a"))
 		require.NoError(t, err)
 		dataStr = strings.ReplaceAll(string(data), "\r\n", "\n")
 		assert.Equal(t, dirInFiles+"/a 2.0.0\n", dataStr)
 		_, err = os.Stat(filepath.Join(gitDir, "b"))
 		assert.True(t, os.IsNotExist(err))
-		data, err = ioutil.ReadFile(filepath.Join(gitDir, "c"))
+		data, err = os.ReadFile(filepath.Join(gitDir, "c"))
 		require.NoError(t, err)
 		dataStr = strings.ReplaceAll(string(data), "\r\n", "\n")
 		assert.Equal(t, dirInFiles+"/c 2.0.0\n", dataStr)
@@ -1130,10 +1130,10 @@ func test_toitPkg(t *tedi.T) {
 				{"pkg", "list"},
 			})
 
-			data, err := ioutil.ReadFile(filepath.Join(pt.dir, yamlFile))
+			data, err := os.ReadFile(filepath.Join(pt.dir, yamlFile))
 			require.NoError(t, err)
 			pkgTestSpecPath := filepath.Join(regPath, yamlFile)
-			err = ioutil.WriteFile(pkgTestSpecPath, data, 0644)
+			err = os.WriteFile(pkgTestSpecPath, data, 0644)
 			require.NoError(t, err)
 
 			repository, err := git.PlainOpen(regPath)
@@ -1179,7 +1179,7 @@ func test_toitPkg(t *tedi.T) {
 			// Touch a file in the cache path, so we can see that the clear-cache
 			// actually removes it.
 			cacheFile := filepath.Join(regCachePath, "foo")
-			err := ioutil.WriteFile(cacheFile, []byte("foo"), 0644)
+			err := os.WriteFile(cacheFile, []byte("foo"), 0644)
 			require.NoError(t, err)
 
 			if i == 0 {
@@ -1359,7 +1359,7 @@ func test_toitPkg(t *tedi.T) {
 		})
 
 		yamlPath := filepath.Join(pt.dir, "package.yaml")
-		err := ioutil.WriteFile(yamlPath, []byte{}, 0644)
+		err := os.WriteFile(yamlPath, []byte{}, 0644)
 		assert.NoError(t, err)
 
 		pt.GoldToit("test2", [][]string{
@@ -1378,7 +1378,7 @@ func test_toitPkg(t *tedi.T) {
 			{"pkg", "registry", "add", "toit", "github.com/toitware/registry"},
 		})
 		configPath := filepath.Join(pt.dir, "config.yaml")
-		data, err := ioutil.ReadFile(configPath)
+		data, err := os.ReadFile(configPath)
 		assert.NoError(t, err)
 		assert.Contains(t, string(data), "toitware/registry")
 	})
@@ -1473,7 +1473,7 @@ func test_toitPkg(t *tedi.T) {
 		})
 		descPath := filepath.Join(pt.dir, "out", "packages", "github.com", "toitware", "toit-morse", "1.0.6", "desc.yaml")
 		assert.FileExists(t, descPath)
-		_, err := ioutil.ReadFile(descPath)
+		_, err := os.ReadFile(descPath)
 		assert.NoError(t, err)
 	})
 
@@ -1647,13 +1647,13 @@ func test_toitPkg(t *tedi.T) {
 		// Modifying the version constraint in the package.spec is copied to the
 		// lock file.
 		packageSpecPath := filepath.Join(pt.dir, "package.yaml")
-		data, err := ioutil.ReadFile(packageSpecPath)
+		data, err := os.ReadFile(packageSpecPath)
 		assert.NoError(t, err)
 		str := string(data) + `
 environment:
   sdk: ^1.20.0
 `
-		err = ioutil.WriteFile(packageSpecPath, []byte(str), 0644)
+		err = os.WriteFile(packageSpecPath, []byte(str), 0644)
 		assert.NoError(t, err)
 		pt.GoldToit("test3", [][]string{
 			{"// sdkVersion = "},
@@ -1709,4 +1709,24 @@ environment:
 		}
 		wg.Wait()
 	})
+
+	t.Run("InstallNoMTimeChange", func(t *tedi.T, pt PkgTest) {
+		regPath := filepath.Join(pt.dir, "registry_git_pkgs")
+		lockPath := filepath.Join(pt.dir, "package.lock")
+		info, err := os.Stat(lockPath)
+		assert.NoError(t, err)
+		mtimeBefore := info.ModTime()
+
+		pt.GoldToit("test", [][]string{
+			{"// Add registry so we can find packages."},
+			{"pkg", "registry", "add", "--local", "test-reg", regPath},
+			{"// Just 'install' doesn't add the missing dependencies."},
+			{"pkg", "install"},
+		})
+
+		info, err = os.Stat(lockPath)
+		assert.NoError(t, err)
+		mtimeAfter := info.ModTime()
+		assert.Equal(t, mtimeBefore, mtimeAfter)
+	})
 }