diff --git a/tools/util_darwin.go b/tools/util_darwin.go index 8b57cda1..782e8cc6 100644 --- a/tools/util_darwin.go +++ b/tools/util_darwin.go @@ -141,7 +141,3 @@ func cloneFileSyscall(dst, src string) *CloneFileError { return nil } - -func TryRename(oldname, newname string) error { - return RenameFileCopyPermissions(oldname, newname) -} diff --git a/tools/util_generic.go b/tools/util_generic.go index fab7f772..5e912210 100644 --- a/tools/util_generic.go +++ b/tools/util_generic.go @@ -21,7 +21,3 @@ func CloneFile(writer io.Writer, reader io.Reader) (bool, error) { func CloneFileByPath(_, _ string) (bool, error) { return false, nil } - -func TryRename(oldname, newname string) error { - return RenameFileCopyPermissions(oldname, newname) -} diff --git a/tools/util_linux.go b/tools/util_linux.go index 850e9e94..35f2d1b6 100644 --- a/tools/util_linux.go +++ b/tools/util_linux.go @@ -71,7 +71,3 @@ func CloneFileByPath(dst, src string) (bool, error) { return CloneFile(dstFile, srcFile) } - -func TryRename(oldname, newname string) error { - return RenameFileCopyPermissions(oldname, newname) -} diff --git a/tools/util_windows.go b/tools/util_windows.go index 6cd6a4e3..5ef8adc3 100644 --- a/tools/util_windows.go +++ b/tools/util_windows.go @@ -6,7 +6,6 @@ import ( "io" "io/ioutil" "os" - "path/filepath" "unsafe" "golang.org/x/sys/windows" @@ -157,26 +156,3 @@ func roundUp(value, base int64) int64 { return value - mod + base } - -// This is a simplified variant of fixLongPath from file_windows.go. Unfortunately, that function is not public -func toSafePath(path string) (*uint16, error) { - abspath, err := filepath.Abs(path) - if err != nil { - return nil, err - } - - return windows.UTF16PtrFromString(`\\?\` + abspath) -} - -// This is almost the same as os.Rename but doesn't overwrite destination if it already exists -func TryRename(oldname, newname string) error { - from, err := toSafePath(oldname) - if err != nil { - return err - } - to, err := toSafePath(newname) - if err != nil { - return err - } - return windows.MoveFileEx(from, to, 0) -} diff --git a/tools/util_windows_test.go b/tools/util_windows_test.go index e506eacd..189df336 100644 --- a/tools/util_windows_test.go +++ b/tools/util_windows_test.go @@ -109,61 +109,3 @@ func fillFile(target *os.File, size int64) (hash string, err error) { return hex.EncodeToString(sha.Sum(nil)), nil } - -func TestTryRenameDestExists(t *testing.T) { - source, err := ioutil.TempFile("", "source") - assert.NoError(t, err) - assert.NoError(t, source.Close()) - defer os.Remove(source.Name()) - - sourceData := []byte("source") - assert.NoError(t, ioutil.WriteFile(source.Name(), sourceData, 0644)) - - dest, err := ioutil.TempFile("", "dest") - assert.NoError(t, err) - defer os.Remove(dest.Name()) - assert.NoError(t, dest.Close()) - - destData := []byte("dest") - assert.NoError(t, ioutil.WriteFile(dest.Name(), destData, 0644)) - - // Perform rename - err = TryRename(source.Name(), dest.Name()) - assert.True(t, os.IsExist(err)) - - sourceData2, err := ioutil.ReadFile(source.Name()) - assert.NoError(t, err) - assert.Equal(t, sourceData, sourceData2) - - destData2, err := ioutil.ReadFile(dest.Name()) - assert.NoError(t, err) - assert.Equal(t, destData, destData2) -} - -func TestTryRename(t *testing.T) { - source, err := ioutil.TempFile("", "source") - assert.NoError(t, err) - assert.NoError(t, source.Close()) - defer os.Remove(source.Name()) - - sourceData := []byte("source") - assert.NoError(t, ioutil.WriteFile(source.Name(), sourceData, 0644)) - - dest, err := ioutil.TempFile("", "dest") - assert.NoError(t, err) - defer os.Remove(dest.Name()) - assert.NoError(t, dest.Close()) - - // Remove destination file - assert.NoError(t, os.Remove(dest.Name())) - - // Perform rename - assert.NoError(t, TryRename(source.Name(), dest.Name())) - - _, err = os.Stat(source.Name()) - assert.True(t, os.IsNotExist(err)) - - destData, err := ioutil.ReadFile(dest.Name()) - assert.NoError(t, err) - assert.Equal(t, sourceData, destData) -} diff --git a/tq/basic_download.go b/tq/basic_download.go index a9fc96bd..0b21559e 100644 --- a/tq/basic_download.go +++ b/tq/basic_download.go @@ -100,7 +100,7 @@ func (a *basicDownloadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb Progr f.Close() // Rename file so next download can resume from where we stopped. // No error checking here, if rename fails then file will be deleted and there just will be no download resuming - tools.TryRename(f.Name(), a.downloadFilename(t)) + os.Rename(f.Name(), a.downloadFilename(t)) } return err @@ -254,15 +254,12 @@ func (a *basicDownloadAdapter) download(t *Transfer, cb ProgressCallback, authOk return fmt.Errorf("can't close tempfile %q: %v", dlfilename, err) } - err = tools.TryRename(dlfilename, t.Path) - // If rename failed because file already exists, we do not treat it as error - // This can happen when multiple git-lfs processes are fetching files concurrently on Windows - // Note that dlfilename needs to be handled regardless of TryRename success - if err != nil && !os.IsExist(err) { - return err + err = tools.RenameFileCopyPermissions(dlfilename, t.Path) + if _, err2 := os.Stat(t.Path); err2 == nil { + // Target file already exists, possibly was downloaded by other git-lfs process + return nil } - - return nil + return err } func configureBasicDownloadAdapter(m *Manifest) {