Step 618 fetch refspec (#138)

* Remove *string usage.

* Use branch ref for commit checkout fetch if branch available.

* Remove unrelated branch test.

* Add new e2e test case to checkout commit on a branch.

bitrise.yml

@@ -43,7 +43,7 @@ workflows:
     - _test_submodule
     - _test_no_checkout
     - _test_checkout_commit
-    - _test_checkout_commit_with_other_branch
+    - _test_checkout_commit_on_branch
     - _test_checkout_tag
     - _test_checkout_tag_with_other_branch
     - _test_checkout_tag_with_equally_named_branch
@@ -183,30 +183,30 @@ workflows:
         inputs:
         - dir_to_check: $CLONE_INTO_DIR
 
-  _test_checkout_commit_with_other_branch:
+  _test_checkout_commit_on_branch:
     before_run:
-      - _create_tmpdir
+    - _create_tmpdir
     steps:
-      - path::./:
-          run_if: true
-          inputs:
-            - clone_into_dir: $CLONE_INTO_DIR
-            - commit: e9667364b397ec5d97b267240928432f74118d72
-            - tag: ""
-            - branch: "master"
-            - pull_request_id: ""
-            - pull_request_merge_branch: ""
-            - pull_request_repository_url: ""
-            - branch_dest: ""
-            - clone_depth: ""
-      - script:
-          inputs:
-            - content: |-
-                #!/bin/bash
-                eval "$EVAL_SCRIPT"
-      - ensure-clean-git:
-          inputs:
-            - dir_to_check: $CLONE_INTO_DIR
+    - path::./:
+        run_if: true
+        inputs:
+        - clone_into_dir: $CLONE_INTO_DIR
+        - commit: 7a93251a9bf08fb8c6b19fc02f6d94618de6b386
+        - tag: ""
+        - branch: "master"
+        - pull_request_id: ""
+        - pull_request_merge_branch: ""
+        - pull_request_repository_url: ""
+        - branch_dest: ""
+        - clone_depth: ""
+    - script:
+        inputs:
+        - content: |-
+            #!/bin/bash
+            eval "$EVAL_SCRIPT"
+    - ensure-clean-git:
+        inputs:
+        - dir_to_check: $CLONE_INTO_DIR
 
   _test_checkout_tag:
     before_run:

gitclone/checkout.go

@@ -107,7 +107,7 @@ func createCheckoutStrategy(checkoutMethod CheckoutMethod, cfg Config, patch pat
 		}
 	case CheckoutCommitMethod:
 		{
-			params, err := NewCommitParams(cfg.Commit)
+			params, err := NewCommitParams(cfg.Commit, cfg.Branch)
 			if err != nil {
 				return nil, err
 			}
@@ -118,11 +118,7 @@ func createCheckoutStrategy(checkoutMethod CheckoutMethod, cfg Config, patch pat
 		}
 	case CheckoutTagMethod:
 		{
-			var branch *string
-			if cfg.Branch != "" {
-				branch = &cfg.Branch
-			}
-			params, err := NewTagParams(cfg.Tag, branch)
+			params, err := NewTagParams(cfg.Tag, cfg.Branch)
 			if err != nil {
 				return nil, err
 			}

gitclone/checkout_helper.go

@@ -27,22 +27,22 @@ func (t fetchOptions) IsFullDepth() bool {
 
 const branchRefPrefix = "refs/heads/"
 
-func fetch(gitCmd git.Git, remote string, ref *string, traits fetchOptions) error {
+func fetch(gitCmd git.Git, remote string, ref string, traits fetchOptions) error {
 	var opts []string
 	if traits.depth != 0 {
 		opts = append(opts, "--depth="+strconv.Itoa(traits.depth))
 	}
 	if traits.allTags {
 		opts = append(opts, "--tags")
 	}
-	if ref != nil {
-		opts = append(opts, remote, *ref)
+	if ref != "" {
+		opts = append(opts, remote, ref)
 	}
 
 	// Not neccessarily a branch, can be tag too
 	branch := ""
-	if ref != nil && strings.HasPrefix(*ref, branchRefPrefix) {
-		branch = strings.TrimPrefix(*ref, branchRefPrefix)
+	if strings.HasPrefix(ref, branchRefPrefix) {
+		branch = strings.TrimPrefix(ref, branchRefPrefix)
 	}
 
 	if err := runner.RunWithRetry(func() *command.Model {
@@ -79,7 +79,7 @@ func checkoutWithCustomRetry(gitCmd git.Git, arg string, retry fallbackRetry) er
 
 func fetchInitialBranch(gitCmd git.Git, remote string, branchRef string, fetchTraits fetchOptions) error {
 	branch := strings.TrimPrefix(branchRef, branchRefPrefix)
-	if err := fetch(gitCmd, remote, &branchRef, fetchTraits); err != nil {
+	if err := fetch(gitCmd, remote, branchRef, fetchTraits); err != nil {
 		return err
 	}
 

gitclone/checkout_method_pr_auto_diff.go

@@ -37,7 +37,7 @@ type checkoutPRDiffFile struct {
 
 func (c checkoutPRDiffFile) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fallbackRetry) error {
 	baseBranchRef := branchRefPrefix + c.params.BaseBranch
-	if err := fetch(gitCmd, defaultRemoteName, &baseBranchRef, fetchOptions); err != nil {
+	if err := fetch(gitCmd, defaultRemoteName, baseBranchRef, fetchOptions); err != nil {
 		return err
 	}
 

gitclone/checkout_method_pr_auto_merge_branch.go

@@ -38,13 +38,13 @@ func (c checkoutPRMergeBranch) do(gitCmd git.Git, fetchOpts fetchOptions, fallba
 	// Check out initial branch (fetchInitialBranch part1)
 	// `git "fetch" "origin" "refs/heads/master"`
 	baseBranchRef := branchRefPrefix + c.params.BaseBranch
-	if err := fetch(gitCmd, defaultRemoteName, &baseBranchRef, fetchOpts); err != nil {
+	if err := fetch(gitCmd, defaultRemoteName, baseBranchRef, fetchOpts); err != nil {
 		return err
 	}
 
 	// `git "fetch" "origin" "refs/pull/7/head:pull/7"`
 	headBranchRef := fetchArg(c.params.MergeBranch)
-	if err := fetch(gitCmd, defaultRemoteName, &headBranchRef, fetchOpts); err != nil {
+	if err := fetch(gitCmd, defaultRemoteName, headBranchRef, fetchOpts); err != nil {
 		return err
 	}
 

gitclone/checkout_method_pr_manual.go

@@ -56,7 +56,7 @@ func (c checkoutPRManualMerge) do(gitCmd git.Git, fetchOptions fetchOptions, fal
 
 	// Fetch and merge
 	headBranchRef := branchRefPrefix + c.params.HeadBranch
-	if err := fetch(gitCmd, defaultRemoteName, &headBranchRef, fetchOptions); err != nil {
+	if err := fetch(gitCmd, defaultRemoteName, headBranchRef, fetchOptions); err != nil {
 		return nil
 	}
 
@@ -121,7 +121,7 @@ func (c checkoutForkPRManualMerge) do(gitCmd git.Git, fetchOptions fetchOptions,
 
 	// Fetch + merge fork branch
 	forkBranchRef := branchRefPrefix + c.params.HeadBranch
-	if err := fetch(gitCmd, forkRemoteName, &forkBranchRef, fetchOptions); err != nil {
+	if err := fetch(gitCmd, forkRemoteName, forkBranchRef, fetchOptions); err != nil {
 		return err
 	}
 

gitclone/checkout_method_simple.go

@@ -18,16 +18,18 @@ func (c checkoutNone) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fal
 // CommitParams are parameters to check out a given commit (In addition to the repository URL)
 type CommitParams struct {
 	Commit string
+	Branch string
 }
 
 // NewCommitParams validates and returns a new CommitParams
-func NewCommitParams(commit string) (*CommitParams, error) {
+func NewCommitParams(commit, branch string) (*CommitParams, error) {
 	if strings.TrimSpace(commit) == "" {
 		return nil, NewParameterValidationError("commit checkout strategy can not be used: no commit hash specified")
 	}
 
 	return &CommitParams{
 		Commit: commit,
+		Branch: branch,
 	}, nil
 }
 
@@ -37,9 +39,12 @@ type checkoutCommit struct {
 }
 
 func (c checkoutCommit) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fallbackRetry) error {
-	// Fetch then checkout
-	// No branch specified for fetch
-	if err := fetch(gitCmd, defaultRemoteName, nil, fetchOptions); err != nil {
+	branchRefParam := ""
+	if c.params.Branch != "" {
+		branchRefParam = branchRefPrefix + c.params.Branch
+	}
+
+	if err := fetch(gitCmd, defaultRemoteName, branchRefParam, fetchOptions); err != nil {
 		return err
 	}
 
@@ -85,17 +90,14 @@ func (c checkoutBranch) do(gitCmd git.Git, fetchOptions fetchOptions, _ fallback
 // TagParams are parameters to check out a given tag
 type TagParams struct {
 	Tag    string
-	Branch *string
+	Branch string
 }
 
 // NewTagParams validates and returns a new TagParams
-func NewTagParams(tag string, branch *string) (*TagParams, error) {
+func NewTagParams(tag, branch string) (*TagParams, error) {
 	if strings.TrimSpace(tag) == "" {
 		return nil, NewParameterValidationError("tag checkout strategy can not be used: no tag specified")
 	}
-	if branch != nil && strings.TrimSpace(*branch) == "" {
-		return nil, NewParameterValidationError("tag checkout strategy can not be used: branch non nil but empty")
-	}
 
 	return &TagParams{
 		Tag:    tag,
@@ -109,10 +111,9 @@ type checkoutTag struct {
 }
 
 func (c checkoutTag) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fallbackRetry) error {
-	var branchRefParam *string
-	if c.params.Branch != nil {
-		branchRef := branchRefPrefix + *c.params.Branch
-		branchRefParam = &branchRef
+	branchRefParam := ""
+	if c.params.Branch != "" {
+		branchRefParam = branchRefPrefix + c.params.Branch
 	}
 
 	if err := fetch(gitCmd, defaultRemoteName, branchRefParam, fetchOptions); err != nil {

gitclone/gitclone_test.go

@@ -44,7 +44,7 @@ var testCases = [...]struct {
 			Branch: "hcnarb",
 		},
 		wantCmds: []string{
-			`git "fetch"`,
+			`git "fetch" "origin" "refs/heads/hcnarb"`,
 			`git "checkout" "76a934ae"`,
 		},
 	},
@@ -113,7 +113,7 @@ var testCases = [...]struct {
 			Branch: "hcnarb",
 		},
 		wantCmds: []string{
-			`git "fetch" "--tags"`,
+			`git "fetch" "--tags" "origin" "refs/heads/hcnarb"`,
 			`git "checkout" "76a934ae"`,
 		},
 	},
@@ -160,7 +160,7 @@ var testCases = [...]struct {
 			ManualMerge: true,
 		},
 		wantCmds: []string{
-			`git "fetch"`,
+			`git "fetch" "origin" "refs/heads/test/commit-messages"`,
 			`git "checkout" "76a934ae"`,
 		},
 	},