refactor(general): move SafeLongFilename to ospath (#5227)

- Move MaybePrefixLongFilenameOnWindows to ospath package and rename
  it to SafeLongFilename, along with corresponding test.
- Elide the function implementation at build time on non-Windows
  platforms.
- Update documentation and comments for clarity.
- Rename package-local helper function.

fs/localfs/shallow_fs.go

@@ -11,6 +11,7 @@
 
 	"github.com/kopia/kopia/fs"
 	"github.com/kopia/kopia/internal/atomicfile"
+	"github.com/kopia/kopia/internal/ospath"
 	"github.com/kopia/kopia/snapshot"
 )
 
@@ -26,7 +27,7 @@ func placeholderPath(path string, et snapshot.EntryType) (string, error) {
 		return path + ShallowEntrySuffix, nil
 	case snapshot.EntryTypeDirectory: // Directories and regular files
 		dirpath := path + ShallowEntrySuffix
-		if err := os.MkdirAll(atomicfile.MaybePrefixLongFilenameOnWindows(dirpath), os.FileMode(dirMode)); err != nil {
+		if err := os.MkdirAll(ospath.SafeLongFilename(dirpath), os.FileMode(dirMode)); err != nil {
 			return "", errors.Wrap(err, "placeholderPath dir creation")
 		}
 
@@ -64,7 +65,7 @@ func WriteShallowPlaceholder(path string, de *snapshot.DirEntry) (string, error)
 }
 
 func dirEntryFromPlaceholder(path string) (*snapshot.DirEntry, error) {
-	b, err := os.ReadFile(atomicfile.MaybePrefixLongFilenameOnWindows(path))
+	b, err := os.ReadFile(ospath.SafeLongFilename(path))
 	if err != nil {
 		return nil, errors.Wrap(err, "dirEntryFromPlaceholder reading placeholder")
 	}
@@ -89,7 +90,7 @@ type shallowFilesystemDirectory struct {
 }
 
 func checkedDirEntryFromPlaceholder(path, php string) (*snapshot.DirEntry, error) {
-	if _, err := os.Lstat(atomicfile.MaybePrefixLongFilenameOnWindows(path)); err == nil {
+	if _, err := os.Lstat(ospath.SafeLongFilename(path)); err == nil {
 		return nil, errors.Errorf("%q, %q exist: shallowrestore tree is corrupt probably because a previous restore into a shallow tree was interrupted", path, php)
 	}
 

internal/atomicfile/atomicfile.go

@@ -3,49 +3,14 @@
 
 import (
 	"io"
-	"runtime"
-	"strings"
 
 	"github.com/natefinch/atomic"
 
 	"github.com/kopia/kopia/internal/ospath"
 )
 
-// Do not prefix files shorter than this, we are intentionally using less than MAX_PATH
-// in Windows to allow some suffixes.
-const maxPathLength = 240
-
-// MaybePrefixLongFilenameOnWindows prefixes the given filename with \\?\ on Windows
-// if the filename is longer than 260 characters, which is required to be able to
-// use some low-level Windows APIs.
-// Because long file names have certain limitations:
-// - we must replace forward slashes with backslashes.
-// - dummy path element (\.\) must be removed.
-//
-// Relative paths are always limited to a total of MAX_PATH characters:
-// https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
-func MaybePrefixLongFilenameOnWindows(fname string) string {
-	if runtime.GOOS != "windows" || len(fname) < maxPathLength ||
-		fname[:4] == `\\?\` || !ospath.IsAbs(fname) {
-		return fname
-	}
-
-	fixed := strings.ReplaceAll(fname, "/", `\`)
-
-	for {
-		fixed2 := strings.ReplaceAll(fixed, `\.\`, `\`)
-		if fixed2 == fixed {
-			break
-		}
-
-		fixed = fixed2
-	}
-
-	return `\\?\` + fixed
-}
-
 // Write is a wrapper around atomic.WriteFile that handles long file names on Windows.
 func Write(filename string, r io.Reader) error {
 	//nolint:wrapcheck
-	return atomic.WriteFile(MaybePrefixLongFilenameOnWindows(filename), r)
+	return atomic.WriteFile(ospath.SafeLongFilename(filename), r)
 }

internal/ospath/ospath_nonwindows.go

@@ -0,0 +1,17 @@
+//go:build !windows
+
+package ospath
+
+// SafeLongFilename handles long absolute file paths in a platform-specific manner.
+// Currently it only handles absolute paths on Windows. It is a no-op on other
+// platforms.
+//
+// On Windows, it prefixes the given filename with \\?\ when the filename length
+// approximates MAX_PATH characters, which is required to be able to use some
+// low-level Windows APIs.
+//
+// Relative paths are always limited to a total of MAX_PATH characters:
+// https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
+func SafeLongFilename(fname string) string {
+	return fname
+}

internal/ospath/ospath_windows.go

@@ -2,9 +2,44 @@
 
 import (
 	"os"
+	"runtime"
+	"strings"
 )
 
 func init() {
 	userSettingsDir = os.Getenv("APPDATA")
 	userLogsDir = os.Getenv("LOCALAPPDATA")
 }
+
+// SafeLongFilename prefixes the given filename with \\?\ on Windows when the
+// filename length approximates MAX_PATH characters, which is required to be
+// able to use some low-level Windows APIs.
+// Because long file names have certain limitations:
+// - we must replace forward slashes with backslashes.
+// - dummy path element (\.\) must be removed.
+//
+// Relative paths are always limited to a total of MAX_PATH characters (typically 260):
+// https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
+func SafeLongFilename(fname string) string {
+	// Do not prefix when the name is shorter than this.
+	// Intentionally using less than MAX_PATH in Windows to allow some suffixes.
+	const maxPathLength = 240
+
+	if runtime.GOOS != "windows" || len(fname) < maxPathLength ||
+		fname[:4] == `\\?\` || !IsAbs(fname) {
+		return fname
+	}
+
+	fixed := strings.ReplaceAll(fname, "/", `\`)
+
+	for {
+		fixed2 := strings.ReplaceAll(fixed, `\.\`, `\`)
+		if fixed2 == fixed {
+			break
+		}
+
+		fixed = fixed2
+	}
+
+	return `\\?\` + fixed
+}

internal/ospath/ospath_windows_test.go

@@ -1,4 +1,4 @@
-package atomicfile
+package ospath
 
 import (
 	"runtime"
@@ -6,13 +6,13 @@
 	"testing"
 )
 
-var veryLongSegment = strings.Repeat("f", 270)
-
-func TestMaybePrefixLongFilenameOnWindows(t *testing.T) {
+func TestSafeLongFilename_Windows(t *testing.T) {
 	if runtime.GOOS != "windows" {
-		return
+		t.Skip("Windows-only test")
 	}
 
+	veryLongSegment := strings.Repeat("f", 270)
+
 	cases := []struct {
 		input string
 		want  string
@@ -37,7 +37,7 @@ func TestMaybePrefixLongFilenameOnWindows(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		if got := MaybePrefixLongFilenameOnWindows(tc.input); got != tc.want {
+		if got := SafeLongFilename(tc.input); got != tc.want {
 			t.Errorf("invalid result for %v: got %v, want %v", tc.input, got, tc.want)
 		}
 	}

repo/blob/filesystem/osinterface_realos.go

@@ -6,20 +6,14 @@
 	"os"
 	"time"
 
-	"github.com/kopia/kopia/internal/atomicfile"
+	"github.com/kopia/kopia/internal/ospath"
 )
 
 // realOS is an implementation of osInterface that uses real operating system calls.
 type realOS struct{}
 
-// longPath applies MaybePrefixLongFilenameOnWindows to handle paths exceeding
-// MAX_PATH on Windows. On non-Windows platforms this is a no-op.
-func longPath(p string) string {
-	return atomicfile.MaybePrefixLongFilenameOnWindows(p)
-}
-
 func (realOS) Open(fname string) (osReadFile, error) {
-	f, err := os.Open(longPath(fname))
+	f, err := os.Open(ospath.SafeLongFilename(fname))
 	if err != nil {
 		//nolint:wrapcheck
 		return nil, err
@@ -36,12 +30,12 @@ func (realOS) IsPathSeparator(c byte) bool { return os.IsPathSeparator(c) }
 
 func (realOS) Rename(oldname, newname string) error {
 	//nolint:wrapcheck
-	return os.Rename(longPath(oldname), longPath(newname))
+	return os.Rename(ospath.SafeLongFilename(oldname), ospath.SafeLongFilename(newname))
 }
 
 func (realOS) ReadDir(dirname string) ([]fs.DirEntry, error) {
 	//nolint:wrapcheck
-	return os.ReadDir(longPath(dirname))
+	return os.ReadDir(ospath.SafeLongFilename(dirname))
 }
 
 func (realOS) IsPathError(err error) bool {
@@ -58,32 +52,32 @@ func (realOS) IsLinkError(err error) bool {
 
 func (realOS) Remove(fname string) error {
 	//nolint:wrapcheck
-	return os.Remove(longPath(fname))
+	return os.Remove(ospath.SafeLongFilename(fname))
 }
 
 func (realOS) Stat(fname string) (os.FileInfo, error) {
 	//nolint:wrapcheck
-	return os.Stat(longPath(fname))
+	return os.Stat(ospath.SafeLongFilename(fname))
 }
 
 func (realOS) CreateNewFile(fname string, perm os.FileMode) (osWriteFile, error) {
 	//nolint:wrapcheck
-	return os.OpenFile(longPath(fname), os.O_CREATE|os.O_EXCL|os.O_WRONLY, perm)
+	return os.OpenFile(ospath.SafeLongFilename(fname), os.O_CREATE|os.O_EXCL|os.O_WRONLY, perm)
 }
 
 func (realOS) Mkdir(fname string, mode os.FileMode) error {
 	//nolint:wrapcheck
-	return os.Mkdir(longPath(fname), mode)
+	return os.Mkdir(ospath.SafeLongFilename(fname), mode)
 }
 
 func (realOS) MkdirAll(fname string, mode os.FileMode) error {
 	//nolint:wrapcheck
-	return os.MkdirAll(longPath(fname), mode)
+	return os.MkdirAll(ospath.SafeLongFilename(fname), mode)
 }
 
 func (realOS) Chtimes(fname string, atime, mtime time.Time) error {
 	//nolint:wrapcheck
-	return os.Chtimes(longPath(fname), atime, mtime)
+	return os.Chtimes(ospath.SafeLongFilename(fname), atime, mtime)
 }
 
 func (realOS) Geteuid() int {
@@ -92,7 +86,7 @@ func (realOS) Geteuid() int {
 
 func (realOS) Chown(fname string, uid, gid int) error {
 	//nolint:wrapcheck
-	return os.Chown(longPath(fname), uid, gid)
+	return os.Chown(ospath.SafeLongFilename(fname), uid, gid)
 }
 
 var _ osInterface = realOS{}

snapshot/restore/local_fs_output.go

@@ -15,6 +15,7 @@
 	"github.com/kopia/kopia/fs/localfs"
 	"github.com/kopia/kopia/internal/atomicfile"
 	"github.com/kopia/kopia/internal/iocopy"
+	"github.com/kopia/kopia/internal/ospath"
 	"github.com/kopia/kopia/internal/sparsefile"
 	"github.com/kopia/kopia/internal/stat"
 	"github.com/kopia/kopia/snapshot"
@@ -433,7 +434,7 @@ func (o *FilesystemOutput) copyFileContent(ctx context.Context, targetPath strin
 	}
 
 	log(ctx).Debugf("copying file contents to: %v", targetPath)
-	targetPath = atomicfile.MaybePrefixLongFilenameOnWindows(targetPath)
+	targetPath = ospath.SafeLongFilename(targetPath)
 
 	if o.WriteFilesAtomically {
 		//nolint:wrapcheck

snapshot/restore/local_fs_output_windows.go

@@ -7,7 +7,7 @@
 	"github.com/pkg/errors"
 	"golang.org/x/sys/windows"
 
-	"github.com/kopia/kopia/internal/atomicfile"
+	"github.com/kopia/kopia/internal/ospath"
 )
 
 //nolint:revive
@@ -24,7 +24,7 @@ func symlinkChtimes(linkPath string, atime, mtime time.Time) error {
 	fta := windows.NsecToFiletime(atime.UnixNano())
 	ftw := windows.NsecToFiletime(mtime.UnixNano())
 
-	linkPath = atomicfile.MaybePrefixLongFilenameOnWindows(linkPath)
+	linkPath = ospath.SafeLongFilename(linkPath)
 
 	fn, err := windows.UTF16PtrFromString(linkPath)
 	if err != nil {

snapshot/restore/shallow_helper.go

@@ -5,7 +5,7 @@
 	"strings"
 
 	"github.com/kopia/kopia/fs/localfs"
-	"github.com/kopia/kopia/internal/atomicfile"
+	"github.com/kopia/kopia/internal/ospath"
 )
 
 // PathIfPlaceholder returns the placeholder suffix trimmed from path or the
@@ -23,7 +23,7 @@ func PathIfPlaceholder(path string) string {
 func SafeRemoveAll(path string) error {
 	if SafelySuffixablePath(path) {
 		//nolint:wrapcheck
-		return os.RemoveAll(atomicfile.MaybePrefixLongFilenameOnWindows(path + localfs.ShallowEntrySuffix))
+		return os.RemoveAll(ospath.SafeLongFilename(path + localfs.ShallowEntrySuffix))
 	}
 
 	// path can't possibly exist because we could have never written a file

tests/end_to_end_test/shallowrestore_test.go

@@ -16,7 +16,7 @@
 	"github.com/stretchr/testify/require"
 
 	"github.com/kopia/kopia/fs/localfs"
-	"github.com/kopia/kopia/internal/atomicfile"
+	"github.com/kopia/kopia/internal/ospath"
 	"github.com/kopia/kopia/repo/object"
 	"github.com/kopia/kopia/snapshot"
 	"github.com/kopia/kopia/snapshot/restore"
@@ -146,7 +146,7 @@ func TestShallowFullCycle(t *testing.T) {
 	fpathinlong := filepath.Join(dirpathlong, "nestedfile")
 
 	require.NoError(t, os.Mkdir(dirpathlong, 0o755))
-	testdirtree.MustCreateRandomFile(t, atomicfile.MaybePrefixLongFilenameOnWindows(fpathinlong), testdirtree.DirectoryTreeOptions{}, (*testdirtree.DirectoryTreeCounters)(nil))
+	testdirtree.MustCreateRandomFile(t, ospath.SafeLongFilename(fpathinlong), testdirtree.DirectoryTreeOptions{}, (*testdirtree.DirectoryTreeCounters)(nil))
 
 	e.RunAndExpectSuccess(t, "snapshot", "create", source)
 	sources := clitestutil.ListSnapshotsAndExpectSuccess(t, e)
@@ -224,9 +224,8 @@ func addOneFile(m *mutatorArgs) {
 func doNothing(_ *mutatorArgs) {
 }
 
-// mplfow makes atomicfile.MaybePrefixLongFilenameOnWindows easier to type.
-func mplfow(fname string) string {
-	return atomicfile.MaybePrefixLongFilenameOnWindows(fname)
+func longName(fname string) string {
+	return ospath.SafeLongFilename(fname)
 }
 
 // moveDirectory moves a directory from one location to another (in the
@@ -252,11 +251,11 @@ func moveDirectory(m *mutatorArgs) {
 	require.NoError(m.t, os.Mkdir(neworiginaldir, 0o755))
 
 	// 3. move shallow dir into new dir, original dir into new dir
-	require.NoError(m.t, os.Rename(mplfow(dirinshallow), mplfow(filepath.Join(newshallowdir, relpath))))
-	require.NoError(m.t, os.Rename(mplfow(filepath.Join(m.original, relpathinreal)), mplfow(filepath.Join(neworiginaldir, relpathinreal))))
+	require.NoError(m.t, os.Rename(longName(dirinshallow), longName(filepath.Join(newshallowdir, relpath))))
+	require.NoError(m.t, os.Rename(longName(filepath.Join(m.original, relpathinreal)), longName(filepath.Join(neworiginaldir, relpathinreal))))
 
 	// 4. fix new directory timestamp to be the same
-	fi, err := os.Stat(mplfow(newshallowdir))
+	fi, err := os.Stat(longName(newshallowdir))
 	require.NoError(m.t, err)
 	require.NoError(m.t, os.Chtimes(neworiginaldir, fi.ModTime(), fi.ModTime()))
 	require.NoError(m.t, os.Chtimes(newshallowdir, fi.ModTime(), fi.ModTime()))
@@ -283,11 +282,11 @@ func moveFile(m *mutatorArgs) {
 	require.NoError(m.t, os.Mkdir(neworiginaldir, 0o755))
 
 	// 3. move shallow file into new dir, original dir into new dir
-	require.NoError(m.t, os.Rename(mplfow(fileinshallow), mplfow(filepath.Join(newshallowdir, relpath))))
-	require.NoError(m.t, os.Rename(mplfow(filepath.Join(m.original, localfs.TrimShallowSuffix(relpath))), mplfow(filepath.Join(neworiginaldir, localfs.TrimShallowSuffix(relpath)))))
+	require.NoError(m.t, os.Rename(longName(fileinshallow), longName(filepath.Join(newshallowdir, relpath))))
+	require.NoError(m.t, os.Rename(longName(filepath.Join(m.original, localfs.TrimShallowSuffix(relpath))), longName(filepath.Join(neworiginaldir, localfs.TrimShallowSuffix(relpath)))))
 
 	// 4. fix new directory timestamp to be the same
-	fi, err := os.Stat(mplfow(newshallowdir))
+	fi, err := os.Stat(longName(newshallowdir))
 	require.NoError(m.t, err)
 	require.NoError(m.t, os.Chtimes(neworiginaldir, fi.ModTime(), fi.ModTime()))
 	require.NoError(m.t, os.Chtimes(newshallowdir, fi.ModTime(), fi.ModTime()))