PR feedback

go/libraries/doltcore/doltdb/commit_hooks.go

@@ -61,11 +61,6 @@ func (ph *PushOnWriteHook) Execute(ctx context.Context, ds datas.Dataset, db dat
 }
 
 func (ph *PushOnWriteHook) pushDataset(ctx context.Context, ds datas.Dataset, db datas.Database) error {
-	targetHash, ok := ds.MaybeHeadAddr()
-	if !ok {
-		return fmt.Errorf("dataset empty")
-	}
-
 	addr, ok := ds.MaybeHeadAddr()
 	if !ok {
 		_, err := ph.destDB.Delete(ctx, ds)
@@ -81,7 +76,7 @@ func (ph *PushOnWriteHook) pushDataset(ctx context.Context, ds datas.Dataset, db
 	destCS := datas.ChunkStoreFromDatabase(ph.destDB)
 	waf := types.WalkAddrsForNBF(ph.fmt)
 
-	err = pull.Pull(ctx, srcCS, destCS, waf, targetHash, nil)
+	err = pull.Pull(ctx, srcCS, destCS, waf, addr, nil)
 	if err != nil {
 		return err
 	}

go/libraries/doltcore/env/remotes.go

@@ -21,7 +21,6 @@ import (
 	"net/url"
 	"path"
 	"path/filepath"
-	"runtime"
 	"sort"
 	"strings"
 
@@ -471,7 +470,6 @@ func GetAbsRemoteUrl(fs filesys2.Filesys, cfg config.ReadableConfig, urlArg stri
 }
 
 func getAbsFileRemoteUrl(u *url.URL, fs filesys2.Filesys) (string, error) {
-
 	urlStr := u.Host + u.Path
 	scheme := u.Scheme
 
@@ -486,6 +484,8 @@ func getAbsFileRemoteUrl(u *url.URL, fs filesys2.Filesys) (string, error) {
 	exists, isDir := fs.Exists(urlStr)
 
 	if !exists {
+		// TODO: very odd that GetAbsRemoteUrl will create a directory if it doesn't exist.
+		//  This concern should be separated
 		err = fs.MkDirs(urlStr)
 		if err != nil {
 			return "", fmt.Errorf("failed to create directory '%s': %w", urlStr, err)
@@ -494,22 +494,10 @@ func getAbsFileRemoteUrl(u *url.URL, fs filesys2.Filesys) (string, error) {
 		return "", filesys2.ErrIsFile
 	}
 
-	urlStr = strings.ReplaceAll(urlStr, `\`, "/")
-	if !strings.HasPrefix(urlStr, "/") && !isDriveLetterPath(urlStr) {
-		urlStr = "/" + urlStr
-	}
-
+	urlStr = filepath.ToSlash(urlStr)
 	return scheme + "://" + urlStr, nil
 }
 
-func isDriveLetter(b byte) bool {
-	return 'a' <= b && b <= 'z' || 'A' <= b && b <= 'Z'
-}
-
-func isDriveLetterPath(name string) bool {
-	return runtime.GOOS == "windows" && len(name) >= 3 && isDriveLetter(name[0]) && name[1] == ':' && name[2] == '/'
-}
-
 // GetDefaultBranch returns the default branch from among the branches given, returning
 // the configs default config branch first, then init branch main, then the old init branch master,
 // and finally the first lexicographical branch if none of the others are found

go/libraries/doltcore/sqle/database_provider.go

@@ -364,13 +364,17 @@ func (p DoltDatabaseProvider) configureReplication(ctx *sql.Context, name string
 		return nil
 	}
 
+	urlTemplate, ok := remoteUrlTemplate.(string)
+	if !ok {
+		return nil
+	}
+
 	// TODO: url sanitize
-	remoteUrl := fmt.Sprintf(remoteUrlTemplate.(string), name)
+	remoteUrl := strings.Replace(urlTemplate, dsess.URLTemplateDatabasePlaceholder, name, 1)
 
 	// TODO: params for AWS, others that need them
 	r := env.NewRemote(remoteName, remoteUrl, nil)
-	// TODO: use the correct format here, should match created DB
-	err := r.Prepare(ctx, types.Format_Default, p.remoteDialer)
+	err := r.Prepare(ctx, newEnv.DoltDB.Format(), p.remoteDialer)
 	if err != nil {
 		return err
 	}

go/libraries/doltcore/sqle/dsess/variables.go

@@ -49,6 +49,8 @@ const (
 	AwsCredsRegion                = "aws_credentials_region"
 )
 
+const URLTemplateDatabasePlaceholder = "{database}"
+
 // DefineSystemVariablesForDB defines per database dolt-session variables in the engine as necessary
 func DefineSystemVariablesForDB(name string) {
 	if _, _, ok := sql.SystemVariables.GetGlobal(name + HeadKeySuffix); !ok {