Skip to content

Commit 78115e8

Browse files
authored
fix cross-device error for temp file - fixed #122 (#320)
* fix cross-device error for temp file - fixed #122 * add changelog * address pr comments
1 parent 25f6453 commit 78115e8

12 files changed

Lines changed: 362 additions & 58 deletions

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
66

77
## [Unreleased]
88

9+
### Added
10+
- `backend/os.WithTempDir` option to specify a custom directory for intermediate temporary files during OS write operations, providing a reliable way to resolve cross-device rename issues (#122).
11+
12+
### Fixed
13+
- OS backend now selects temporary file locations on the same device/volume as the target file by default, preventing cross-device rename errors (#122).
14+
- Fixed OS backend handling of Windows drive letters in URIs: the drive letter (e.g. `C:`) is now correctly part of the path (`file:///C:/...`) rather than the authority, conforming to RFC 8089.
15+
916
## [v7.15.0] - 2026-03-06
1017
### Added
1118
- Added AGENTS.md with comprehensive guidelines for AI agents, including development standards, testing requirements, CHANGELOG/PR process, Go version policy, GitHub Actions maintenance, and module management.

backend/os/doc.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,47 @@ Package os - built-in os lib VFS implementation.
33
44
# Usage
55
6-
Rely on github.com/c2fo/vfs/v7/backend
6+
The OS backend interacts directly with the local operating system's file system.
7+
8+
It can be initialized using the backend registry:
79
810
import(
911
"github.com/c2fo/vfs/v7/backend"
10-
"github.com/c2fo/vfs/v7/backend/os"
12+
_ "github.com/c2fo/vfs/v7/backend/os"
1113
)
1214
1315
func UseFs() error {
14-
fs := backend.Backend(os.Scheme)
16+
fs, err := backend.NewFileSystem(os.Scheme)
17+
if err != nil {
18+
return err
19+
}
20+
// Use fs
1521
...
1622
}
1723
18-
Or call directly:
24+
Or by calling directly:
1925
2026
import _os "github.com/c2fo/vfs/v7/backend/os"
2127
2228
func DoSomething() {
23-
fs := &_os.NewFileSystem()
29+
fs := _os.NewFileSystem()
30+
31+
// Or with a custom temporary directory
32+
fsWithOpts := _os.NewFileSystem(
33+
_os.WithTempDir{TempDir: "/path/to/my/temp"},
34+
)
2435
...
2536
}
2637
38+
# Options
39+
40+
The OS backend supports the following options for NewFileSystem:
41+
42+
- WithTempDir: Specifies a custom directory path for temporary files used
43+
during write operations. If not provided, the backend uses the system
44+
temp directory when it's on the same device as the target file, or falls
45+
back to the target file's parent directory.
46+
2747
# See Also
2848
2949
See: https://golang.org/pkg/os/

backend/os/file.go

Lines changed: 77 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ type File struct {
3232
fileOpener opener
3333
seekCalled bool
3434
readCalled bool
35+
tempDir string
3536
}
3637

3738
// Delete unlinks the file returning any error or nil.
@@ -461,30 +462,45 @@ func (f *File) getInternalFile() (*os.File, error) {
461462
}
462463

463464
func (f *File) copyToLocalTempReader() (*os.File, error) {
464-
tmpFile, err := os.CreateTemp("", fmt.Sprintf("%s.%d", f.Name(), time.Now().UnixNano()))
465+
filePath := osFilePath(f)
466+
467+
tempDir, err := f.selectTempDir(filePath)
465468
if err != nil {
466-
return nil, err
469+
return nil, fmt.Errorf("failed to select temp directory for %s: %w", filePath, err)
467470
}
468471

469-
exists, err := f.Exists()
472+
tmpFile, err := os.CreateTemp(tempDir, ".vfs-*.tmp")
470473
if err != nil {
471-
return nil, err
474+
return nil, fmt.Errorf("failed to create temp file for %s: %w", filePath, err)
472475
}
473476

477+
// Clean up temp file on any error after this point
478+
ok := false
479+
defer func() {
480+
if !ok {
481+
_ = tmpFile.Close()
482+
_ = os.Remove(tmpFile.Name())
483+
}
484+
}()
485+
474486
// If file exists AND we've called Seek or Read first, any subsequent writes should edit the file (temp),
475487
// so we copy the original file to the temp file then set the cursor position on the temp file to the current position.
476488
// If we're opening because Write is called first, we always overwrite the file, so no need to copy the original contents.
477489
//
478490
// So imagine we have a file with content "hello world" and we call Seek(6, 0) and then Write([]byte("there")), the
479491
// temp file should have "hello there" and not "there". Then finally when Close is called, the temp file is renamed
480492
// to the original file. This code ensures that scenario works as expected.
493+
exists, err := f.Exists()
494+
if err != nil {
495+
return nil, err
496+
}
481497
if exists && (f.seekCalled || f.readCalled) {
482498
openFunc := openOSFile
483499
if f.fileOpener != nil {
484500
openFunc = f.fileOpener
485501
}
486502

487-
actualFile, err := openFunc(osFilePath(f))
503+
actualFile, err := openFunc(filePath)
488504
if err != nil {
489505
return nil, err
490506
}
@@ -494,19 +510,71 @@ func (f *File) copyToLocalTempReader() (*os.File, error) {
494510
}
495511

496512
if f.cursorPos > 0 {
497-
// match cursor position in tmep file
498513
if _, err := tmpFile.Seek(f.cursorPos, 0); err != nil {
499514
return nil, err
500515
}
501516
}
502517
}
503518

519+
ok = true
504520
return tmpFile, nil
505521
}
506522

523+
// selectTempDir determines the best directory for creating a temporary file.
524+
// Priority:
525+
// 1. User-specified tempDir (via WithTempDir option)
526+
// 2. System temp dir, if on the same device/volume as the target file
527+
// 3. Target file's parent directory (last resort; may be visible to file watchers)
528+
func (f *File) selectTempDir(targetPath string) (string, error) {
529+
if f.tempDir != "" {
530+
if err := os.MkdirAll(f.tempDir, 0750); err != nil {
531+
return "", fmt.Errorf("cannot create specified temp dir %s: %w", f.tempDir, err)
532+
}
533+
return f.tempDir, nil
534+
}
535+
536+
osTempDir := os.TempDir()
537+
same, err := areSameVolumeOrDevice(targetPath, osTempDir)
538+
if err == nil && same {
539+
return osTempDir, nil
540+
}
541+
542+
// Fall back to target's parent directory — guaranteed same device
543+
targetDir := filepath.Dir(targetPath)
544+
if err := os.MkdirAll(targetDir, 0750); err != nil {
545+
return "", fmt.Errorf("cannot create target directory %s: %w", targetDir, err)
546+
}
547+
return targetDir, nil
548+
}
549+
550+
// areSameVolumeOrDevice checks if two paths reside on the same volume (Windows)
551+
// or device (Unix).
552+
func areSameVolumeOrDevice(path1, path2 string) (bool, error) {
553+
vol1, err := getVolumeOrDevice(path1)
554+
if err != nil {
555+
return false, err
556+
}
557+
vol2, err := getVolumeOrDevice(path2)
558+
if err != nil {
559+
return false, err
560+
}
561+
return vol1 == vol2, nil
562+
}
563+
564+
// osFilePath converts a VFS file path back to a native OS path.
565+
// On Windows, the internal path /C:/foo/bar.txt becomes C:\foo\bar.txt.
507566
func osFilePath(f vfs.File) string {
508-
if runtime.GOOS == "windows" {
509-
return f.Location().Authority().String() + filepath.FromSlash(f.Path())
567+
return toNativeOSPath(f.Path())
568+
}
569+
570+
// toNativeOSPath converts an internal forward-slash path to a native OS path.
571+
// On Windows, strips the leading "/" before a drive letter and converts slashes.
572+
func toNativeOSPath(p string) string {
573+
if p == "" {
574+
return ""
575+
}
576+
if runtime.GOOS == "windows" && len(p) >= 3 && p[0] == '/' && p[2] == ':' {
577+
p = p[1:]
510578
}
511-
return f.Path()
579+
return filepath.FromSlash(p)
512580
}

backend/os/fileSystem.go

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package os
33
import (
44
"path"
55
"path/filepath"
6-
"runtime"
76

87
"github.com/c2fo/vfs/v7"
98
"github.com/c2fo/vfs/v7/backend"
@@ -17,7 +16,9 @@ const Scheme = "file"
1716
const name = "os"
1817

1918
// FileSystem implements vfs.FileSystem for the OS file system.
20-
type FileSystem struct{}
19+
type FileSystem struct {
20+
tempDir string
21+
}
2122

2223
// NewFileSystem creates a new instance of the OS file system.
2324
func NewFileSystem(opts ...options.NewFileSystemOption[FileSystem]) *FileSystem {
@@ -38,14 +39,7 @@ func (fs *FileSystem) Retry() vfs.Retry {
3839

3940
// NewFile function returns the os implementation of vfs.File.
4041
func (fs *FileSystem) NewFile(authorityStr, filePath string, opts ...options.NewFileOption) (vfs.File, error) {
41-
if runtime.GOOS == "windows" && filepath.IsAbs(filePath) {
42-
if v := filepath.VolumeName(filePath); v != "" {
43-
authorityStr = v
44-
filePath = filePath[len(v):]
45-
}
46-
}
47-
48-
filePath = filepath.ToSlash(filePath)
42+
filePath = normalizeOSPath(filePath)
4943
err := utils.ValidateAbsoluteFilePath(filePath)
5044
if err != nil {
5145
return nil, err
@@ -60,19 +54,13 @@ func (fs *FileSystem) NewFile(authorityStr, filePath string, opts ...options.New
6054
name: filePath,
6155
location: loc.(*Location),
6256
opts: opts,
57+
tempDir: fs.tempDir,
6358
}, nil
6459
}
6560

6661
// NewLocation function returns the os implementation of vfs.Location.
6762
func (fs *FileSystem) NewLocation(authorityStr, locPath string) (vfs.Location, error) {
68-
if runtime.GOOS == "windows" && filepath.IsAbs(locPath) {
69-
if v := filepath.VolumeName(locPath); v != "" {
70-
authorityStr = v
71-
locPath = locPath[len(v):]
72-
}
73-
}
74-
75-
locPath = filepath.ToSlash(locPath)
63+
locPath = normalizeOSPath(locPath)
7664
err := utils.ValidateAbsoluteLocationPath(locPath)
7765
if err != nil {
7866
return nil, err
@@ -103,3 +91,19 @@ func (fs *FileSystem) Scheme() string {
10391
func init() {
10492
backend.Register(Scheme, &FileSystem{})
10593
}
94+
95+
// normalizeOSPath converts a native OS path to the canonical forward-slash form
96+
// used internally. On Windows, drive-letter paths like "C:\foo" become "/C:/foo"
97+
// so the drive letter stays in the path (not the URI authority).
98+
// UNC paths like "\\server\share\path" become "//server/share/path" via
99+
// filepath.ToSlash and already satisfy the leading-slash requirement.
100+
func normalizeOSPath(p string) string {
101+
if p == "" {
102+
return ""
103+
}
104+
p = filepath.ToSlash(p)
105+
if len(p) >= 2 && p[1] == ':' && ((p[0] >= 'A' && p[0] <= 'Z') || (p[0] >= 'a' && p[0] <= 'z')) {
106+
p = "/" + p
107+
}
108+
return p
109+
}

backend/os/fileSystem_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,21 @@ func (o *osFileSystemTest) TestNewFileSystem() {
7474
o.IsType((*FileSystem)(nil), fs)
7575
}
7676

77+
func (o *osFileSystemTest) TestWithTempDirOption() {
78+
opt := WithTempDir{TempDir: "/custom/temp"}
79+
80+
o.Equal("WithTempDir", opt.NewFileSystemOptionName())
81+
82+
fs := &FileSystem{}
83+
opt.Apply(fs)
84+
o.Equal("/custom/temp", fs.tempDir)
85+
}
86+
87+
func (o *osFileSystemTest) TestNewFileSystemWithTempDir() {
88+
fs := NewFileSystem(WithTempDir{TempDir: "/my/temp"})
89+
o.Equal("/my/temp", fs.tempDir)
90+
}
91+
7792
func TestOSFileSystem(t *testing.T) {
7893
suite.Run(t, new(osFileSystemTest))
7994
}

0 commit comments

Comments
 (0)