Merge pull request #10327 from dolthub/aaron/lazy-temp-file-provider

go/cmd/dolt: Make movable temp file configuration lazy.

go/cmd/dolt/system_checks.go

@@ -36,69 +36,78 @@ func reconfigIfTempFileMoveFails(dataDir filesys.Filesys) error {
 		return err
 	}
 
-	dotDoltCreated := false
-	tmpDirCreated := false
+	// Configure MovableTempFileProvider so that it lazily checks if os.TempDir() can be moved from to the data directory
+	// or not. If it cannot be, this will configure .dolt/temptf as the movable temp file directory.
+	//
+	// The intent of this being lazy is that we do not want to mess with the local filesystem unless we are asked to,
+	// but the concrete way we check for moveability is to create a temp file and move it into a known subdirectory
+	// of the .dolt subdirectory. We shouldn't create any of those things unless we need to because we are actually
+	// doing filesystem writes.
+	origprovider := tempfiles.MovableTempFileProvider
+	tempfiles.MovableTempFileProvider = tempfiles.NewLazyTempFileProvider(func() (tempfiles.TempFileProvider, error) {
+		dotDoltCreated := false
+		tmpDirCreated := false
 
-	doltDir := filepath.Join(absP, dbfactory.DoltDir)
-	stat, err := os.Stat(doltDir)
-	if err != nil {
-		err := os.MkdirAll(doltDir, os.ModePerm)
+		doltDir := filepath.Join(absP, dbfactory.DoltDir)
+		stat, err := os.Stat(doltDir)
 		if err != nil {
-			return fmt.Errorf("failed to create dolt dir '%s': %s", doltDir, err.Error())
+			err := os.MkdirAll(doltDir, os.ModePerm)
+			if err != nil {
+				return nil, fmt.Errorf("failed to create dolt dir '%s': %s", doltDir, err.Error())
+			}
+
+			dotDoltCreated = true
 		}
 
-		dotDoltCreated = true
-	}
-
-	doltTmpDir := filepath.Join(doltDir, env.TmpDirName)
-	stat, err = os.Stat(doltTmpDir)
-	if err != nil {
-		err := os.MkdirAll(doltTmpDir, os.ModePerm)
+		doltTmpDir := filepath.Join(doltDir, env.TmpDirName)
+		stat, err = os.Stat(doltTmpDir)
 		if err != nil {
-			return fmt.Errorf("failed to create temp dir '%s': %s", doltTmpDir, err.Error())
-		}
-		tmpDirCreated = true
+			err := os.MkdirAll(doltTmpDir, os.ModePerm)
+			if err != nil {
+				return nil, fmt.Errorf("failed to create temp dir '%s': %s", doltTmpDir, err.Error())
+			}
+			tmpDirCreated = true
 
-	} else if !stat.IsDir() {
-		return fmt.Errorf("attempting to use '%s' as a temp directory, but there exists a file with that name", doltTmpDir)
-	}
-
-	tmpF, err := os.CreateTemp("", "")
-	if err != nil {
-		return err
-	}
-
-	name := tmpF.Name()
-	err = tmpF.Close()
-	if err != nil {
-		return err
-	}
-
-	movedName := filepath.Join(doltTmpDir, "testfile")
-
-	if os.Getenv("DOLT_FORCE_LOCAL_TEMP_FILES") == "" {
-		err = file.Rename(name, movedName)
-	} else {
-		err = errors.New("treating rename as failed because DOLT_FORCE_LOCAL_TEMP_FILES is set")
-	}
-	if err == nil {
-		// If rename was successful, then the tmp dir is fine, so no need to change it. Clean up the things we created.
-		_ = file.Remove(movedName)
-
-		if tmpDirCreated {
-			_ = file.Remove(doltTmpDir)
+		} else if !stat.IsDir() {
+			return nil, fmt.Errorf("attempting to use '%s' as a temp directory, but there exists a file with that name", doltTmpDir)
 		}
 
-		if dotDoltCreated {
-			_ = file.Remove(doltDir)
+		tmpF, err := os.CreateTemp("", "")
+		if err != nil {
+			return nil, err
 		}
 
-		return nil
-	}
-	_ = file.Remove(name)
+		name := tmpF.Name()
+		err = tmpF.Close()
+		if err != nil {
+			return nil, err
+		}
 
-	// Rename failed. So we force the tmp dir to be the data dir.
-	tempfiles.MovableTempFileProvider = tempfiles.NewTempFileProviderAt(doltTmpDir)
+		movedName := filepath.Join(doltTmpDir, "testfile")
+
+		if os.Getenv("DOLT_FORCE_LOCAL_TEMP_FILES") == "" {
+			err = file.Rename(name, movedName)
+		} else {
+			err = errors.New("treating rename as failed because DOLT_FORCE_LOCAL_TEMP_FILES is set")
+		}
+		if err == nil {
+			// If rename was successful, then the tmp dir is fine, so no need to change it. Clean up the things we created.
+			_ = file.Remove(movedName)
+
+			if tmpDirCreated {
+				_ = file.Remove(doltTmpDir)
+			}
+
+			if dotDoltCreated {
+				_ = file.Remove(doltDir)
+			}
+
+			return origprovider, nil
+		}
+		_ = file.Remove(name)
+		// Rename failed. So we force the tmp dir to be the data dir.
+		return tempfiles.NewTempFileProviderAt(doltTmpDir), nil
+	})
 
 	return nil
 }

go/store/util/tempfiles/temp_files.go

@@ -80,7 +80,55 @@ func (tfp *TempFileProviderAt) Clean() {
 	}
 }
 
-// MovableTemFile is an object that implements TempFileProvider that is used by the nbs to create temp files that
+// LazyTempFileProvider will load the TempFileProvider from |loader|
+// on first access and then return temp files based on that result
+// going forward. This is configured for the dolt process's data
+// directory to get our process-wide MovableTempFileProvider early in
+// the Dolt process's life cycle, but the required capabilities are
+// not checked for until first use.
+type LazyTempFileProvider struct {
+	once     sync.Once
+	loader   func() (TempFileProvider, error)
+	provider TempFileProvider
+	perr     error
+}
+
+func NewLazyTempFileProvider(loader func() (TempFileProvider, error)) *LazyTempFileProvider {
+	return &LazyTempFileProvider{
+		loader: loader,
+	}
+}
+
+func (p *LazyTempFileProvider) loadit() {
+	p.once.Do(func() {
+		p.provider, p.perr = p.loader()
+	})
+}
+
+func (p *LazyTempFileProvider) Clean() {
+	// Don't load if we haven't already been loaded.
+	if p.provider != nil {
+		p.provider.Clean()
+	}
+}
+
+func (p *LazyTempFileProvider) GetTempDir() string {
+	p.loadit()
+	if p.perr != nil {
+		return os.TempDir()
+	}
+	return p.provider.GetTempDir()
+}
+
+func (p *LazyTempFileProvider) NewFile(dir, pattern string) (*os.File, error) {
+	p.loadit()
+	if p.perr != nil {
+		return nil, p.perr
+	}
+	return p.provider.NewFile(dir, pattern)
+}
+
+// MovableTempFile is an object that implements TempFileProvider that is used by the nbs to create temp files that
 // ultimately will be renamed.  It is important to use this instance rather than using os.TempDir, or os.CreateTemp
 // directly as those may have errors executing a rename against if the volume the default temporary directory lives on
 // is different than the volume of the destination of the rename.