amend dolt_backup impl to work in non-dolt directories

Author: elianddb

go/cmd/dolt/commands/backup.go

@@ -29,6 +29,8 @@ import (
 	eventsapi "github.com/dolthub/eventsapi_schema/dolt/services/eventsapi/v1alpha1"
 )
 
+const DoltBackupCommandName = "backup"
+
 var backupDocs = cli.CommandDocumentationContent{
 	ShortDesc: "Manage database backups, including creation, sync, and restore.",
 	LongDesc: `

go/cmd/dolt/dolt.go

@@ -680,8 +680,14 @@ If you're interested in running this command against a remote host, hit us up on
 	//
 	// This is also allowed when --help is passed. So we defer the error
 	// until the caller tries to use the cli.LateBindQueryist.
-	isValidRepositoryRequired := subcommandName != "init" && subcommandName != "sql" && subcommandName != "sql-server" && subcommandName != "sql-client"
-
+	commandsNotRequiringRepo := map[string]bool{
+		"init":                         true,
+		"sql":                          true,
+		"sql-server":                   true,
+		"sql-client":                   true,
+		commands.DoltBackupCommandName: true,
+	}
+	isValidRepositoryRequired := !commandsNotRequiringRepo[subcommandName]
 	if noValidRepository && isValidRepositoryRequired {
 		return func(ctx context.Context, opts ...cli.LateBindQueryistOption) (res cli.LateBindQueryistResult, err error) {
 			err = errors.New("The current directory is not a valid dolt repository.")

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

@@ -31,6 +31,7 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqlserver"
 	"github.com/dolthub/dolt/go/libraries/utils/argparser"
 	"github.com/dolthub/dolt/go/store/datas/pull"
+	"github.com/dolthub/dolt/go/store/types"
 )
 
 const (
@@ -55,17 +56,25 @@ var awsParamsUsage = []string{
 // based on the first argument. The procedure requires superuser privileges and write access to the current database.
 // Supported operations are: add, remove/rm, sync, sync-url, and restore.
 func doltBackup(ctx *sql.Context, args ...string) (sql.RowIter, error) {
-	dbName := ctx.GetCurrentDatabase()
-	if dbName == "" {
-		return nil, fmt.Errorf("empty database name")
-	}
-
-	err := branch_control.CheckAccess(ctx, branch_control.Permissions_Write)
+	apr, err := cli.CreateBackupArgParser().Parse(args)
 	if err != nil {
 		return nil, err
 	}
 
-	apr, err := cli.CreateBackupArgParser().Parse(args)
+	if apr.NArg() == 0 || (apr.NArg() == 1 && apr.Contains(cli.VerboseFlag)) {
+		return nil, fmt.Errorf("use '%s' table to list backups", doltdb.BackupsTableName)
+	}
+
+	var dbName string
+	funcParam := apr.Arg(0)
+	if funcParam != DoltBackupParamRestore {
+		dbName = ctx.GetCurrentDatabase()
+		if dbName == "" {
+			return nil, fmt.Errorf("empty database name")
+		}
+	}
+
+	err = branch_control.CheckAccess(ctx, branch_control.Permissions_Write)
 	if err != nil {
 		return nil, err
 	}
@@ -82,17 +91,12 @@ func doltBackup(ctx *sql.Context, args ...string) (sql.RowIter, error) {
 		}
 	}
 
-	if apr.NArg() == 0 || (apr.NArg() == 1 && apr.Contains(cli.VerboseFlag)) {
-		return nil, fmt.Errorf("use '%s' table to list backups", doltdb.BackupsTableName)
-	}
-
 	doltSess := dsess.DSessFromSess(ctx.Session)
 	dbData, ok := doltSess.GetDbData(ctx, dbName)
-	if !ok {
+	if !ok && funcParam != DoltBackupParamRestore {
 		return nil, sql.ErrDatabaseNotFound.New(dbName)
 	}
 
-	funcParam := apr.Arg(0)
 	switch funcParam {
 	case DoltBackupParamAdd:
 		if apr.NArg() != 3 {
@@ -225,7 +229,14 @@ func doltBackupRestore(ctx *sql.Context, dbData env.DbData[*sql.Context], dsess
 	}
 
 	remote := env.NewRemote(DoltBackupParamRestore, remoteUrl, remoteParams)
-	remoteDb, err := dsess.Provider().GetRemoteDB(ctx, dbData.Ddb.Format(), remote, true)
+
+	// Use default format if no database context is available (e.g., when run from invalid directory).
+	format := types.Format_Default
+	if dbData.Ddb != nil {
+		format = dbData.Ddb.Format()
+	}
+
+	remoteDb, err := dsess.Provider().GetRemoteDB(ctx, format, remote, true)
 	if err != nil {
 		return err
 	}
@@ -247,7 +258,7 @@ func doltBackupRestore(ctx *sql.Context, dbData env.DbData[*sql.Context], dsess
 		}
 	}
 
-	if lookupDbInFileSys {
+	if lookupDbInFileSys && !hasLookupDb {
 		err = fileSys.Delete(lookupDbName, forceRestore)
 		if err != nil {
 			return err

integration-tests/bats/helper/local-remote.bash

@@ -68,7 +68,6 @@ SKIP_SERVER_TESTS=$(cat <<-EOM
 ~arg-parsing.bats~
 ~dump.bats~
 ~rename-tables.bats~
-~sql-backup.bats~
 ~drop-create.bats~
 ~constraint-violations.bats~
 ~branch-control.bats~

integration-tests/bats/sql-backup.bats

@@ -7,15 +7,16 @@ setup() {
 }
 
 teardown() {
-    stop_sql_server
     teardown_common
 }
 
 @test "sql-backup: dolt_backup no argument" {
     run dolt sql -q "call dolt_backup()"
     [ "$status" -ne 0 ]
+    [[ "$output" =~ "use 'dolt_backups' table to list backups" ]]
     run dolt sql -q "CALL dolt_backup()"
     [ "$status" -ne 0 ]
+    [[ "$output" =~ "use 'dolt_backups' table to list backups" ]]
 }
 
 @test "sql-backup: dolt_backup add" {
@@ -28,16 +29,16 @@ teardown() {
 }
 
 @test "sql-backup: dolt_backup add cannot add remote with address of existing backup" {
-    mkdir bac1
-    dolt sql -q "call dolt_backup('add','bac1','file://./bac1')"
-    run dolt sql -q "call dolt_backup('add','rem1','file://./bac1')"
+    backupFileUrl="file://$BATS_TEST_TMPDIR/backup"
+    dolt sql -q "call dolt_backup('add','bac1', '$backupFileUrl')"
+    run dolt sql -q "call dolt_backup('add','rem1', '$backupFileUrl')"
     [ "$status" -eq 1 ]
     [[ "$output" =~ "address conflict with a remote: 'bac1'" ]] || false
 }
 
 @test "sql-backup: dolt_backup remove" {
-    mkdir bac1
-    dolt sql -q "call dolt_backup('add', 'bac1', 'file://./bac1')"
+    backupFileUrl="file://$BATS_TEST_TMPDIR/backup"
+    dolt sql -q "call dolt_backup('add', 'bac1', '$backupFileUrl')"
     run dolt backup -v
     [ "$status" -eq 0 ]
     [ "${#lines[@]}" -eq 1 ]
@@ -61,8 +62,8 @@ teardown() {
 }
 
 @test "sql-backup: dolt_backup rm" {
-    mkdir bac1
-    dolt sql -q "call dolt_backup('add', 'bac1', 'file://./bac1')"
+    backupFileUrl="files://$BATS_TEST_TMPDIR/backup"
+    dolt sql -q "call dolt_backup('add', 'bac1', '$backupFileUrl')"
     run dolt backup -v
     [ "$status" -eq 0 ]
     [ "${#lines[@]}" -eq 1 ]
@@ -93,18 +94,18 @@ teardown() {
 }
 
 @test "sql-backup: dolt_backup restore" {
-    backupsDir="$PWD/backups"
+    backupFileUrl="file://$BATS_TEST_TMPDIR/backups"
     mkdir backupsDir
 
     # Created a nested database, back it up, drop it, then restore it with a new name
     dolt sql -q "create database db1;"
     cd db1
     dolt sql -q "create table t1 (pk int primary key); insert into t1 values (42); call dolt_commit('-Am', 'creating table t1');"
-    dolt sql -q "call dolt_backup('add', 'backups', 'file://$backupsDir');"
+    dolt sql -q "call dolt_backup('add', 'backups', '$backupFileUrl');"
     dolt sql -q "call dolt_backup('sync', 'backups');"
     cd ..
     dolt sql -q "drop database db1;"
-    dolt sql -q "call dolt_backup('restore', 'file://$backupsDir', 'db2');"
+    dolt sql -q "call dolt_backup('restore', '$backupFileUrl', 'db2');"
 
     # Assert that db2 is present, and db1 is not
     run dolt sql -q "show databases;"
@@ -124,33 +125,52 @@ teardown() {
     [ "${#lines[@]}" -eq 0 ]
 }
 
-@test "sql-backup: dolt_backup restore --force" {
-    backupsDir="$PWD/backups"
-    mkdir backupsDir
+@test "sql-backup: dolt_backup restore --force on current database" {
+    backupFileUrl="file://$BATS_TEST_TMPDIR/backups"
 
     # Created a nested database, and back it up
     dolt sql -q "create database db1;"
-    cd db1
-    dolt sql -q "create table t1 (pk int primary key); insert into t1 values (42); call dolt_commit('-Am', 'creating table t1');"
-    dolt sql -q "call dolt_backup('add', 'backups', 'file://$backupsDir');"
-    dolt sql -q "call dolt_backup('sync', 'backups');"
+    # We could cd into db1 but Windows does not like us touching its CWD when we drop the database when restoring.
+    dolt sql -q "use db1; create table t1 (pk int primary key); insert into t1 values (42); call dolt_commit('-Am', 'creating table t1');"
+    dolt sql -q "use db1; call dolt_backup('add', 'backups', '$backupFileUrl');"
+    dolt sql -q "use db1; call dolt_backup('sync', 'backups');"
 
     # Make a new commit in db1, but don't push it to the backup
-    dolt sql -q "update t1 set pk=100; call dolt_commit('-Am', 'updating table t1');"
+    dolt sql -q "use db1; update t1 set pk=100; call dolt_commit('-Am', 'updating table t1');"
 
     # Assert that without --force, we can't update an existing db from a backup
-    run dolt sql -q "call dolt_backup('restore', 'file://$backupsDir', 'db1');"
+    run dolt sql -q "use db1; call dolt_backup('restore', '$backupFileUrl', 'db1');"
     [ "$status" -eq 1 ]
     [[ "$output" =~ "database 'db1' already exists, use '--force' to overwrite" ]] || false
 
     # Use --force to overwrite the existing database and sanity check the data
-    run dolt sql -q "call dolt_backup('restore', '--force', 'file://$backupsDir', 'db1');"
+    run dolt sql -q "use db1; call dolt_backup('restore', '--force', '$backupFileUrl', 'db1');"
     [ "$status" -eq 0 ]
     run dolt sql -q "use db1; select * from t1;"
     [ "$status" -eq 0 ]
     [[ "$output" =~ "42" ]] || false
 }
 
+@test "sql-backup: dolt_backup restore --force on current database and as cwd" {
+    skiponwindows "Windows storage system locks the terminal cwd when trying to drop database in restore procedure; this includes mounted storage in WSL"
+    backupFileUrl="$BATS_TEST_TMPDIR/backup"
+    dolt sql -q "create database db1;"
+    cd db1
+    dolt sql <<'EOF'
+create table t (i int);
+insert into t values (3), (4);
+dolt_backup('sync-url', '$fileBackupUrl');
+EOF
+
+    run dolt sql -q "call dolt_backup('restore', '', 'db1');"
+    [ "$status" -eq 1 ]
+    [[ "$output" =~ "database 'db1' already exists, use '--force' to overwrite" ]] || false
+
+    run dolt sql -q "call dolt_backup('restore', '$backupFileUrl', 'db1');"
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ i.*3.*4 ]]
+}
+
 @test "sql-backup: dolt_backup unrecognized" {
     run dolt sql -q "call dolt_backup('unregonized', 'hostedapidb-0', 'file:///some_directory')"
     [ "$status" -ne 0 ]
@@ -177,37 +197,31 @@ teardown() {
 }
 
 @test "sql-backup: dolt_backup sync to a backup" {
-    mkdir the_backup
-    dolt backup add hostedapidb-0 file://./the_backup
+    backupFileUrl="file://$BATS_TEST_TMPDIR/the_backup"
+    dolt backup add hostedapidb-0 "$backupFileUrl"
     dolt backup -v
     dolt sql -q "call dolt_backup('sync', 'hostedapidb-0')"
     # Initial backup works.
-    dolt backup restore file://./the_backup the_restore
+    dolt backup restore "$backupFileUrl" the_restore
     (cd the_restore && dolt status)
     # Backup with nothing to push works.
     dolt sql -q "call dolt_backup('sync', 'hostedapidb-0')"
 
-    rm -rf the_backup the_restore
-
-    mkdir the_backup
     dolt sql -q "CALL dolt_backup('sync', 'hostedapidb-0')"
-    dolt backup restore file://./the_backup the_restore
+    dolt backup restore "$backupFileUrl" the_restore --force
     (cd the_restore && dolt status)
     dolt sql -q "CALL dolt_backup('sync', 'hostedapidb-0')"
 }
 
 @test "sql-backup: dolt_backup sync-url" {
-    mkdir the_backup
-    dolt sql -q "call dolt_backup('sync-url', 'file://./the_backup')"
+    backupFileUrl="file://$BATS_TEST_TMPDIR/the_backup"
+    dolt sql -q "call dolt_backup('sync-url', '$backupFileUrl')"
     # Initial backup works.
-    dolt backup restore file://./the_backup the_restore
+    dolt backup restore "$backupFileUrl" the_restore
     (cd the_restore && dolt status)
 
-    rm -rf the_backup the_restore
-
-    mkdir the_backup
-    dolt sql -q "CALL dolt_backup('sync-url', 'file://./the_backup')"
-    dolt backup restore file://./the_backup the_restore
+    dolt sql -q "CALL dolt_backup('sync-url', '$backupFileUrl')"
+    dolt backup restore "$backupFileUrl" the_restore --force
     (cd the_restore && dolt status)
 }
 
@@ -219,6 +233,7 @@ teardown() {
 }
 
 @test "sql-backup: dolt_backup rejects AWS parameters fails in sql-server" {
+    skip_if_remote
     start_sql_server
 
     run dolt sql -q "call dolt_backup('add', 'backup1', 'aws://[table:bucket]/db', '--aws-region=us-east-1')"
@@ -236,4 +251,27 @@ teardown() {
     run dolt sql -q "call dolt_backup('add', 'backup4', 'aws://[table:bucket]/db', '--aws-creds-profile=profile')"
     [ "$status" -eq 1 ]
     [[ "$output" =~ "AWS parameters are unavailable when running in server mode" ]] || false
+
+    stop_sql_server 1
+}
+
+@test "sql-backup: dolt_backup works in invalid dolt repository" {
+    backupFileUrl="file://$BATS_TEST_TMPDIR/t_backup"
+    run dolt sql -q "create table t (i int);"
+    [ "$status" -eq 0 ]
+    run dolt sql -q "insert into t values (4), (3);"
+    [ "$status" -eq 0 ]
+    run dolt sql -q "call dolt_backup('sync-url', '$backupFileUrl')"
+    [ "$status" -eq 0 ]
+
+    invalidRepoDir="$BATS_TEST_TMPDIR/invalid_repo"
+    mkdir -p "$invalidRepoDir"
+    cd $invalidRepoDir
+    dolt sql -q "call dolt_backup('restore', '$backupFileUrl', 't_db')"
+    [ "$status" -eq 0 ]
+
+    cd t_db
+    run dolt sql --result-format csv -q "select * from t"
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ i.*3.*4 ]]
 }