Revert "Stop replacing files in LFS storage when downloading them concurrently on Windows"

This reverts commit 0c8edfc097a8364819649c9ba4df2221082a8cf8.

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)
-}

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)
-}

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)
-}

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)
-}

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)
-}

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 err
 }
 
 func configureBasicDownloadAdapter(m *Manifest) {