From ddecfe6e7791676e7911de6248965646c264bbf9 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 26 Feb 2025 11:32:05 +0000 Subject: [PATCH] build: make go1.23 the minimum go version This is necessary now that golang.org/x/crypto is only allowing the last two versions of Go. See: https://go.googlesource.com/crypto/+/89ff08d67c4d79f9ac619aaf1f7388888798651f --- .github/workflows/build.yml | 8 +- fs/versioncheck.go | 6 +- go.mod | 2 +- lib/file/mkdir.go | 8 ++ lib/file/mkdir_other.go | 10 -- lib/file/mkdir_windows.go | 78 ------------- lib/file/mkdir_windows_go122_test.go | 122 -------------------- lib/file/mkdir_windows_test.go | 161 --------------------------- 8 files changed, 13 insertions(+), 382 deletions(-) create mode 100644 lib/file/mkdir.go delete mode 100644 lib/file/mkdir_other.go delete mode 100644 lib/file/mkdir_windows.go delete mode 100644 lib/file/mkdir_windows_go122_test.go delete mode 100644 lib/file/mkdir_windows_test.go diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index feee119c7..615dd2231 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -26,7 +26,7 @@ jobs: strategy: fail-fast: false matrix: - job_name: ['linux', 'linux_386', 'mac_amd64', 'mac_arm64', 'windows', 'other_os', 'go1.22', 'go1.23'] + job_name: ['linux', 'linux_386', 'mac_amd64', 'mac_arm64', 'windows', 'other_os', 'go1.23'] include: - job_name: linux @@ -80,12 +80,6 @@ jobs: compile_all: true deploy: true - - job_name: go1.22 - os: ubuntu-latest - go: '1.22' - quicktest: true - racequicktest: true - - job_name: go1.23 os: ubuntu-latest go: '1.23' diff --git a/fs/versioncheck.go b/fs/versioncheck.go index 03f65f961..4e56fb8bc 100644 --- a/fs/versioncheck.go +++ b/fs/versioncheck.go @@ -1,7 +1,7 @@ -//go:build !go1.22 +//go:build !go1.23 package fs -// Upgrade to Go version 1.22 to compile rclone - latest stable go +// Upgrade to Go version 1.23 to compile rclone - latest stable go // compiler recommended. -func init() { Go_version_1_22_required_for_compilation() } +func init() { Go_version_1_23_required_for_compilation() } diff --git a/go.mod b/go.mod index 08a076848..f100bb436 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/rclone/rclone -go 1.22.0 +go 1.23.0 require ( bazil.org/fuse v0.0.0-20230120002735-62a210ff1fd5 diff --git a/lib/file/mkdir.go b/lib/file/mkdir.go new file mode 100644 index 000000000..0a2b350d7 --- /dev/null +++ b/lib/file/mkdir.go @@ -0,0 +1,8 @@ +package file + +import "os" + +// MkdirAll now just calls os.MkdirAll +func MkdirAll(path string, perm os.FileMode) error { + return os.MkdirAll(path, perm) +} diff --git a/lib/file/mkdir_other.go b/lib/file/mkdir_other.go deleted file mode 100644 index 3803885fb..000000000 --- a/lib/file/mkdir_other.go +++ /dev/null @@ -1,10 +0,0 @@ -//go:build !windows || go1.22 - -package file - -import "os" - -// MkdirAll just calls os.MkdirAll on non-Windows and with go1.22 or newer on Windows -func MkdirAll(path string, perm os.FileMode) error { - return os.MkdirAll(path, perm) -} diff --git a/lib/file/mkdir_windows.go b/lib/file/mkdir_windows.go deleted file mode 100644 index 0da50c0c8..000000000 --- a/lib/file/mkdir_windows.go +++ /dev/null @@ -1,78 +0,0 @@ -//go:build windows && !go1.22 - -package file - -import ( - "os" - "path/filepath" - "syscall" -) - -// MkdirAll creates a directory named path, along with any necessary parents. -// -// Improves os.MkdirAll by avoiding trying to create a folder `\\?` when the -// volume of a given extended length path does not exist, and `\\?\UNC` when -// a network host name does not exist. -// -// Based on source code from golang's os.MkdirAll -// (https://github.com/golang/go/blob/master/src/os/path.go) -func MkdirAll(path string, perm os.FileMode) error { - // Fast path: if we can tell whether path is a directory or file, stop with success or error. - dir, err := os.Stat(path) - if err == nil { - if dir.IsDir() { - return nil - } - return &os.PathError{ - Op: "mkdir", - Path: path, - Err: syscall.ENOTDIR, - } - } - - // Slow path: make sure parent exists and then call Mkdir for path. - i := len(path) - for i > 0 && os.IsPathSeparator(path[i-1]) { // Skip trailing path separator. - i-- - } - if i > 0 { - path = path[:i] - if path == filepath.VolumeName(path) { - // Make reference to a drive's root directory include the trailing slash. - // In extended-length form without trailing slash ("\\?\C:"), os.Stat - // and os.Mkdir both fails. With trailing slash ("\\?\C:\") works, - // and regular paths with or without it ("C:" and "C:\") both works. - path += string(os.PathSeparator) - } else { - // See if there is a parent to be created first. - // Not when path refer to a drive's root directory, because we don't - // want to return noninformative error trying to create \\?. - j := i - for j > 0 && !os.IsPathSeparator(path[j-1]) { // Scan backward over element. - j-- - } - if j > 1 { - if path[:j-1] != `\\?\UNC` && path[:j-1] != `\\?` { - // Create parent. - err = MkdirAll(path[:j-1], perm) - if err != nil { - return err - } - } - } - } - } - - // Parent now exists; invoke Mkdir and use its result. - err = os.Mkdir(path, perm) - if err != nil { - // Handle arguments like "foo/." by - // double-checking that directory doesn't exist. - dir, err1 := os.Lstat(path) - if err1 == nil && dir.IsDir() { - return nil - } - return err - } - return nil -} diff --git a/lib/file/mkdir_windows_go122_test.go b/lib/file/mkdir_windows_go122_test.go deleted file mode 100644 index eb9bbbef9..000000000 --- a/lib/file/mkdir_windows_go122_test.go +++ /dev/null @@ -1,122 +0,0 @@ -//go:build windows && go1.22 - -package file - -import ( - "fmt" - "os" - "path/filepath" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func checkMkdirAll(t *testing.T, path string, valid bool, errormsgs ...string) { - if valid { - assert.NoError(t, os.MkdirAll(path, 0777)) - } else { - err := os.MkdirAll(path, 0777) - assert.Error(t, err) - ok := false - for _, msg := range errormsgs { - if err.Error() == msg { - ok = true - } - } - assert.True(t, ok, fmt.Sprintf("Error message '%v' didn't match any of %v", err, errormsgs)) - } -} - -func checkMkdirAllSubdirs(t *testing.T, path string, valid bool, errormsgs ...string) { - checkMkdirAll(t, path, valid, errormsgs...) - checkMkdirAll(t, path+`\`, valid, errormsgs...) - checkMkdirAll(t, path+`\parent`, valid, errormsgs...) - checkMkdirAll(t, path+`\parent\`, valid, errormsgs...) - checkMkdirAll(t, path+`\parent\child`, valid, errormsgs...) - checkMkdirAll(t, path+`\parent\child\`, valid, errormsgs...) -} - -// Testing paths on existing drive -func TestMkdirAllOnDrive(t *testing.T) { - path := t.TempDir() - - dir, err := os.Stat(path) - require.NoError(t, err) - require.True(t, dir.IsDir()) - - drive := filepath.VolumeName(path) - - checkMkdirAll(t, drive, true, "") - checkMkdirAll(t, drive+`\`, true, "") - checkMkdirAll(t, `\\?\`+drive, false, fmt.Sprintf(`mkdir \\?\%s: Access is denied.`, drive)) // This isn't actually a valid Windows path, it worked under go1.21.3 but fails under go1.21.4 and newer - checkMkdirAll(t, `\\?\`+drive+`\`, true, "") - checkMkdirAllSubdirs(t, path, true, "") - checkMkdirAllSubdirs(t, `\\?\`+path, true, "") -} - -// Testing paths on unused drive -// This covers the cases that we wanted to improve with our own custom version -// of golang's os.MkdirAll, introduced in PR #5401. Before go1.22 the original -// os.MkdirAll would recurse extended-length paths down to the "\\?" prefix and -// return the noninformative error: -// "mkdir \\?: The filename, directory name, or volume label syntax is incorrect." -// Our version stopped the recursion at drive's root directory, and reported, -// before go1.21.4: -// "mkdir \\?\A:\: The system cannot find the path specified." -// or, starting with go1.21.4: -// "mkdir \\?\A:: The system cannot find the path specified." -// See https://github.com/rclone/rclone/pull/5401. -// Starting with go1.22 golang's os.MkdirAll have similar improvements that made -// our custom version no longer necessary. -func TestMkdirAllOnUnusedDrive(t *testing.T) { - letter := FindUnusedDriveLetter() - require.NotEqual(t, letter, 0) - drive := string(letter) + ":" - checkMkdirAll(t, drive, false, fmt.Sprintf(`mkdir %s: The system cannot find the path specified.`, drive)) - checkMkdirAll(t, drive+`\`, false, fmt.Sprintf(`mkdir %s\: The system cannot find the path specified.`, drive)) - checkMkdirAll(t, drive+`\parent`, false, fmt.Sprintf(`mkdir %s\parent: The system cannot find the path specified.`, drive)) - checkMkdirAll(t, drive+`\parent\`, false, fmt.Sprintf(`mkdir %s\parent\: The system cannot find the path specified.`, drive)) - checkMkdirAll(t, drive+`\parent\child`, false, fmt.Sprintf(`mkdir %s\parent: The system cannot find the path specified.`, drive)) - checkMkdirAll(t, drive+`\parent\child\`, false, fmt.Sprintf(`mkdir %s\parent: The system cannot find the path specified.`, drive)) - - drive = `\\?\` + drive - checkMkdirAll(t, drive, false, fmt.Sprintf(`mkdir %s: The system cannot find the file specified.`, drive)) - checkMkdirAll(t, drive+`\`, false, fmt.Sprintf(`mkdir %s\: The system cannot find the path specified.`, drive)) - checkMkdirAll(t, drive+`\parent`, false, fmt.Sprintf(`mkdir %s\parent: The system cannot find the path specified.`, drive)) - checkMkdirAll(t, drive+`\parent\`, false, fmt.Sprintf(`mkdir %s\parent\: The system cannot find the path specified.`, drive)) - checkMkdirAll(t, drive+`\parent\child`, false, fmt.Sprintf(`mkdir %s\parent: The system cannot find the path specified.`, drive)) - checkMkdirAll(t, drive+`\parent\child\`, false, fmt.Sprintf(`mkdir %s\parent: The system cannot find the path specified.`, drive)) -} - -// Testing paths on unknown network host -// This covers more cases that we wanted to improve in our custom version of -// golang's os.MkdirAll, extending that explained in TestMkdirAllOnUnusedDrive. -// With our first fix, stopping it from recursing extended-length paths down to -// the "\\?" prefix, it would now stop at `\\?\UNC`, because that is what -// filepath.VolumeName returns (which is wrong, that is not a volume name!), -// and still return a noninformative error: -// "mkdir \\?\UNC\\: The filename, directory name, or volume label syntax is incorrect." -// Our version stopped the recursion at level before this, and reports: -// "mkdir \\?\UNC\0.0.0.0: The specified path is invalid." -// See https://github.com/rclone/rclone/pull/6420. -// Starting with go1.22 golang's os.MkdirAll have similar improvements that made -// our custom version no longer necessary. -func TestMkdirAllOnUnusedNetworkHost(t *testing.T) { - sharePath := `\\0.0.0.0\share` - checkMkdirAll(t, sharePath, false, fmt.Sprintf(`mkdir %s: The format of the specified network name is invalid.`, sharePath)) - checkMkdirAll(t, sharePath+`\`, false, fmt.Sprintf(`mkdir %s\: The format of the specified network name is invalid.`, sharePath)) - checkMkdirAll(t, sharePath+`\parent`, false, fmt.Sprintf(`mkdir %s\parent: The format of the specified network name is invalid.`, sharePath)) - checkMkdirAll(t, sharePath+`\parent\`, false, fmt.Sprintf(`mkdir %s\parent\: The format of the specified network name is invalid.`, sharePath)) - checkMkdirAll(t, sharePath+`\parent\child`, false, fmt.Sprintf(`mkdir %s\parent: The format of the specified network name is invalid.`, sharePath)) - checkMkdirAll(t, sharePath+`\parent\child\`, false, fmt.Sprintf(`mkdir %s\parent: The format of the specified network name is invalid.`, sharePath)) - - serverPath := `\\?\UNC\0.0.0.0` - sharePath = serverPath + `\share` - checkMkdirAll(t, sharePath, false, fmt.Sprintf(`mkdir %s: The specified path is invalid.`, serverPath)) - checkMkdirAll(t, sharePath+`\`, false, fmt.Sprintf(`mkdir %s: The specified path is invalid.`, serverPath)) - checkMkdirAll(t, sharePath+`\parent`, false, fmt.Sprintf(`mkdir %s: The specified path is invalid.`, serverPath)) - checkMkdirAll(t, sharePath+`\parent\`, false, fmt.Sprintf(`mkdir %s: The specified path is invalid.`, serverPath)) - checkMkdirAll(t, sharePath+`\parent\child`, false, fmt.Sprintf(`mkdir %s: The specified path is invalid.`, serverPath)) - checkMkdirAll(t, sharePath+`\parent\child\`, false, fmt.Sprintf(`mkdir %s: The specified path is invalid.`, serverPath)) -} diff --git a/lib/file/mkdir_windows_test.go b/lib/file/mkdir_windows_test.go deleted file mode 100644 index e6d4bd389..000000000 --- a/lib/file/mkdir_windows_test.go +++ /dev/null @@ -1,161 +0,0 @@ -//go:build windows && !go1.22 - -package file - -import ( - "fmt" - "os" - "path/filepath" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// Basic test from golang's os/path_test.go -func TestMkdirAll(t *testing.T) { - tmpDir := t.TempDir() - - path := tmpDir + "/dir/./dir2" - err := MkdirAll(path, 0777) - if err != nil { - t.Fatalf("MkdirAll %q: %s", path, err) - } - - // Already exists, should succeed. - err = MkdirAll(path, 0777) - if err != nil { - t.Fatalf("MkdirAll %q (second time): %s", path, err) - } - - // Make file. - fpath := path + "/file" - f, err := Create(fpath) - if err != nil { - t.Fatalf("create %q: %s", fpath, err) - } - defer func() { - if err := f.Close(); err != nil { - t.Fatalf("Close %q: %s", fpath, err) - } - }() - - // Can't make directory named after file. - err = MkdirAll(fpath, 0777) - if err == nil { - t.Fatalf("MkdirAll %q: no error", fpath) - } - perr, ok := err.(*os.PathError) - if !ok { - t.Fatalf("MkdirAll %q returned %T, not *PathError", fpath, err) - } - if filepath.Clean(perr.Path) != filepath.Clean(fpath) { - t.Fatalf("MkdirAll %q returned wrong error path: %q not %q", fpath, filepath.Clean(perr.Path), filepath.Clean(fpath)) - } - - // Can't make subdirectory of file. - ffpath := fpath + "/subdir" - err = MkdirAll(ffpath, 0777) - if err == nil { - t.Fatalf("MkdirAll %q: no error", ffpath) - } - perr, ok = err.(*os.PathError) - if !ok { - t.Fatalf("MkdirAll %q returned %T, not *PathError", ffpath, err) - } - if filepath.Clean(perr.Path) != filepath.Clean(fpath) { - t.Fatalf("MkdirAll %q returned wrong error path: %q not %q", ffpath, filepath.Clean(perr.Path), filepath.Clean(fpath)) - } - - path = tmpDir + `\dir\.\dir2\` - err = MkdirAll(path, 0777) - if err != nil { - t.Fatalf("MkdirAll %q: %s", path, err) - } -} - -func unusedDrive(t *testing.T) string { - letter := FindUnusedDriveLetter() - require.NotEqual(t, letter, 0) - return string(letter) + ":" -} - -func checkMkdirAll(t *testing.T, path string, valid bool, errormsgs ...string) { - if valid { - assert.NoError(t, MkdirAll(path, 0777)) - } else { - err := MkdirAll(path, 0777) - assert.Error(t, err) - ok := false - for _, msg := range errormsgs { - if err.Error() == msg { - ok = true - } - } - assert.True(t, ok, fmt.Sprintf("Error message '%v' didn't match any of %v", err, errormsgs)) - } -} - -func checkMkdirAllSubdirs(t *testing.T, path string, valid bool, errormsgs ...string) { - checkMkdirAll(t, path, valid, errormsgs...) - checkMkdirAll(t, path+`\`, valid, errormsgs...) - checkMkdirAll(t, path+`\parent`, valid, errormsgs...) - checkMkdirAll(t, path+`\parent\`, valid, errormsgs...) - checkMkdirAll(t, path+`\parent\child`, valid, errormsgs...) - checkMkdirAll(t, path+`\parent\child\`, valid, errormsgs...) -} - -// Testing paths on existing drive -func TestMkdirAllOnDrive(t *testing.T) { - path := t.TempDir() - - dir, err := os.Stat(path) - require.NoError(t, err) - require.True(t, dir.IsDir()) - - drive := filepath.VolumeName(path) - - checkMkdirAll(t, drive, true, "") - checkMkdirAll(t, drive+`\`, true, "") - // checkMkdirAll(t, `\\?\`+drive, true, "") - this isn't actually a Valid Windows path - this test used to work under go1.21.3 but fails under go1.21.4 - checkMkdirAll(t, `\\?\`+drive+`\`, true, "") - checkMkdirAllSubdirs(t, path, true, "") - checkMkdirAllSubdirs(t, `\\?\`+path, true, "") -} - -// Testing paths on unused drive -// This is where there is a difference from golang's os.MkdirAll. It would -// recurse extended-length paths down to the "\\?" prefix and return the -// noninformative error: -// "mkdir \\?: The filename, directory name, or volume label syntax is incorrect." -// Our version stops the recursion at drive's root directory, and reports: -// "mkdir \\?\A:\: The system cannot find the path specified." -func TestMkdirAllOnUnusedDrive(t *testing.T) { - path := unusedDrive(t) - errormsg := fmt.Sprintf(`mkdir %s\: The system cannot find the path specified.`, path) - checkMkdirAllSubdirs(t, path, false, errormsg) - errormsg1 := fmt.Sprintf(`mkdir \\?\%s\: The system cannot find the path specified.`, path) // pre go1.21.4 - errormsg2 := fmt.Sprintf(`mkdir \\?\%s: The system cannot find the file specified.`, path) // go1.21.4 and after - checkMkdirAllSubdirs(t, `\\?\`+path, false, errormsg1, errormsg2) -} - -// Testing paths on unknown network host -// This is an additional difference from golang's os.MkdirAll. With our -// first fix, stopping it from recursing extended-length paths down to -// the "\\?" prefix, it would now stop at `\\?\UNC`, because that is what -// filepath.VolumeName returns (which is wrong, that is not a volume name!), -// and still return a nonifnromative error: -// "mkdir \\?\UNC\\: The filename, directory name, or volume label syntax is incorrect." -// Our version stops the recursion at level before this, and reports: -// "mkdir \\?\UNC\0.0.0.0: The specified path is invalid." -func TestMkdirAllOnUnusedNetworkHost(t *testing.T) { - path := `\\0.0.0.0\share` - errormsg := fmt.Sprintf("mkdir %s\\: The format of the specified network name is invalid.", path) - checkMkdirAllSubdirs(t, path, false, errormsg) - path = `\\?\UNC\0.0.0.0\share` - checkMkdirAllSubdirs(t, path, false, - `mkdir \\?\UNC\0.0.0.0: The specified path is invalid.`, // pre go1.20 - `mkdir \\?\UNC\0.0.0.0\share\: The format of the specified network name is invalid.`, - ) - -}