Skip to content

Commit

Permalink
Revert "Stop replacing files in LFS storage when downloading them con…
Browse files Browse the repository at this point in the history
…currently on Windows"

This reverts commit 0c8edfc.
  • Loading branch information
slonopotamus committed Oct 29, 2019
1 parent 03a64ff commit 470398c
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 103 deletions.
4 changes: 0 additions & 4 deletions tools/util_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,3 @@ func cloneFileSyscall(dst, src string) *CloneFileError {

return nil
}

func TryRename(oldname, newname string) error {
return RenameFileCopyPermissions(oldname, newname)
}
4 changes: 0 additions & 4 deletions tools/util_generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
4 changes: 0 additions & 4 deletions tools/util_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
24 changes: 0 additions & 24 deletions tools/util_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"io"
"io/ioutil"
"os"
"path/filepath"
"unsafe"

"golang.org/x/sys/windows"
Expand Down Expand Up @@ -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)
}
58 changes: 0 additions & 58 deletions tools/util_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
15 changes: 6 additions & 9 deletions tq/basic_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 470398c

Please sign in to comment.