From f54449dbb2ef412b7ddb5f5c1d3453f9ac1b95a1 Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Mon, 23 Jun 2025 16:41:10 -0700 Subject: [PATCH] go: clone: Fix dolt clone to work even when dolt startup logic detects that TMPDIR is not renamable to the local directory and it creates a .dolt/tmp directory. --- go/cmd/dolt/system_checks.go | 7 +- go/libraries/doltcore/env/actions/clone.go | 13 ++- go/libraries/doltcore/env/environment.go | 94 +++++++++++++++------- integration-tests/bats/remotes.bats | 15 ++++ 4 files changed, 92 insertions(+), 37 deletions(-) diff --git a/go/cmd/dolt/system_checks.go b/go/cmd/dolt/system_checks.go index bffeff96d1b..ff974fc8f8b 100644 --- a/go/cmd/dolt/system_checks.go +++ b/go/cmd/dolt/system_checks.go @@ -15,6 +15,7 @@ package main import ( + "errors" "fmt" "os" "path/filepath" @@ -75,7 +76,11 @@ func reconfigIfTempFileMoveFails(dataDir filesys.Filesys) error { movedName := filepath.Join(doltTmpDir, "testfile") - err = file.Rename(name, movedName) + 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) diff --git a/go/libraries/doltcore/env/actions/clone.go b/go/libraries/doltcore/env/actions/clone.go index 815629eb488..20780aa12b0 100644 --- a/go/libraries/doltcore/env/actions/clone.go +++ b/go/libraries/doltcore/env/actions/clone.go @@ -18,7 +18,6 @@ import ( "context" "errors" "fmt" - "path/filepath" "sort" "strings" "sync" @@ -26,7 +25,6 @@ import ( "github.com/dustin/go-humanize" "github.com/dolthub/dolt/go/cmd/dolt/cli" - "github.com/dolthub/dolt/go/libraries/doltcore/dbfactory" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" "github.com/dolthub/dolt/go/libraries/doltcore/ref" @@ -58,13 +56,14 @@ var ErrCloneFailed = errors.New("clone failed") // EnvForClone creates a new DoltEnv and configures it with repo state from the specified remote. The returned DoltEnv is ready for content to be cloned into it. The directory used for the new DoltEnv is determined by resolving the specified dir against the specified Filesys. func EnvForClone(ctx context.Context, nbf *types.NomsBinFormat, r env.Remote, dir string, fs filesys.Filesys, version string, homeProvider env.HomeDirProvider) (*env.DoltEnv, error) { - exists, _ := fs.Exists(filepath.Join(dir, dbfactory.DoltDir)) - - if exists { - return nil, fmt.Errorf("%w: %s", ErrRepositoryExists, dir) + canCreate, err := env.CanCreateDatabaseAtPath(fs, dir) + if !canCreate { + if errors.Is(err, env.ErrCannotCreateDoltDirAlreadyExists) { + return nil, fmt.Errorf("%w: %s", ErrRepositoryExists, dir) + } } - err := fs.MkDirs(dir) + err = fs.MkDirs(dir) if err != nil { return nil, fmt.Errorf("%w: %s; %s", ErrFailedToCreateDirectory, dir, err.Error()) } diff --git a/go/libraries/doltcore/env/environment.go b/go/libraries/doltcore/env/environment.go index c36a2bca384..75ffba39a19 100644 --- a/go/libraries/doltcore/env/environment.go +++ b/go/libraries/doltcore/env/environment.go @@ -18,7 +18,6 @@ import ( "context" "errors" "fmt" - "os" "path/filepath" "strings" "sync" @@ -472,40 +471,77 @@ func (dEnv *DoltEnv) InitRepoWithNoData(ctx context.Context, nbf *types.NomsBinF return err } -func (dEnv *DoltEnv) createDirectories(dir string) (string, error) { - absPath, err := dEnv.FS.Abs(dir) +var ErrCannotCreateDirDoesNotExist = errors.New("dir does not exist") +var ErrCannotCreateDirPathIsFile = errors.New("dir path is a file") +var ErrCannotCreateDoltDirAlreadyExists = errors.New(".dolt dir already exists") +// Returns |true, nil| if it is allowed to create a database at the given |dir|. +// Returns |false| and potentially an error if it is not allowed. +// +// For the purposes of this function, it is allowed to create a +// database at |dir| if: +// +// * |dir| itself already exists and is a directory. +// * |dir|/.dolt does not exist, or +// * |dir|/.dolt exists and is a directory and is empty, or +// * |dir|/.dolt exists and is a directory and has only one other entry in it, a directory with name "tmp", or +// * |dir|/.dolt exists and is a directory and has only one other entry in it, a file with name "config.json", or +// * |dir|/.dolt exists and is a directory and contains both a |tmp| directory and a |config.json| file and nothing else. +func CanCreateDatabaseAtPath(fs filesys.Filesys, dir string) (bool, error) { + absPath, err := fs.Abs(dir) if err != nil { - return "", err - } - - exists, isDir := dEnv.FS.Exists(absPath) + return false, err + } + dirExists, dirIsDir := fs.Exists(absPath) + if !dirExists { + return false, fmt.Errorf("%w: '%s' does not exist so could not create '%s", ErrCannotCreateDirDoesNotExist, absPath, dbfactory.DoltDataDir) + } else if !dirIsDir { + return false, fmt.Errorf("%w: '%s' exists but it's a file not a directory", ErrCannotCreateDirPathIsFile, absPath) + } + doltDirPath := filepath.Join(absPath, dbfactory.DoltDir) + doltDirExists, doltDirIsDir := fs.Exists(doltDirPath) + if doltDirExists { + if !doltDirIsDir { + return false, fmt.Errorf("%w: '%s' exists but is a file, not a directory", ErrCannotCreateDirPathIsFile, doltDirPath) + } + tmpPath := filepath.Join(doltDirPath, TmpDirName) + configPath := filepath.Join(doltDirPath, configFile) + isOK := true + err := fs.Iter(doltDirPath, true, func(path string, sz int64, isDir bool) (stop bool) { + if path == doltDirPath { + return false + } else if path == tmpPath && isDir { + return false + } else if path == configPath && !isDir { + return false + } else { + isOK = false + return true + } + }) + if err != nil { + return false, err + } + if !isOK { + return false, fmt.Errorf("%w: .dolt directory already exists at '%s'", ErrCannotCreateDoltDirAlreadyExists, dir) + } - if !exists { - return "", fmt.Errorf("'%s' does not exist so could not create '%s", absPath, dbfactory.DoltDataDir) - } else if !isDir { - return "", fmt.Errorf("'%s' exists but it's a file not a directory", absPath) } + return true, nil +} - if dEnv.hasDoltDir(dir) { - // Special case a completely empty directory, or one which has a "tmp" directory but nothing else. - // The `.dolt/tmp` directory is created while verifying that we can rename table files which is early - // in the startup process. It will only exist if we need it because the TMPDIR environment variable is set to - // a path which is on a different partition than the .dolt directory. - dotDolt := mustAbs(dEnv, dbfactory.DoltDir) - entries, err := os.ReadDir(dotDolt) - if err != nil { - return "", err - } +func (dEnv *DoltEnv) createDirectories(dir string) (string, error) { + canCreate, err := CanCreateDatabaseAtPath(dEnv.FS, dir) + if err != nil { + return "", err + } + if !canCreate { + return "", fmt.Errorf("cannot create a dolt database at '%s'", dir) + } - if len(entries) == 1 { - entry := entries[0] - if (entry.IsDir() && entry.Name() != TmpDirName) || (!entry.IsDir() && entry.Name() != configFile) { - return "", fmt.Errorf(".dolt directory already exists at '%s'", dir) - } - } else if len(entries) != 0 { - return "", fmt.Errorf(".dolt directory already exists at '%s'", dir) - } + absPath, err := dEnv.FS.Abs(dir) + if err != nil { + return "", err } absDataDir := filepath.Join(absPath, dbfactory.DoltDataDir) diff --git a/integration-tests/bats/remotes.bats b/integration-tests/bats/remotes.bats index 498db06d766..376256cc6f2 100644 --- a/integration-tests/bats/remotes.bats +++ b/integration-tests/bats/remotes.bats @@ -2021,3 +2021,18 @@ SQL [ "$status" -eq 0 ] [[ "$output" =~ "new branch" ]] || false } + +@test "remotes: can clone to . when local temp files are being used" { + mkdir toclone + + mkdir topush + cd topush + dolt init + dolt remote add origin file://../toclone + dolt push origin main:main + cd .. + + mkdir dest + cd dest + env DOLT_FORCE_LOCAL_TEMP_FILES=1 dolt clone file://../toclone . +}