Further attempts to fix ACCESS_DENIED error on Windows

go/cmd/dolt/system_checks.go

@@ -19,9 +19,9 @@ import (
 	"io/ioutil"
 	"os"
 
-	"github.com/dolthub/dolt/go/store/util/tempfiles"
-
 	"github.com/dolthub/dolt/go/libraries/doltcore/env"
+	"github.com/dolthub/dolt/go/libraries/utils/file"
+	"github.com/dolthub/dolt/go/store/util/tempfiles"
 )
 
 // returns false if it fails to verify that it can move files from the default temp directory to the local directory.
@@ -41,14 +41,14 @@ func canMoveTempFile() bool {
 		return false
 	}
 
-	err = os.Rename(name, testfile)
+	err = file.Rename(name, testfile)
 
 	if err != nil {
-		_ = os.Remove(name)
+		_ = file.Remove(name)
 		return false
 	}
 
-	_ = os.Remove(testfile)
+	_ = file.Remove(testfile)
 	return true
 }
 

go/cmd/git-dolt/utils/find_git_config_dir_test.go

@@ -20,6 +20,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -30,7 +32,7 @@ func TestFindGitConfigDir(t *testing.T) {
 		t.Errorf("Error creating temp directory: %v", err)
 	}
 	defer func() {
-		if err := os.RemoveAll(tmpDir); err != nil {
+		if err := file.RemoveAll(tmpDir); err != nil {
 			t.Errorf("Error removing test directories: %v", err)
 		}
 	}()

go/libraries/utils/file/open.go

@@ -0,0 +1,42 @@
+// Copyright 2021 Dolthub, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// +build !windows
+
+package file
+
+import (
+	"os"
+)
+
+// The file issues are only observed on Windows, therefore on other platforms these function no differently than direct
+// calls.
+
+// Rename functions exactly like os.Rename, except that it retries upon failure on Windows. This "fixes" some errors
+// that appear on Windows.
+func Rename(oldpath, newpath string) error {
+	return os.Rename(oldpath, newpath)
+}
+
+// Remove functions exactly like os.Remove, except that it retries upon failure on Windows. This "fixes" some errors
+// that appear on Windows.
+func Remove(name string) error {
+	return os.Remove(name)
+}
+
+// RemoveAll functions exactly like os.RemoveAll, except that it retries upon failure on Windows. This "fixes" some errors
+// that appear on Windows.
+func RemoveAll(path string) error {
+	return os.RemoveAll(path)
+}

go/libraries/utils/file/open_windows.go

@@ -0,0 +1,85 @@
+// Copyright 2021 Dolthub, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// +build windows
+
+package file
+
+import (
+	"os"
+	"syscall"
+	"time"
+
+	"golang.org/x/sys/windows"
+)
+
+// Rename functions exactly like os.Rename, except that it retries upon failure on Windows. This "fixes" some errors
+// that appear on Windows.
+func Rename(oldpath, newpath string) error {
+	err := os.Rename(oldpath, newpath)
+	if isAccessError(err) {
+		for waitTime := time.Duration(1); isAccessError(err) && waitTime <= 10000; waitTime *= 10 {
+			time.Sleep(waitTime * time.Millisecond)
+			err = os.Rename(oldpath, newpath)
+		}
+	}
+	return err
+}
+
+// Remove functions exactly like os.Remove, except that it retries upon failure on Windows. This "fixes" some errors
+// that appear on Windows.
+func Remove(name string) error {
+	err := os.Remove(name)
+	if isAccessError(err) {
+		for waitTime := time.Duration(1); isAccessError(err) && waitTime <= 10000; waitTime *= 10 {
+			time.Sleep(waitTime * time.Millisecond)
+			err = os.Remove(name)
+		}
+	}
+	return err
+}
+
+// RemoveAll functions exactly like os.RemoveAll, except that it retries upon failure on Windows. This "fixes" some errors
+// that appear on Windows.
+func RemoveAll(path string) error {
+	err := os.RemoveAll(path)
+	if isAccessError(err) {
+		for waitTime := time.Duration(1); isAccessError(err) && waitTime <= 10000; waitTime *= 10 {
+			time.Sleep(waitTime * time.Millisecond)
+			err = os.RemoveAll(path)
+		}
+	}
+	return err
+}
+
+func isAccessError(err error) bool {
+	switch err := err.(type) {
+	case *os.LinkError:
+		sysErr, ok := err.Err.(syscall.Errno)
+		if ok && (sysErr == windows.ERROR_ACCESS_DENIED || sysErr == windows.ERROR_SHARING_VIOLATION) {
+			return true
+		}
+	case *os.PathError:
+		sysErr, ok := err.Err.(syscall.Errno)
+		if ok && (sysErr == windows.ERROR_ACCESS_DENIED || sysErr == windows.ERROR_SHARING_VIOLATION) {
+			return true
+		}
+	case *os.SyscallError:
+		sysErr, ok := err.Err.(syscall.Errno)
+		if ok && (sysErr == windows.ERROR_ACCESS_DENIED || sysErr == windows.ERROR_SHARING_VIOLATION) {
+			return true
+		}
+	}
+	return false
+}

go/libraries/utils/filesys/localfs.go

@@ -22,6 +22,8 @@ import (
 	"os"
 	"path/filepath"
 	"time"
+
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 )
 
 // LocalFS is the machines local filesystem
@@ -217,7 +219,7 @@ func (fs *localFS) DeleteFile(path string) error {
 			return ErrIsDir
 		}
 
-		return os.Remove(path)
+		return file.Remove(path)
 	}
 
 	return os.ErrNotExist
