/go/libraries/doltcore: pr feedback

go/libraries/doltcore/dbfactory/git_remote.go

@@ -221,16 +221,17 @@ func ensureGitRemoteURL(ctx context.Context, gitDir string, remoteName string, r
 	if strings.TrimSpace(remoteURL) == "" {
 		return fmt.Errorf("empty remote url")
 	}
-	got, err := runGitInDir(ctx, gitDir, "remote", "get-url", remoteName)
+	// Insert `--` so remoteName can't be interpreted as a flag.
+	got, err := runGitInDir(ctx, gitDir, "remote", "get-url", "--", remoteName)
 	if err != nil {
 		// Remote likely doesn't exist; attempt to add.
-		return runGitInDirNoOutput(ctx, gitDir, "remote", "add", remoteName, remoteURL)
+		return runGitInDirNoOutput(ctx, gitDir, "remote", "add", "--", remoteName, remoteURL)
 	}
 	got = strings.TrimSpace(got)
 	if got == remoteURL {
 		return nil
 	}
-	return runGitInDirNoOutput(ctx, gitDir, "remote", "set-url", remoteName, remoteURL)
+	return runGitInDirNoOutput(ctx, gitDir, "remote", "set-url", "--", remoteName, remoteURL)
 }
 
 func runGitInitBare(ctx context.Context, dir string) error {

go/libraries/doltcore/sqle/dprocedures/dolt_backup.go

@@ -307,6 +307,17 @@ func syncRemote(ctx *sql.Context, dbData env.DbData[*sql.Context], dsess *dsess.
 // not AWS, it verifies that no AWS parameters are present in |apr|.
 func newParams(apr *argparser.ArgParseResults, url string, urlScheme string) (map[string]string, error) {
 	params := map[string]string{}
+
+	isGitRemote := urlScheme == dbfactory.GitFileScheme || urlScheme == dbfactory.GitHTTPScheme || urlScheme == dbfactory.GitHTTPSScheme || urlScheme == dbfactory.GitSSHScheme
+	if !isGitRemote {
+		if _, ok := apr.GetValue("git-cache-dir"); ok {
+			return nil, fmt.Errorf("error: --git-cache-dir is only supported for git remotes")
+		}
+		if _, ok := apr.GetValue("ref"); ok {
+			return nil, fmt.Errorf("error: --ref is only supported for git remotes")
+		}
+	}
+
 	var err error
 	switch urlScheme {
 	case dbfactory.AWSScheme:
@@ -326,9 +337,10 @@ func newParams(apr *argparser.ArgParseResults, url string, urlScheme string) (ma
 		}
 		if ref, ok := apr.GetValue("ref"); ok {
 			ref = strings.TrimSpace(ref)
-			if ref != "" {
-				params[dbfactory.GitRefParam] = ref
+			if ref == "" {
+				return nil, fmt.Errorf("error: --ref cannot be empty")
 			}
+			params[dbfactory.GitRefParam] = ref
 		}
 	default:
 		err = cli.VerifyNoAwsParams(apr)

go/libraries/doltcore/sqle/dprocedures/dolt_clone.go

@@ -73,12 +73,13 @@ func doltClone(ctx *sql.Context, args ...string) (sql.RowIter, error) {
 
 	if ref, ok := apr.GetValue("ref"); ok {
 		ref = strings.TrimSpace(ref)
-		if ref != "" {
-			if !isGitRemote {
-				return nil, errhand.BuildDError("error: --ref is only supported for git remotes").Build()
-			}
-			remoteParms[dbfactory.GitRefParam] = ref
+		if ref == "" {
+			return nil, errhand.BuildDError("error: --ref cannot be empty").Build()
 		}
+		if !isGitRemote {
+			return nil, errhand.BuildDError("error: --ref is only supported for git remotes").Build()
+		}
+		remoteParms[dbfactory.GitRefParam] = ref
 	}
 
 	depth, ok := apr.GetInt(cli.DepthFlag)

go/libraries/doltcore/sqle/dprocedures/dolt_remote.go

@@ -120,12 +120,13 @@ func addRemote(_ *sql.Context, dbName string, dbd env.DbData[*sql.Context], apr
 
 	if ref, ok := apr.GetValue("ref"); ok {
 		ref = strings.TrimSpace(ref)
-		if ref != "" {
-			if !isGitRemote {
-				return fmt.Errorf("error: --ref is only supported for git remotes")
-			}
-			params[dbfactory.GitRefParam] = ref
+		if ref == "" {
+			return fmt.Errorf("error: --ref cannot be empty")
 		}
+		if !isGitRemote {
+			return fmt.Errorf("error: --ref is only supported for git remotes")
+		}
+		params[dbfactory.GitRefParam] = ref
 	}
 
 	r := env.NewRemote(remoteName, absRemoteUrl, params)