diff --git a/backend/local/local.go b/backend/local/local.go index 99c9c6f0f..65782308f 100644 --- a/backend/local/local.go +++ b/backend/local/local.go @@ -569,9 +569,8 @@ func (f *Fs) PutStream(ctx context.Context, in io.Reader, src fs.ObjectInfo, opt // Mkdir creates the directory if it doesn't exist func (f *Fs) Mkdir(ctx context.Context, dir string) error { - // FIXME: https://github.com/syncthing/syncthing/blob/master/lib/osutil/mkdirall_windows.go localPath := f.localPath(dir) - err := os.MkdirAll(localPath, 0777) + err := file.MkdirAll(localPath, 0777) if err != nil { return err } @@ -765,7 +764,7 @@ func (f *Fs) DirMove(ctx context.Context, src fs.Fs, srcRemote, dstRemote string // Create parent of destination dstParentPath := filepath.Dir(dstPath) - err = os.MkdirAll(dstParentPath, 0777) + err = file.MkdirAll(dstParentPath, 0777) if err != nil { return err } @@ -1099,7 +1098,7 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (in io.Read // mkdirAll makes all the directories needed to store the object func (o *Object) mkdirAll() error { dir := filepath.Dir(o.path) - return os.MkdirAll(dir, 0777) + return file.MkdirAll(dir, 0777) } type nopWriterCloser struct { diff --git a/cmd/copy/copy.go b/cmd/copy/copy.go index 887024450..ba2815852 100644 --- a/cmd/copy/copy.go +++ b/cmd/copy/copy.go @@ -78,6 +78,7 @@ recently very efficiently like this: **Note**: Use the |--dry-run| or the |--interactive|/|-i| flag to test without copying anything. `, "|", "`"), Run: func(command *cobra.Command, args []string) { + cmd.CheckArgs(2, 2, command, args) fsrc, srcFileName, fdst := cmd.NewFsSrcFileDst(args) cmd.Run(true, true, command, func() error { diff --git a/cmd/gendocs/gendocs.go b/cmd/gendocs/gendocs.go index 91b80929a..fda7d55a4 100644 --- a/cmd/gendocs/gendocs.go +++ b/cmd/gendocs/gendocs.go @@ -13,6 +13,7 @@ import ( "time" "github.com/rclone/rclone/cmd" + "github.com/rclone/rclone/lib/file" "github.com/spf13/cobra" "github.com/spf13/cobra/doc" "github.com/spf13/pflag" @@ -55,7 +56,7 @@ rclone.org website.`, // Create the directory structure root := args[0] out := filepath.Join(root, "commands") - err := os.MkdirAll(out, 0777) + err := file.MkdirAll(out, 0777) if err != nil { return err } diff --git a/cmd/selfupdate/selfupdate_test.go b/cmd/selfupdate/selfupdate_test.go index 2e6b15b58..b8c52a349 100644 --- a/cmd/selfupdate/selfupdate_test.go +++ b/cmd/selfupdate/selfupdate_test.go @@ -16,6 +16,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fstest/testy" + "github.com/rclone/rclone/lib/file" "github.com/rclone/rclone/lib/random" "github.com/stretchr/testify/assert" ) @@ -52,7 +53,7 @@ func makeTestDir() (testDir string, err error) { for attempt := 0; attempt < maxAttempts; attempt++ { testDir = testDirBase + random.String(4) - err = os.MkdirAll(testDir, os.ModePerm) + err = file.MkdirAll(testDir, os.ModePerm) if err == nil { break } diff --git a/cmd/serve/docker/docker_test.go b/cmd/serve/docker/docker_test.go index 0a8f99b76..169fd6e70 100644 --- a/cmd/serve/docker/docker_test.go +++ b/cmd/serve/docker/docker_test.go @@ -20,6 +20,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config" "github.com/rclone/rclone/fstest" + "github.com/rclone/rclone/lib/file" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -36,7 +37,7 @@ func initialise(ctx context.Context, t *testing.T) (string, fs.Fs) { // Make test cache directory testDir, err := fstest.LocalRemote() require.NoError(t, err) - err = os.MkdirAll(testDir, 0755) + err = file.MkdirAll(testDir, 0755) require.NoError(t, err) // Make test file system @@ -350,7 +351,7 @@ func testMountAPI(t *testing.T, sockAddr string) { // Run test sequence path1 := filepath.Join(testDir, "path1") - require.NoError(t, os.MkdirAll(path1, 0755)) + require.NoError(t, file.MkdirAll(path1, 0755)) mount1 := filepath.Join(testDir, "vol1") res := "" diff --git a/cmd/serve/docker/driver.go b/cmd/serve/docker/driver.go index 1d1fcefdf..14970958f 100644 --- a/cmd/serve/docker/driver.go +++ b/cmd/serve/docker/driver.go @@ -19,6 +19,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config" "github.com/rclone/rclone/lib/atexit" + "github.com/rclone/rclone/lib/file" "github.com/rclone/rclone/vfs/vfscommon" "github.com/rclone/rclone/vfs/vfsflags" ) @@ -44,12 +45,12 @@ func NewDriver(ctx context.Context, root string, mntOpt *mountlib.Options, vfsOp if err != nil { return nil, errors.Wrap(err, "failed to make --cache-dir absolute") } - err = os.MkdirAll(cacheDir, 0700) + err = file.MkdirAll(cacheDir, 0700) if err != nil { return nil, errors.Wrapf(err, "failed to create cache directory: %s", cacheDir) } - //err = os.MkdirAll(root, 0755) + //err = file.MkdirAll(root, 0755) if err != nil { return nil, errors.Wrapf(err, "failed to create mount root: %s", root) } diff --git a/cmd/serve/docker/serve.go b/cmd/serve/docker/serve.go index 63c5f8d2a..94a1ce45b 100644 --- a/cmd/serve/docker/serve.go +++ b/cmd/serve/docker/serve.go @@ -13,6 +13,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/lib/atexit" + "github.com/rclone/rclone/lib/file" ) // Server connects plugin with docker daemon by protocol @@ -87,7 +88,7 @@ func writeSpecFile(addr, proto, specDir string) (string, error) { if specDir == "" { specDir = defSpecDir } - if err := os.MkdirAll(specDir, 0755); err != nil { + if err := file.MkdirAll(specDir, 0755); err != nil { return "", err } specFile := filepath.Join(specDir, "rclone.spec") diff --git a/cmd/serve/docker/unix.go b/cmd/serve/docker/unix.go index 696f8d4f7..3c533d454 100644 --- a/cmd/serve/docker/unix.go +++ b/cmd/serve/docker/unix.go @@ -8,6 +8,8 @@ import ( "net" "os" "path/filepath" + + "github.com/rclone/rclone/lib/file" ) func newUnixListener(path string, gid int) (net.Listener, string, error) { @@ -31,7 +33,7 @@ func newUnixListener(path string, gid int) (net.Listener, string, error) { path = filepath.Join(sockDir, path) } - if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + if err := file.MkdirAll(filepath.Dir(path), 0755); err != nil { return nil, "", err } if err := os.Remove(path); err != nil && !os.IsNotExist(err) { diff --git a/cmd/serve/docker/volume.go b/cmd/serve/docker/volume.go index fd37d5211..770a5f893 100644 --- a/cmd/serve/docker/volume.go +++ b/cmd/serve/docker/volume.go @@ -14,6 +14,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config" "github.com/rclone/rclone/fs/rc" + "github.com/rclone/rclone/lib/file" ) // Errors @@ -153,7 +154,7 @@ func (vol *Volume) checkMountpoint() error { } _, err := os.Lstat(path) if os.IsNotExist(err) { - if err = os.MkdirAll(path, 0700); err != nil { + if err = file.MkdirAll(path, 0700); err != nil { return errors.Wrapf(err, "failed to create mountpoint: %s", path) } } else if err != nil { diff --git a/cmd/serve/sftp/server.go b/cmd/serve/sftp/server.go index 178169714..a6ba3802d 100644 --- a/cmd/serve/sftp/server.go +++ b/cmd/serve/sftp/server.go @@ -25,6 +25,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config" "github.com/rclone/rclone/lib/env" + "github.com/rclone/rclone/lib/file" "github.com/rclone/rclone/vfs" "github.com/rclone/rclone/vfs/vfsflags" "golang.org/x/crypto/ssh" @@ -227,7 +228,7 @@ func (s *server) serve() (err error) { if err != nil && len(s.opt.HostKeys) == 0 { fs.Debugf(nil, "Failed to load %q: %v", keyPath, err) // If loading a cached key failed, make the keys and retry - err = os.MkdirAll(cachePath, 0700) + err = file.MkdirAll(cachePath, 0700) if err != nil { return errors.Wrap(err, "failed to create cache path") } diff --git a/cmd/test/makefiles/makefiles.go b/cmd/test/makefiles/makefiles.go index 4edaebf07..08d1e3ccd 100644 --- a/cmd/test/makefiles/makefiles.go +++ b/cmd/test/makefiles/makefiles.go @@ -14,6 +14,7 @@ import ( "github.com/rclone/rclone/cmd/test" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config/flags" + "github.com/rclone/rclone/lib/file" "github.com/rclone/rclone/lib/random" "github.com/spf13/cobra" ) @@ -134,7 +135,7 @@ func (d *dir) list(path string, output []string) []string { // writeFile writes a random file at dir/name func writeFile(dir, name string) int64 { - err := os.MkdirAll(dir, 0777) + err := file.MkdirAll(dir, 0777) if err != nil { log.Fatalf("Failed to make directory %q: %v", dir, err) } diff --git a/fs/config/config.go b/fs/config/config.go index 0bded3ee2..b1c7a51d9 100644 --- a/fs/config/config.go +++ b/fs/config/config.go @@ -275,7 +275,7 @@ func makeConfigPath() string { return configFile } var mkdirErr error - if mkdirErr = os.MkdirAll(configDir, os.ModePerm); mkdirErr == nil { + if mkdirErr = file.MkdirAll(configDir, os.ModePerm); mkdirErr == nil { return configFile } // Problem: Try a fallback location. If we did find a home directory then diff --git a/fs/config/configfile/configfile.go b/fs/config/configfile/configfile.go index f1eb79ba6..6b93d4943 100644 --- a/fs/config/configfile/configfile.go +++ b/fs/config/configfile/configfile.go @@ -13,6 +13,7 @@ import ( "github.com/pkg/errors" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config" + "github.com/rclone/rclone/lib/file" ) // Install installs the config file handler @@ -109,7 +110,7 @@ func (s *Storage) Save() error { } dir, name := filepath.Split(configPath) - err := os.MkdirAll(dir, os.ModePerm) + err := file.MkdirAll(dir, os.ModePerm) if err != nil { return errors.Wrap(err, "failed to create config directory") } diff --git a/fs/rc/webgui/webgui.go b/fs/rc/webgui/webgui.go index 39ee5d9c1..a272d73e6 100644 --- a/fs/rc/webgui/webgui.go +++ b/fs/rc/webgui/webgui.go @@ -17,6 +17,7 @@ import ( "github.com/pkg/errors" "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/lib/file" ) // GetLatestReleaseURL returns the latest release details of the rclone-webui-react @@ -95,7 +96,7 @@ func CheckAndDownloadWebGUIRelease(checkUpdate bool, forceUpdate bool, fetchURL cachePathExist, cachePathStat, _ := exists(cachePath) if !cachePathExist { - if err := os.MkdirAll(cachePath, 0755); err != nil { + if err := file.MkdirAll(cachePath, 0755); err != nil { return errors.New("Error creating cache directory: " + cachePath) } } @@ -174,7 +175,7 @@ func Unzip(src, dest string) (err error) { } defer fs.CheckClose(r, &err) - if err := os.MkdirAll(dest, 0755); err != nil { + if err := file.MkdirAll(dest, 0755); err != nil { return err } @@ -193,14 +194,14 @@ func Unzip(src, dest string) (err error) { defer fs.CheckClose(rc, &err) if f.FileInfo().IsDir() { - if err := os.MkdirAll(path, 0755); err != nil { + if err := file.MkdirAll(path, 0755); err != nil { return err } } else { - if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + if err := file.MkdirAll(filepath.Dir(path), 0755); err != nil { return err } - f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + f, err := file.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) if err != nil { return err } @@ -239,7 +240,7 @@ func exists(path string) (existence bool, stat os.FileInfo, err error) { func CreatePathIfNotExist(path string) (err error) { exists, stat, _ := exists(path) if !exists { - if err := os.MkdirAll(path, 0755); err != nil { + if err := file.MkdirAll(path, 0755); err != nil { return errors.New("Error creating : " + path) } } diff --git a/fstest/run.go b/fstest/run.go index 8e99b3f89..70c396d69 100644 --- a/fstest/run.go +++ b/fstest/run.go @@ -44,6 +44,7 @@ import ( "github.com/rclone/rclone/fs/hash" "github.com/rclone/rclone/fs/object" "github.com/rclone/rclone/fs/walk" + "github.com/rclone/rclone/lib/file" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -212,7 +213,7 @@ func (r *Run) WriteFile(filePath, content string, t time.Time) Item { // FIXME make directories? filePath = path.Join(r.LocalName, filePath) dirPath := path.Dir(filePath) - err := os.MkdirAll(dirPath, 0770) + err := file.MkdirAll(dirPath, 0770) if err != nil { r.Fatalf("Failed to make directories %q: %v", dirPath, err) } diff --git a/fstest/test_all/report.go b/fstest/test_all/report.go index 03ca3a518..bcc1b5658 100644 --- a/fstest/test_all/report.go +++ b/fstest/test_all/report.go @@ -15,6 +15,7 @@ import ( "time" "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/lib/file" "github.com/skratchdot/open-golang/open" ) @@ -75,7 +76,7 @@ func NewReport() *Report { // Create output directory for logs and report r.LogDir = path.Join(*outputDir, r.DateTime) - err = os.MkdirAll(r.LogDir, 0777) + err = file.MkdirAll(r.LogDir, 0777) if err != nil { log.Fatalf("Failed to make log directory: %v", err) } diff --git a/lib/file/mkdir_other.go b/lib/file/mkdir_other.go new file mode 100644 index 000000000..9f49826a2 --- /dev/null +++ b/lib/file/mkdir_other.go @@ -0,0 +1,11 @@ +//go:build !windows +// +build !windows + +package file + +import "os" + +// MkdirAll just calls os.MkdirAll on non-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 new file mode 100644 index 000000000..36ccadd9c --- /dev/null +++ b/lib/file/mkdir_windows.go @@ -0,0 +1,77 @@ +//go:build windows +// +build windows + +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. +// +// 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 = 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 { + // 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_test.go b/lib/file/mkdir_windows_test.go new file mode 100644 index 000000000..9342f3c2e --- /dev/null +++ b/lib/file/mkdir_windows_test.go @@ -0,0 +1,132 @@ +//go:build windows +// +build windows + +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, tidy := testDir(t) + defer tidy() + + 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 f.Close() + + // 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, errormsg string) { + if valid { + assert.NoError(t, MkdirAll(path, 0777)) + } else { + err := MkdirAll(path, 0777) + assert.Error(t, err) + assert.Equal(t, errormsg, err.Error()) + } +} + +func checkMkdirAllSubdirs(t *testing.T, path string, valid bool, errormsg string) { + checkMkdirAll(t, path, valid, errormsg) + checkMkdirAll(t, path+`\`, valid, errormsg) + checkMkdirAll(t, path+`\parent`, valid, errormsg) + checkMkdirAll(t, path+`\parent\`, valid, errormsg) + checkMkdirAll(t, path+`\parent\child`, valid, errormsg) + checkMkdirAll(t, path+`\parent\child\`, valid, errormsg) +} + +// Testing paths on existing drive +func TestMkdirAllOnDrive(t *testing.T) { + path, tidy := testDir(t) + defer tidy() + + 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, "") + 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) + errormsg = fmt.Sprintf("mkdir \\\\?\\%s\\: The system cannot find the path specified.", path) + checkMkdirAllSubdirs(t, `\\?\`+path, false, errormsg) +} diff --git a/vfs/test_vfs/test_vfs.go b/vfs/test_vfs/test_vfs.go index 9d6f3168e..bd11ac02c 100644 --- a/vfs/test_vfs/test_vfs.go +++ b/vfs/test_vfs/test_vfs.go @@ -296,7 +296,7 @@ func main() { log.Fatalf("%s: Syntax [opts] ", os.Args[0]) } dir := args[0] - _ = os.MkdirAll(dir, 0777) + _ = file.MkdirAll(dir, 0777) var ( wg sync.WaitGroup diff --git a/vfs/vfscache/cache.go b/vfs/vfscache/cache.go index 48efc0490..c12d5c409 100644 --- a/vfs/vfscache/cache.go +++ b/vfs/vfscache/cache.go @@ -150,7 +150,7 @@ func New(ctx context.Context, fremote fs.Fs, opt *vfscommon.Options, avFn AddVir // createDir creates a directory path, along with any necessary parents func createDir(dir string) error { - return os.MkdirAll(dir, 0700) + return file.MkdirAll(dir, 0700) } // createRootDir creates a single cache root directory