@@ -234,9 +236,9 @@ func (fs *localFS) Delete(path string, force bool) error {
 	}
 
 	if !force {
-		return os.Remove(path)
+		return file.Remove(path)
 	} else {
-		return os.RemoveAll(path)
+		return file.RemoveAll(path)
 	}
 }
 
@@ -255,7 +257,7 @@ func (fs *localFS) MoveFile(srcPath, destPath string) error {
 		return err
 	}
 
-	return os.Rename(srcPath, destPath)
+	return file.Rename(srcPath, destPath)
 }
 
 // converts a path to an absolute path.  If it's already an absolute path the input path will be returned unaltered

go/performance/benchmarks/benchmarks.go

@@ -23,6 +23,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
+
 	"github.com/dolthub/dolt/go/cmd/dolt/commands"
 	"github.com/dolthub/dolt/go/cmd/dolt/commands/tblcmds"
 	"github.com/dolthub/dolt/go/libraries/doltcore/dbfactory"
@@ -229,7 +231,7 @@ func getStdinForSQLBenchmark(fs filesys.Filesys, pathToImportFile string) *os.Fi
 	if err != nil {
 		log.Fatal(err)
 	}
-	defer os.Remove(tmpfile.Name()) // clean up
+	defer file.Remove(tmpfile.Name()) // clean up
 
 	if _, err := tmpfile.Write(content); err != nil {
 		log.Fatal(err)

go/store/blobstore/local.go

@@ -22,10 +22,11 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/dolthub/dolt/go/store/util/tempfiles"
-
 	"github.com/dolthub/fslock"
 	"github.com/google/uuid"
+
+	"github.com/dolthub/dolt/go/libraries/utils/file"
+	"github.com/dolthub/dolt/go/store/util/tempfiles"
 )
 
 const (
@@ -169,7 +170,7 @@ func (bs *LocalBlobstore) Put(ctx context.Context, key string, reader io.Reader)
 	}
 
 	path := filepath.Join(bs.RootDir, key) + bsExt
