Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add --single-branch flag for dolt clone #7088

Merged
merged 6 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions go/cmd/dolt/cli/arg_parser_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func CreateCloneArgParser() *argparser.ArgParser {
ap.SupportsString(dbfactory.OSSCredsFileParam, "", "file", "OSS credentials file.")
ap.SupportsString(dbfactory.OSSCredsProfile, "", "profile", "OSS profile to use.")
ap.SupportsString(UserFlag, "u", "user", "User name to use when authenticating with the remote. Gets password from the environment variable {{.EmphasisLeft}}DOLT_REMOTE_PASSWORD{{.EmphasisRight}}.")
ap.SupportsFlag(SingleBranchFlag, "", "Clone only the history leading to the tip of a single branch, either specified by --branch or the primary branch remote's HEAD points at.")
stephkyou marked this conversation as resolved.
Show resolved Hide resolved
return ap
}

Expand Down
1 change: 1 addition & 0 deletions go/cmd/dolt/cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const (
ShallowFlag = "shallow"
ShowIgnoredFlag = "ignored"
SilentFlag = "silent"
SingleBranchFlag = "single-branch"
SkipEmptyFlag = "skip-empty"
SoftResetParam = "soft"
SquashParam = "squash"
Expand Down
3 changes: 2 additions & 1 deletion go/cmd/dolt/commands/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ func (cmd CloneCmd) Exec(ctx context.Context, commandStr string, args []string,
func clone(ctx context.Context, apr *argparser.ArgParseResults, dEnv *env.DoltEnv) errhand.VerboseError {
remoteName := apr.GetValueOrDefault(cli.RemoteParam, "origin")
branch := apr.GetValueOrDefault(cli.BranchParam, "")
singleBranch := apr.Contains(cli.SingleBranchFlag)
dir, urlStr, verr := parseArgs(apr)
if verr != nil {
return verr
Expand Down Expand Up @@ -143,7 +144,7 @@ func clone(ctx context.Context, apr *argparser.ArgParseResults, dEnv *env.DoltEn
// Nil out the old Dolt env so we don't accidentally operate on the wrong database
dEnv = nil

err = actions.CloneRemote(ctx, srcDB, remoteName, branch, clonedEnv)
err = actions.CloneRemote(ctx, srcDB, remoteName, branch, singleBranch, clonedEnv)
if err != nil {
// If we're cloning into a directory that already exists do not erase it. Otherwise
// make best effort to delete the directory we created.
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/dtestutils/testcommands/multienv.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func (mr *MultiRepoTestSetup) CloneDB(fromRemote, dbName string) {
mr.Errhand(err)
}

err = actions.CloneRemote(ctx, srcDB, r.Name, "", dEnv)
err = actions.CloneRemote(ctx, srcDB, r.Name, "", false, dEnv)
if err != nil {
mr.Errhand(err)
}
Expand Down
30 changes: 15 additions & 15 deletions go/libraries/doltcore/env/actions/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func sortedKeys(m map[string]iohelp.ReadStats) []string {
return keys
}

func CloneRemote(ctx context.Context, srcDB *doltdb.DoltDB, remoteName, branch string, dEnv *env.DoltEnv) error {
func CloneRemote(ctx context.Context, srcDB *doltdb.DoltDB, remoteName, branch string, singleBranch bool, dEnv *env.DoltEnv) error {
eventCh := make(chan pull.TableFileEvent, 128)

wg := &sync.WaitGroup{}
Expand Down Expand Up @@ -215,25 +215,25 @@ func CloneRemote(ctx context.Context, srcDB *doltdb.DoltDB, remoteName, branch s
// create remote refs corresponding to each of them. We delete all of
// the local branches except for the one corresponding to |branch|.
for _, brnch := range branches {
stephkyou marked this conversation as resolved.
Show resolved Hide resolved
cs, _ := doltdb.NewCommitSpec(brnch.GetPath())
cm, err := dEnv.DoltDB.Resolve(ctx, cs, nil)
if err != nil {
return fmt.Errorf("%w: %s; %s", ErrFailedToResolveBranchRef, brnch.String(), err.Error())

}

remoteRef := ref.NewRemoteRef(remoteName, brnch.GetPath())
err = dEnv.DoltDB.SetHeadToCommit(ctx, remoteRef, cm)
if err != nil {
return fmt.Errorf("%w: %s; %s", ErrFailedToCreateRemoteRef, remoteRef.String(), err.Error())

}

if brnch.GetPath() != branch {
err := dEnv.DoltDB.DeleteBranch(ctx, brnch, nil)
if err != nil {
return fmt.Errorf("%w: %s; %s", ErrFailedToDeleteBranch, brnch.String(), err.Error())
}
} else if !singleBranch || brnch.GetPath() == branch {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only way you can reach this conditional is if the branches are equal (because we checked for inequality in the first conditional), so this block will always be entered regardless of the singleBranch flag, because anything || TRUE is always true.

You can convince me this is right if you can point me to tests which verify the remote tracking branches work without the --single-branch (or write a test for that if we don't have one).

cs, _ := doltdb.NewCommitSpec(brnch.GetPath())
cm, err := dEnv.DoltDB.Resolve(ctx, cs, nil)
if err != nil {
return fmt.Errorf("%w: %s; %s", ErrFailedToResolveBranchRef, brnch.String(), err.Error())

}

remoteRef := ref.NewRemoteRef(remoteName, brnch.GetPath())
err = dEnv.DoltDB.SetHeadToCommit(ctx, remoteRef, cm)
if err != nil {
return fmt.Errorf("%w: %s; %s", ErrFailedToCreateRemoteRef, remoteRef.String(), err.Error())

}
}
}

Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/sqle/database_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ func (p *DoltDatabaseProvider) cloneDatabaseFromRemote(
return nil, err
}

err = actions.CloneRemote(ctx, srcDB, remoteName, branch, dEnv)
err = actions.CloneRemote(ctx, srcDB, remoteName, branch, false, dEnv)
if err != nil {
return nil, err
}
Expand Down
15 changes: 15 additions & 0 deletions integration-tests/bats/remotes.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,21 @@ create_five_remote_branches_main_and_master() {
[[ "$output" =~ "remotes/origin/branch-two" ]] || false
}

@test "remotes: clone --single-branch does not create remote refs for all remote branches" {
stephkyou marked this conversation as resolved.
Show resolved Hide resolved
create_three_remote_branches
cd dolt-repo-clones
dolt clone --single-branch http://localhost:50051/test-org/test-repo
cd test-repo
run dolt branch -a
[ "$status" -eq 0 ]
[[ "$output" =~ "* main" ]] || false
[[ ! "$output" =~ " branch-one" ]] || false
[[ ! "$output" =~ " branch-two" ]] || false
[[ "$output" =~ "remotes/origin/main" ]] || false
[[ ! "$output" =~ "remotes/origin/branch-one" ]] || false
[[ ! "$output" =~ "remotes/origin/branch-two" ]] || false
}

@test "remotes: fetch creates new remote refs for new remote branches" {
create_main_remote_branch

Expand Down