Improve Error Handling for Tags

go/libraries/doltcore/env/actions/branch.go

@@ -365,14 +365,15 @@ func createBranch(ctx context.Context, dbData env.DbData, newBranch, startingPoi
 var emptyHash = hash.Hash{}
 
 func IsBranch(ctx context.Context, ddb *doltdb.DoltDB, str string) (bool, error) {
-	return IsBranchOnDB(ctx, ddb, str)
-}
-
-func IsBranchOnDB(ctx context.Context, ddb *doltdb.DoltDB, str string) (bool, error) {
 	dref := ref.NewBranchRef(str)
 	return ddb.HasRef(ctx, dref)
 }
 
+func IsTag(ctx context.Context, ddb *doltdb.DoltDB, str string) (bool, error) {
+	tRef := ref.NewTagRef(str)
+	return ddb.HasRef(ctx, tRef)
+}
+
 func MaybeGetCommit(ctx context.Context, dEnv *env.DoltEnv, str string) (*doltdb.Commit, error) {
 	cs, err := doltdb.NewCommitSpec(str)
 

go/libraries/doltcore/env/actions/clone.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"path/filepath"
 	"sort"
+	"strings"
 	"sync"
 
 	"github.com/dustin/go-humanize"
@@ -169,6 +170,12 @@ func CloneRemote(ctx context.Context, srcDB *doltdb.DoltDB, remoteName, branch s
 	if err != nil {
 		return fmt.Errorf("%w; %s", ErrCloneFailed, err.Error())
 	}
+	// prevent cloning tags (branches in detached head state)
+	for _, srcRef := range srcRefHashes {
+		if srcRef.Ref.GetType() == ref.TagRefType && strings.EqualFold(srcRef.Ref.GetPath(), branch) {
+			return doltdb.ErrOperationNotSupportedInDetachedHead
+		}
+	}
 	if remoteName == "" {
 		remoteName = "origin"
 	}

go/libraries/doltcore/env/actions/workspace.go

@@ -36,7 +36,7 @@ func CreateWorkspace(ctx context.Context, dEnv *env.DoltEnv, name, startPoint st
 }
 
 func CreateWorkspaceOnDB(ctx context.Context, ddb *doltdb.DoltDB, name, startPoint string, headRef ref.DoltRef) error {
-	isBranch, err := IsBranchOnDB(ctx, ddb, name)
+	isBranch, err := IsBranch(ctx, ddb, name)
 	if err != nil {
 		return err
 	}

go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go

@@ -290,15 +290,30 @@ func checkoutRemoteBranch(ctx *sql.Context, dSess *dsess.DoltSession, dbName str
 	}
 
 	if len(remoteRefs) == 0 {
-		if doltdb.IsValidCommitHash(branchName) && apr.Contains(cli.MoveFlag) {
+		if isTag, err := actions.IsTag(ctx, dbData.Ddb, branchName); err != nil {
+			return "", err
+		} else if isTag {
+			// User tried to enter a detached head state, which we don't support.
+			// Inform and suggest that they check-out a new branch at this tag instead.
+			if apr.Contains(cli.MoveFlag) {
+				return "", fmt.Errorf(`dolt does not support a detached head state. To create a branch at this tag, run: 
+	dolt checkout %s -b {new_branch_name}`, branchName)
+			} else {
+				return "", fmt.Errorf(`dolt does not support a detached head state. To create a branch at this tag, run: 
+	CALL DOLT_CHECKOUT('%s', '-b', <new_branch_name>)`, branchName)
+			}
+		}
 
+		if doltdb.IsValidCommitHash(branchName) {
 			// User tried to enter a detached head state, which we don't support.
 			// Inform and suggest that they check-out a new branch at this commit instead.
-
-			return "", fmt.Errorf(`dolt does not support a detached head state. To create a branch at this commit instead, run:
-
-	dolt checkout %s -b {new_branch_name}
-`, branchName)
+			if apr.Contains(cli.MoveFlag) {
+				return "", fmt.Errorf(`dolt does not support a detached head state. To create a branch at this commit instead, run:
+	dolt checkout %s -b {new_branch_name}`, branchName)
+			} else {
+				return "", fmt.Errorf(`dolt does not support a detached head state. To create a branch at this commit instead, run:
+	CALL DOLT_CHECKOUT('%s', '-b', <new_branch_name>)`, branchName)
+			}
 		}
 		return "", fmt.Errorf("error: could not find %s", branchName)
 	} else if len(remoteRefs) == 1 {

go/libraries/doltcore/sqle/enginetest/dolt_queries.go

@@ -4678,6 +4678,36 @@ var DoltTagTestScripts = []queries.ScriptTest{
 			},
 		},
 	},
+	{
+		Name: "dolt-tag: checkout errors",
+		SetUpScript: []string{
+			"CREATE TABLE test(pk int primary key);",
+			"CALL DOLT_COMMIT('-Am','created table test');",
+			"CALL DOLT_TAG('v1');",
+			"INSERT INTO test VALUES (0),(1),(2);",
+			"CALL DOLT_COMMIT('-am','inserted rows into test');",
+		},
+		Assertions: []queries.ScriptTestAssertion{
+			{
+				Query: "SELECT tag_name FROM dolt_tags",
+				Expected: []sql.Row{
+					{"v1"},
+				},
+			},
+			{
+				Query: "select * from test;",
+				Expected: []sql.Row{
+					{0},
+					{1},
+					{2},
+				},
+			},
+			{
+				Query:          "call dolt_checkout('v1');",
+				ExpectedErrStr: "dolt does not support a detached head state. To create a branch at this tag, run: \n\tCALL DOLT_CHECKOUT('v1', '-b', <new_branch_name>)",
+			},
+		},
+	},
 }
 
 var DoltRemoteTestScripts = []queries.ScriptTest{

integration-tests/bats/commit_tags.bats

@@ -66,7 +66,7 @@ teardown() {
 @test "commit_tags: checkout a tag" {
     dolt branch comp HEAD^
     dolt tag v1 HEAD^
-    skip "need to implelement detached head first"
+    skip "need to implement detached head first"
     run dolt checkout v1
     [ $status -eq 0 ]
     run dolt diff comp
@@ -75,8 +75,8 @@ teardown() {
 }
 
 @test "commit_tags: commit onto checked out tag" {
-    dolt tag v1 HEAD^
     skip "need to implement detached head first"
+    dolt tag v1 HEAD^
     dolt checkout v1
     run dolt sql -q "insert into test values (8),(9)"
     [ $status -eq 0 ]
@@ -94,12 +94,12 @@ teardown() {
 }
 
 @test "commit_tags: use a tag as a ref for merge" {
-    dolt tag v1 HEAD
-    # TODO: remove this once dolt checkout is migrated
     if [ "$SQL_ENGINE" = "remote-engine" ]; then
-      skip "This test relies on dolt checkout, which has not been migrated yet."
+      skip "needs checkout which is unsupported for remote-engine"
     fi
-    dolt checkout -b other HEAD^
+    dolt tag v1 HEAD
+    dolt branch other HEAD~
+    dolt checkout other
     dolt sql -q "insert into test values (8),(9)"
     dolt add -A && dolt commit -m 'made changes'
     run dolt merge v1 -m "merge v1"
@@ -126,18 +126,35 @@ teardown() {
     dolt sql -q "insert into test values (7),(8),(9);"
     dolt add -A && dolt commit -m "more rows"
 
-    dolt remote add origin file://../remote
+    dolt remote add origin file://./../remote
     dolt push origin main
-    cd .. && dolt clone file://remote repo_clone && cd repo
 
     run dolt tag v1 HEAD^
     [ $status -eq 0 ]
     run dolt tag v2 HEAD -m "SAMO"
     [ $status -eq 0 ]
 
-    skip "todo"
-    run dolt push origin master
+    # tags are not pushed by default
+    run dolt push origin main
+    [ $status -eq 0 ]
+    [[ "$output" =~ "Everything up-to-date" ]] || false
+
+    cd .. && dolt clone file://./remote repo_clone && cd repo
+
+    cd ../repo_clone
+    run dolt pull --no-edit
+    [ $status -eq 0 ]
+    run dolt tag
+    [ $status -eq 0 ]
+    [[ ! "$output" =~ "v1" ]] || false
+    [[ ! "$output" =~ "v2" ]] || false
+
+    cd ../repo
+    run dolt push origin v1
     [ $status -eq 0 ]
+    run dolt push origin v2
+    [ $status -eq 0 ]
+
     cd ../repo_clone
     run dolt pull --no-edit
     [ $status -eq 0 ]
@@ -150,6 +167,28 @@ teardown() {
     [[ "$output" =~ "SAMO" ]] || false
 }
 
+@test "commit_tags: prevent cloning tags" {
+    # reset env
+    rm -rf .dolt
+    mkdir repo remote
+    cd repo
+
+    dolt init
+    dolt sql -q "create table test (pk int primary key);"
+    dolt sql -q "insert into test values (0),(1),(2);"
+    dolt add -A && dolt commit -m "table test"
+    dolt tag v1 HEAD
+
+    dolt remote add origin file://./../remote
+    dolt push origin main
+    dolt push origin v1
+
+    cd ..
+    run dolt clone --branch v1 file://./remote repo_clone
+    [ $status -eq 1 ]
+    [[ "$output" =~ "this operation is not supported while in a detached head state" ]] || false
+}
+
 @test "commit_tags: create a tag with semver string" {
     dolt tag v1.0.0 HEAD^