-	err = os.Rename(tempFile, path)
+	err = file.Rename(tempFile, path)
 
 	if err != nil {
 		return "", err

go/store/cmd/noms/noms_merge_test.go

@@ -25,12 +25,12 @@ import (
 	"bytes"
 	"context"
 	"io/ioutil"
-	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/libraries/utils/osutil"
 	"github.com/dolthub/dolt/go/store/datas"
 	"github.com/dolthub/dolt/go/store/spec"
@@ -47,7 +47,7 @@ func TestNomsMerge(t *testing.T) {
 }
 
 func (s *nomsMergeTestSuite) TearDownTest() {
-	err := os.RemoveAll(s.DBDir)
+	err := file.RemoveAll(s.DBDir)
 	if !osutil.IsWindows {
 		s.NoError(err)
 	}

go/store/cmd/noms/noms_sync_test.go

@@ -23,11 +23,11 @@ package main
 
 import (
 	"context"
-	"os"
 	"testing"
 
 	"github.com/stretchr/testify/suite"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/datas"
 	"github.com/dolthub/dolt/go/store/nbs"
 	"github.com/dolthub/dolt/go/store/spec"
@@ -67,7 +67,7 @@ func (s *nomsSyncTestSuite) TestSyncValidation() {
 }
 
 func (s *nomsSyncTestSuite) TestSync() {
-	defer s.NoError(os.RemoveAll(s.DBDir2))
+	defer s.NoError(file.RemoveAll(s.DBDir2))
 
 	cs, err := nbs.NewLocalStore(context.Background(), types.Format_Default.VersionString(), s.DBDir, clienttest.DefaultMemTableSize)
 	s.NoError(err)
@@ -128,7 +128,7 @@ func (s *nomsSyncTestSuite) TestSync() {
 }
 
 func (s *nomsSyncTestSuite) TestSync_Issue2598() {
-	defer s.NoError(os.RemoveAll(s.DBDir2))
+	defer s.NoError(file.RemoveAll(s.DBDir2))
 
 	cs, err := nbs.NewLocalStore(context.Background(), types.Format_Default.VersionString(), s.DBDir, clienttest.DefaultMemTableSize)
 	s.NoError(err)

go/store/datas/puller.go

@@ -26,6 +26,7 @@ import (
 
 	"golang.org/x/sync/semaphore"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/libraries/utils/iohelp"
 	"github.com/dolthub/dolt/go/store/atomicerr"
 	"github.com/dolthub/dolt/go/store/chunks"
@@ -239,7 +240,7 @@ func (p *Puller) uploadTempTableFile(ctx context.Context, ae *atomicerr.AtomicEr
 		fWithStats.Stop()
 
 		go func() {
-			_ = os.Remove(tmpTblFile.path)
+			_ = file.Remove(tmpTblFile.path)
 		}()
 	}()
 

go/store/nbs/benchmarks/main.go

@@ -38,6 +38,7 @@ import (
 	flag "github.com/juju/gnuflag"
 	"github.com/stretchr/testify/assert"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/chunks"
 	"github.com/dolthub/dolt/go/store/d"
 	"github.com/dolthub/dolt/go/store/nbs"
@@ -104,14 +105,14 @@ func main() {
 		if *toNBS != "" {
 			dir := makeTempDir(*toNBS, pb)
 			defer func() {
-				err := os.RemoveAll(dir)
+				err := file.RemoveAll(dir)
 				d.PanicIfError(err)
 			}()
 			open = func() (chunks.ChunkStore, error) {
 				return nbs.NewLocalStore(context.Background(), types.Format_Default.VersionString(), dir, bufSize)
 			}
 			reset = func() {
-				err := os.RemoveAll(dir)
+				err := file.RemoveAll(dir)
 				d.PanicIfError(err)
 				err = os.MkdirAll(dir, 0777)
 				d.PanicIfError(err)
@@ -120,7 +121,7 @@ func main() {
 		} else if *toFile != "" {
 			dir := makeTempDir(*toFile, pb)
 			defer func() {
-				err := os.RemoveAll(dir)
+				err := file.RemoveAll(dir)
 				d.PanicIfError(err)
 			}()
 			open = func() (chunks.ChunkStore, error) {
@@ -129,7 +130,7 @@ func main() {
 				return newFileBlockStore(f)
 			}
 			reset = func() {
-				err := os.RemoveAll(dir)
+				err := file.RemoveAll(dir)
 				d.PanicIfError(err)
 				err = os.MkdirAll(dir, 0777)
 				d.PanicIfError(err)

go/store/nbs/block_store_test.go

@@ -36,6 +36,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/libraries/utils/osutil"
 	"github.com/dolthub/dolt/go/store/chunks"
 	"github.com/dolthub/dolt/go/store/constants"
@@ -70,7 +71,7 @@ func (suite *BlockStoreSuite) SetupTest() {
 func (suite *BlockStoreSuite) TearDownTest() {
 	err := suite.store.Close()
 	suite.NoError(err)
-	err = os.RemoveAll(suite.dir)
+	err = file.RemoveAll(suite.dir)
 	if !osutil.IsWindowsSharingViolation(err) {
 		suite.NoError(err)
 	}

go/store/nbs/byte_sink.go

@@ -22,10 +22,10 @@ import (
 	"os"
 	"sync"
 
-	"github.com/dolthub/dolt/go/store/util/tempfiles"
-
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/libraries/utils/iohelp"
 	"github.com/dolthub/dolt/go/store/atomicerr"
+	"github.com/dolthub/dolt/go/store/util/tempfiles"
 )
 
 func flushSinkToFile(sink ByteSink, path string) (err error) {
@@ -285,7 +285,7 @@ func (sink *BufferedFileByteSink) FlushToFile(path string) (err error) {
 		return err
 	}
 
-	return os.Rename(sink.path, path)
+	return file.Rename(sink.path, path)
 }
 
 // HashingByteSink is a ByteSink that keeps an md5 hash of all the data written to it.

go/store/nbs/cache.go

@@ -25,8 +25,8 @@ import (
 	"context"
 	"errors"
 	"io/ioutil"
-	"os"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/chunks"
 	"github.com/dolthub/dolt/go/store/hash"
 	"github.com/dolthub/dolt/go/store/types"
@@ -101,7 +101,7 @@ func (nbc *NomsBlockCache) Count() (uint32, error) {
 // Destroy drops the cache and deletes any backing storage.
 func (nbc *NomsBlockCache) Destroy() error {
 	chunkErr := nbc.chunks.Close()
-	remErr := os.RemoveAll(nbc.dbDir)
+	remErr := file.RemoveAll(nbc.dbDir)
 
 	if chunkErr != nil {
 		return chunkErr

go/store/nbs/fd_cache_test.go

@@ -30,13 +30,15 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestFDCache(t *testing.T) {
 	dir := makeTempDir(t)
-	defer os.RemoveAll(dir)
+	defer file.RemoveAll(dir)
 
 	paths := [3]string{}
 	for i := range paths {

go/store/nbs/file_manifest.go

@@ -34,6 +34,7 @@ import (
 
 	"github.com/dolthub/fslock"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/chunks"
 	"github.com/dolthub/dolt/go/store/hash"
 	"github.com/dolthub/dolt/go/store/util/tempfiles"
@@ -475,7 +476,7 @@ func updateWithParseWriterAndChecker(_ context.Context, dir string, write manife
 		return manifestContents{}, err
 	}
 
-	defer os.Remove(tempManifestPath) // If we rename below, this will be a no-op
+	defer file.Remove(tempManifestPath) // If we rename below, this will be a no-op
 
 	// Take manifest file lock
 	lck := newLock(dir)
@@ -552,23 +553,9 @@ func updateWithParseWriterAndChecker(_ context.Context, dir string, write manife
 		return manifestContents{}, err
 	}
 
-	err = os.Rename(tempManifestPath, manifestPath)
+	err = file.Rename(tempManifestPath, manifestPath)
 	if err != nil {
-		// On Windows, renaming the temporary manifest file to the current manifest file overwrites the current file.
-		// This can occasionally cause an "ACCESS DENIED" error, aborting the entire operation. The cause is not clear,
-		// as it does not appear to be a dangling file handle (observed through Process Explorer). The error is also
-		// hard to reproduce and inconsistent, as it seems to occur between 200-7,000 renames, but averages around 1,500
-		// renames. This adds a delay before retrying the rename, increasing the wait time by a factor of 10 after each
-		// failure, up to a max of 10 seconds. If an error still occurs after that time, then we just fail. It is
-		// unknown if this is sufficient enough to completely eliminate the issue, however it has so far been able to
-		// succeed before reaching the retry limit.
-		for waitTime := time.Duration(1); err != nil && waitTime <= 10000; waitTime *= 10 {
-			time.Sleep(waitTime * time.Millisecond)
-			err = os.Rename(tempManifestPath, manifestPath)
-		}
-		if err != nil {
-			return manifestContents{}, err
-		}
+		return manifestContents{}, err
 	}
 
 	return newContents, nil

go/store/nbs/file_manifest_test.go

@@ -24,7 +24,6 @@ package nbs
 import (
 	"context"
 	"io/ioutil"
-	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
@@ -34,6 +33,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/constants"
 	"github.com/dolthub/dolt/go/store/hash"
 )
@@ -47,7 +47,7 @@ func makeFileManifestTempDir(t *testing.T) fileManifestV5 {
 func TestFileManifestLoadIfExists(t *testing.T) {
 	assert := assert.New(t)
 	fm := makeFileManifestTempDir(t)
-	defer os.RemoveAll(fm.dir)
+	defer file.RemoveAll(fm.dir)
 	stats := &Stats{}
 
 	exists, upstream, err := fm.ParseIfExists(context.Background(), stats, nil)
@@ -79,7 +79,7 @@ func TestFileManifestLoadIfExists(t *testing.T) {
 func TestFileManifestLoadIfExistsHoldsLock(t *testing.T) {
 	assert := assert.New(t)
 	fm := makeFileManifestTempDir(t)
-	defer os.RemoveAll(fm.dir)
+	defer file.RemoveAll(fm.dir)
 	stats := &Stats{}
 
 	// Simulate another process writing a manifest.
@@ -115,7 +115,7 @@ func TestFileManifestLoadIfExistsHoldsLock(t *testing.T) {
 func TestFileManifestUpdateWontClobberOldVersion(t *testing.T) {
 	assert := assert.New(t)
 	fm := makeFileManifestTempDir(t)
-	defer os.RemoveAll(fm.dir)
+	defer file.RemoveAll(fm.dir)
 	stats := &Stats{}
 
 	// Simulate another process having already put old Noms data in dir/.
@@ -130,7 +130,7 @@ func TestFileManifestUpdateWontClobberOldVersion(t *testing.T) {
 func TestFileManifestUpdateEmpty(t *testing.T) {
 	assert := assert.New(t)
 	fm := makeFileManifestTempDir(t)
-	defer os.RemoveAll(fm.dir)
+	defer file.RemoveAll(fm.dir)
 	stats := &Stats{}
 
 	l := computeAddr([]byte{0x01})
@@ -159,7 +159,7 @@ func TestFileManifestUpdateEmpty(t *testing.T) {
 func TestFileManifestUpdate(t *testing.T) {
 	assert := assert.New(t)
 	fm := makeFileManifestTempDir(t)
-	defer os.RemoveAll(fm.dir)
+	defer file.RemoveAll(fm.dir)
 	stats := &Stats{}
 
 	// First, test winning the race against another process.

go/store/nbs/file_table_persister.go

@@ -32,9 +32,9 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/dolthub/dolt/go/store/util/tempfiles"
-
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/d"
+	"github.com/dolthub/dolt/go/store/util/tempfiles"
 )
 
 const tempTablePrefix = "nbs_table_"
@@ -123,7 +123,7 @@ func (ftp *fsTablePersister) persistTable(ctx context.Context, name addr, data [
 		return nil, err
 	}
 
-	err = os.Rename(tempName, newName)
+	err = file.Rename(tempName, newName)
 
 	if err != nil {
 		return nil, err
@@ -203,7 +203,7 @@ func (ftp *fsTablePersister) ConjoinAll(ctx context.Context, sources chunkSource
 		return nil, err
 	}
 
-	err = os.Rename(tempName, filepath.Join(ftp.dir, name.String()))
+	err = file.Rename(tempName, filepath.Join(ftp.dir, name.String()))
 
 	if err != nil {
 		return nil, err
@@ -236,7 +236,7 @@ func (ftp *fsTablePersister) PruneTableFiles(ctx context.Context, contents manif
 		filePath := path.Join(ftp.dir, info.Name())
 
 		if strings.HasPrefix(info.Name(), tempTablePrefix) {
-			err = os.Remove(filePath)
+			err = file.Remove(filePath)
 			if err != nil {
 				ea.add(filePath, err)
 			}
@@ -256,7 +256,7 @@ func (ftp *fsTablePersister) PruneTableFiles(ctx context.Context, contents manif
 			continue // file is referenced in the manifest
 		}
 
-		err = os.Remove(filePath)
+		err = file.Remove(filePath)
 		if err != nil {
 			ea.add(filePath, err)
 		}

go/store/nbs/file_table_persister_test.go

@@ -30,6 +30,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -37,7 +39,7 @@ import (
 func TestFSTableCacheOnOpen(t *testing.T) {
 	assert := assert.New(t)
 	dir := makeTempDir(t)
-	defer os.RemoveAll(dir)
+	defer file.RemoveAll(dir)
 
 	names := []addr{}
 	cacheSize := 2
@@ -106,7 +108,7 @@ func writeTableData(dir string, chunx ...[]byte) (addr, error) {
 
 func removeTables(dir string, names ...addr) error {
 	for _, name := range names {
-		if err := os.Remove(filepath.Join(dir, name.String())); err != nil {
+		if err := file.Remove(filepath.Join(dir, name.String())); err != nil {
 			return err
 		}
 	}
@@ -116,7 +118,7 @@ func removeTables(dir string, names ...addr) error {
 func TestFSTablePersisterPersist(t *testing.T) {
 	assert := assert.New(t)
 	dir := makeTempDir(t)
-	defer os.RemoveAll(dir)
+	defer file.RemoveAll(dir)
 	fc := newFDCache(defaultMaxTables)
 	defer fc.Drop()
 	fts := newFSTablePersister(dir, fc, nil)
@@ -154,7 +156,7 @@ func TestFSTablePersisterPersistNoData(t *testing.T) {
 	}
 
 	dir := makeTempDir(t)
-	defer os.RemoveAll(dir)
+	defer file.RemoveAll(dir)
 	fc := newFDCache(defaultMaxTables)
 	defer fc.Drop()
 	fts := newFSTablePersister(dir, fc, nil)
@@ -173,7 +175,7 @@ func TestFSTablePersisterCacheOnPersist(t *testing.T) {
 	fc := newFDCache(1)
 	defer fc.Drop()
 	fts := newFSTablePersister(dir, fc, nil)
-	defer os.RemoveAll(dir)
+	defer file.RemoveAll(dir)
 
 	var name addr
 	func() {
@@ -205,7 +207,7 @@ func TestFSTablePersisterConjoinAll(t *testing.T) {
 	sources := make(chunkSources, len(testChunks))
 
 	dir := makeTempDir(t)
-	defer os.RemoveAll(dir)
+	defer file.RemoveAll(dir)
 	fc := newFDCache(len(sources))
 	defer fc.Drop()
 	fts := newFSTablePersister(dir, fc, nil)
@@ -240,7 +242,7 @@ func TestFSTablePersisterConjoinAll(t *testing.T) {
 func TestFSTablePersisterConjoinAllDups(t *testing.T) {
 	assert := assert.New(t)
 	dir := makeTempDir(t)
-	defer os.RemoveAll(dir)
+	defer file.RemoveAll(dir)
 	fc := newFDCache(defaultMaxTables)
 	defer fc.Drop()
 	fts := newFSTablePersister(dir, fc, nil)

go/store/nbs/fs_table_cache.go

@@ -29,6 +29,8 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
+
 	"github.com/dolthub/dolt/go/store/atomicerr"
 	"github.com/dolthub/dolt/go/store/util/sizecache"
 	"github.com/dolthub/dolt/go/store/util/tempfiles"
@@ -88,7 +90,7 @@ func (ftc *fsTableCache) init(concurrency int) error {
 			}
 			if isTempTableFile(info) {
 				// ignore failure to remove temp file
-				_ = os.Remove(path)
+				_ = file.Remove(path)
 				return nil
 			}
 			if !isTableFile(info) {
@@ -204,7 +206,7 @@ func (ftc *fsTableCache) store(h addr, data io.Reader, size uint64) error {
 		return err
 	}
 
-	err = os.Rename(tempName, path)
+	err = file.Rename(tempName, path)
 
 	if err != nil {
 		return err
@@ -225,5 +227,5 @@ func (ftc *fsTableCache) store(h addr, data io.Reader, size uint64) error {
 }
 
 func (ftc *fsTableCache) expire(h addr) error {
-	return os.Remove(filepath.Join(ftc.dir, h.String()))
+	return file.Remove(filepath.Join(ftc.dir, h.String()))
 }

go/store/nbs/fs_table_cache_test.go

@@ -30,6 +30,8 @@ import (
 	"sort"
 	"testing"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -41,7 +43,7 @@ func TestFSTableCache(t *testing.T) {
 	t.Run("ExpireLRU", func(t *testing.T) {
 		t.Parallel()
 		dir := makeTempDir(t)
-		defer os.RemoveAll(dir)
+		defer file.RemoveAll(dir)
 
 		sum := 0
 		for _, s := range datas[1:] {
@@ -77,7 +79,7 @@ func TestFSTableCache(t *testing.T) {
 		t.Run("Success", func(t *testing.T) {
 			t.Parallel()
 			dir := makeTempDir(t)
-			defer os.RemoveAll(dir)
+			defer file.RemoveAll(dir)
 			assert := assert.New(t)
 
 			var names []addr
@@ -99,7 +101,7 @@ func TestFSTableCache(t *testing.T) {
 		t.Run("BadFile", func(t *testing.T) {
 			t.Parallel()
 			dir := makeTempDir(t)
-			defer os.RemoveAll(dir)
+			defer file.RemoveAll(dir)
 
 			require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "boo"), nil, 0666))
 			_, err := newFSTableCache(dir, 1024, 4)
@@ -109,7 +111,7 @@ func TestFSTableCache(t *testing.T) {
 		t.Run("ClearTempFile", func(t *testing.T) {
 			t.Parallel()
 			dir := makeTempDir(t)
-			defer os.RemoveAll(dir)
+			defer file.RemoveAll(dir)
 
 			tempFile := filepath.Join(dir, tempTablePrefix+"boo")
 			require.NoError(t, ioutil.WriteFile(tempFile, nil, 0666))
@@ -122,7 +124,7 @@ func TestFSTableCache(t *testing.T) {
 		t.Run("Dir", func(t *testing.T) {
 			t.Parallel()
 			dir := makeTempDir(t)
-			defer os.RemoveAll(dir)
+			defer file.RemoveAll(dir)
 			require.NoError(t, os.Mkdir(filepath.Join(dir, "sub"), 0777))
 			_, err := newFSTableCache(dir, 1024, 4)
 			assert.Error(t, err)

go/store/nbs/mmap_table_reader_test.go

@@ -23,10 +23,11 @@ package nbs
 
 import (
 	"io/ioutil"
-	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -35,7 +36,7 @@ func TestMmapTableReader(t *testing.T) {
 	assert := assert.New(t)
 	dir, err := ioutil.TempDir("", "")
 	require.NoError(t, err)
-	defer os.RemoveAll(dir)
+	defer file.RemoveAll(dir)
 
 	fc := newFDCache(1)
 	defer fc.Drop()

go/store/nbs/stats_test.go

@@ -24,12 +24,12 @@ package nbs
 import (
 	"context"
 	"io/ioutil"
-	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/chunks"
 	"github.com/dolthub/dolt/go/store/constants"
 	"github.com/dolthub/dolt/go/store/hash"
@@ -144,5 +144,5 @@ func TestStats(t *testing.T) {
 	// TODO: Once random conjoin hack is out, test other conjoin stats
 
 	defer store.Close()
-	defer os.RemoveAll(dir)
+	defer file.RemoveAll(dir)
 }

go/store/perf/suite/suite.go

@@ -111,6 +111,7 @@ import (
 	"github.com/stretchr/testify/require"
 	testifySuite "github.com/stretchr/testify/suite"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/libraries/utils/osutil"
 	"github.com/dolthub/dolt/go/store/chunks"
 	"github.com/dolthub/dolt/go/store/datas"
@@ -229,10 +230,10 @@ func Run(datasetID string, t *testing.T, suiteT perfSuiteT) {
 	defer func() {
 		for _, f := range suite.tempFiles {
 			f.Close()
-			os.Remove(f.Name())
+			file.Remove(f.Name())
 		}
 		for _, d := range suite.tempDirs {
-			os.RemoveAll(d)
+			file.RemoveAll(d)
 		}
 	}()
 

go/store/perf/suite/suite_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/spec"
 	"github.com/dolthub/dolt/go/store/types"
 )
@@ -175,7 +176,7 @@ func runTestSuite(t *testing.T, mem bool) {
 	// Write test results to our own temporary LDB database.
 	ldbDir, err := ioutil.TempDir("", "suite.TestSuite")
 	require.NoError(t, err)
-	defer os.RemoveAll(ldbDir)
+	defer file.RemoveAll(ldbDir)
 
 	flagVal, repeatFlagVal, memFlagVal := *perfFlag, *perfRepeatFlag, *perfMemFlag
 	*perfFlag, *perfRepeatFlag, *perfMemFlag = ldbDir, 3, mem
@@ -290,7 +291,7 @@ func TestPrefixFlag(t *testing.T) {
 	// Write test results to a temporary database.
 	ldbDir, err := ioutil.TempDir("", "suite.TestSuite")
 	require.NoError(t, err)
-	defer os.RemoveAll(ldbDir)
+	defer file.RemoveAll(ldbDir)
 
 	flagVal, prefixFlagVal := *perfFlag, *perfPrefixFlag
 	*perfFlag, *perfPrefixFlag = ldbDir, "foo/"

go/store/spec/spec_test.go

@@ -31,6 +31,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/chunks"
 	"github.com/dolthub/dolt/go/store/d"
 	"github.com/dolthub/dolt/go/store/datas"
@@ -161,7 +162,7 @@ func TestNBSDatabaseSpec(t *testing.T) {
 	run := func(prefix string) {
 		tmpDir, err := ioutil.TempDir("", "spec_test")
 		assert.NoError(err)
-		defer os.RemoveAll(tmpDir)
+		defer file.RemoveAll(tmpDir)
 
 		s := types.String("string")
 
@@ -263,7 +264,7 @@ func TestForDatabase(t *testing.T) {
 
 	tmpDir, err := ioutil.TempDir("", "spec_test")
 	assert.NoError(err)
-	defer os.RemoveAll(tmpDir)
+	defer file.RemoveAll(tmpDir)
 
 	testCases := []struct {
 		spec, protocol, databaseName, canonicalSpecIfAny string
@@ -327,7 +328,7 @@ func TestForDataset(t *testing.T) {
 
 	tmpDir, err := ioutil.TempDir("", "spec_test")
 	assert.NoError(err)
-	defer os.RemoveAll(tmpDir)
+	defer file.RemoveAll(tmpDir)
 
 	testCases := []struct {
 		spec, protocol, databaseName, datasetName, canonicalSpecIfAny string
@@ -372,7 +373,7 @@ func TestForPath(t *testing.T) {
 
 	tmpDir, err := ioutil.TempDir("", "spec_test")
 	assert.NoError(err)
-	defer os.RemoveAll(tmpDir)
+	defer file.RemoveAll(tmpDir)
 
 	testCases := []struct {
 		spec, protocol, databaseName, pathString, canonicalSpecIfAny string
@@ -499,7 +500,7 @@ func TestMultipleSpecsSameNBS(t *testing.T) {
 
 	tmpDir, err := ioutil.TempDir("", "spec_test")
 	assert.NoError(err)
-	defer os.RemoveAll(tmpDir)
+	defer file.RemoveAll(tmpDir)
 
 	spec1, err1 := ForDatabase(tmpDir)
 	spec2, err2 := ForDatabase(tmpDir)

go/store/types/perf/perf_test.go

@@ -31,6 +31,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/store/atomicerr"
 	"github.com/dolthub/dolt/go/store/perf/suite"
 	"github.com/dolthub/dolt/go/store/types"
@@ -164,7 +165,7 @@ func (s *perfSuite) testBuild500megBlob(p int) {
 			f := r.(*os.File)
 			err := f.Close()
 			assert.NoError(err)
-			err = os.Remove(f.Name())
+			err = file.Remove(f.Name())
 			assert.NoError(err)
 		}
 	}()

go/store/util/clienttest/client_test_suite.go

@@ -29,6 +29,7 @@ import (
 	flag "github.com/juju/gnuflag"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 	"github.com/dolthub/dolt/go/libraries/utils/osutil"
 	"github.com/dolthub/dolt/go/store/d"
 	"github.com/dolthub/dolt/go/store/util/exit"
@@ -74,7 +75,7 @@ func (suite *ClientTestSuite) SetupSuite() {
 func (suite *ClientTestSuite) TearDownSuite() {
 	suite.out.Close()
 	suite.err.Close()
-	err := os.RemoveAll(suite.TempDir)
+	err := file.RemoveAll(suite.TempDir)
 	if !osutil.IsWindows {
 		d.Chk.NoError(err)
 	}

go/store/util/tempfiles/temp_files.go

@@ -18,6 +18,8 @@ import (
 	"io/ioutil"
 	"os"
 	"sync"
+
+	"github.com/dolthub/dolt/go/libraries/utils/file"
 )
 
 // TempFileProvider is an interface which provides methods for creating temporary files.
@@ -75,7 +77,7 @@ func (tfp *TempFileProviderAt) Clean() {
 	defer tfp.mu.Unlock()
 	for _, filename := range tfp.filesCreated {
 		// best effort. ignore errors
-		_ = os.Remove(filename)
+		_ = file.Remove(filename)
 	}
 }