Add "squash merge" support to merge API (#8464)

* Add "squash merge" support to merge APIgen

Squashing really just means removing the source branch as a parent of the
resulting commit.

Fixes #7382.

* Test "squash merges" API

* Recommend in OpenAPI docs to add the source commit to other fields

* [CR] State that "squash" is more like Git{Hub,Lab} than like Git

* [CR] Add "--merge" option to "lakectl merge"

* [CR] Test lakectl with "--merge" option to "lakectl merge"

api/swagger.yml

@@ -686,7 +686,15 @@ components:
           type: boolean
           default: false
           description: Allow merge when the branches have the same content
-
+        squash_merge:
+          type: boolean
+          default: true
+          description: |
+            If set, set only the destination branch as a parent, which "squashes" the merge to
+            appear as a single commit on the destination branch.  The source commit is no longer
+            a part of the merge commit; consider adding it to the 'metadata' or 'message'
+            fields.  This behaves like a GitHub or GitLab "squash merge", or in Git terms 'git
+            merge --squash; git commit ...'.
     BranchCreation:
       type: object
       required:

cmd/lakectl/cmd/merge.go

@@ -42,6 +42,7 @@ var mergeCmd = &cobra.Command{
 		strategy := Must(cmd.Flags().GetString("strategy"))
 		force := Must(cmd.Flags().GetBool("force"))
 		allowEmpty := Must(cmd.Flags().GetBool("allow-empty"))
+		squash := Must(cmd.Flags().GetBool("squash"))
 
 		fmt.Println("Source:", sourceRef)
 		fmt.Println("Destination:", destinationRef)
@@ -54,11 +55,12 @@ var mergeCmd = &cobra.Command{
 		}
 
 		body := apigen.MergeIntoBranchJSONRequestBody{
-			Message:    &message,
-			Metadata:   &apigen.Merge_Metadata{AdditionalProperties: kvPairs},
-			Strategy:   &strategy,
-			Force:      &force,
-			AllowEmpty: &allowEmpty,
+			Message:     &message,
+			Metadata:    &apigen.Merge_Metadata{AdditionalProperties: kvPairs},
+			Strategy:    &strategy,
+			Force:       &force,
+			AllowEmpty:  &allowEmpty,
+			SquashMerge: &squash,
 		}
 
 		resp, err := client.MergeIntoBranchWithResponse(cmd.Context(), destinationRef.Repository, sourceRef.Ref, destinationRef.Ref, body)
@@ -89,6 +91,7 @@ func init() {
 	flags.String("strategy", "", "In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch (\"dest-wins\") or from the source branch(\"source-wins\"). In case no selection is made, the merge process will fail in case of a conflict")
 	flags.Bool("force", false, "Allow merge into a read-only branch or into a branch with the same content")
 	flags.Bool("allow-empty", false, "Allow merge when the branches have the same content")
+	flags.Bool("squash", false, "Squash all changes from source into a single commit on destination")
 	withCommitFlags(mergeCmd, true)
 	rootCmd.AddCommand(mergeCmd)
 }

docs/assets/js/swagger.yml

@@ -686,7 +686,15 @@ components:
           type: boolean
           default: false
           description: Allow merge when the branches have the same content
-
+        squash_merge:
+          type: boolean
+          default: true
+          description: |
+            If set, set only the destination branch as a parent, which "squashes" the merge to
+            appear as a single commit on the destination branch.  The source commit is no longer
+            a part of the merge commit; consider adding it to the 'metadata' or 'message'
+            fields.  This behaves like a GitHub or GitLab "squash merge", or in Git terms 'git
+            merge --squash; git commit ...'.
     BranchCreation:
       type: object
       required:

docs/reference/cli.md

@@ -2384,6 +2384,7 @@ lakectl merge <source ref> <destination ref> [flags]
   -h, --help                  help for merge
   -m, --message string        commit message
       --meta strings          key value pair in the form of key=value
+      --squash                Squash all changes from source into a single commit on destination
       --strategy string       In case of a merge conflict, this option will force the merge process to automatically favor changes from the dest branch ("dest-wins") or from the source branch("source-wins"). In case no selection is made, the merge process will fail in case of a conflict
 ```
 

esti/golden/lakectl_merge_with_squashed_commit.golden

@@ -0,0 +1,15 @@
+
+ID:            <COMMIT_ID>
+Author:        esti
+Date:          <DATE> <TIME> <TZ>
+
+	${MESSAGE}
+
+Metadata:
+
+	.lakefs.merge.strategy = default
+
+	key1               = value1
+
+	key2               = value2
+

esti/lakectl_test.go

@@ -224,31 +224,69 @@ func TestLakectlMerge(t *testing.T) {
 	commitMessage := "first commit to main"
 	vars["MESSAGE"] = commitMessage
 	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" commit lakefs://"+repoName+"/"+mainBranch+" -m \""+commitMessage+"\"", false, "lakectl_commit", vars)
+
+	// create new feature branch
 	featureBranch := "feature"
-	branchVars := map[string]string{
+	featureBranchVars := map[string]string{
 		"REPO":          repoName,
 		"STORAGE":       storage,
 		"SOURCE_BRANCH": mainBranch,
 		"DEST_BRANCH":   featureBranch,
 	}
 
-	t.Run("merge with commit message and meta", func(t *testing.T) {
-		// create new branch 'feature'
-		RunCmdAndVerifySuccessWithFile(t, Lakectl()+" branch create lakefs://"+repoName+"/"+featureBranch+" --source lakefs://"+repoName+"/"+mainBranch, false, "lakectl_branch_create", branchVars)
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" branch create lakefs://"+repoName+"/"+featureBranch+" --source lakefs://"+repoName+"/"+mainBranch, false, "lakectl_branch_create", featureBranchVars)
 
-		// update 'file1' on 'main' and commit
-		RunCmdAndVerifySuccessWithFile(t, Lakectl()+" fs upload -s files/ro_1k_other lakefs://"+repoName+"/"+mainBranch+"/"+filePath1, false, "lakectl_fs_upload", vars)
-		commitMessage = "file update on main branch"
-		vars["MESSAGE"] = commitMessage
-		RunCmdAndVerifySuccessWithFile(t, Lakectl()+" commit lakefs://"+repoName+"/"+mainBranch+" -m \""+commitMessage+"\"", false, "lakectl_commit", vars)
+	// update 'file1' on feature branch and commit
+	vars["FILE_PATH"] = filePath1
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" fs upload -s files/ro_1k_other lakefs://"+repoName+"/"+featureBranch+"/"+filePath1, false, "lakectl_fs_upload", vars)
+	commitMessage = "file update on feature branch"
+	vars["BRANCH"] = featureBranch
+	vars["MESSAGE"] = commitMessage
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" commit lakefs://"+repoName+"/"+featureBranch+" -m \""+commitMessage+"\"", false, "lakectl_commit", vars)
 
-		commitMessage = "merge commit"
-		vars["MESSAGE"] = commitMessage
-		meta := "key1=value1,key2=value2"
-		RunCmdAndVerifySuccessWithFile(t, Lakectl()+" merge lakefs://"+repoName+"/"+mainBranch+" lakefs://"+repoName+"/"+featureBranch+" -m '"+commitMessage+"' --meta "+meta, false, "lakectl_merge_success", branchVars)
+	// update 'file2' on 'main' and commit
+	vars["FILE_PATH"] = filePath2
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" fs upload -s files/ro_1k_other lakefs://"+repoName+"/"+featureBranch+"/"+filePath2, false, "lakectl_fs_upload", vars)
+	commitMessage = "another file update on main branch"
+	vars["MESSAGE"] = commitMessage
+	RunCmdAndVerifySuccessWithFile(t, Lakectl()+" commit lakefs://"+repoName+"/"+featureBranch+" -m \""+commitMessage+"\"", false, "lakectl_commit", vars)
 
-		RunCmdAndVerifySuccessWithFile(t, Lakectl()+" log --amount 1 lakefs://"+repoName+"/"+featureBranch, false, "lakectl_merge_with_commit", vars)
-	})
+	cases := []struct {
+		Name   string
+		Squash bool
+	}{
+		{Name: "regular", Squash: false},
+		{Name: "squash", Squash: true},
+	}
+	for _, tc := range cases {
+		t.Run("merge with commit message and meta "+tc.Name, func(t *testing.T) {
+			destBranch := "dest-" + tc.Name
+			destBranchVars := map[string]string{
+				"REPO":          repoName,
+				"STORAGE":       storage,
+				"SOURCE_BRANCH": mainBranch,
+				"DEST_BRANCH":   destBranch,
+			}
+			// create new destBranch from main, before the additions to main.
+			RunCmdAndVerifySuccessWithFile(t, Lakectl()+" branch create lakefs://"+repoName+"/"+destBranch+" --source lakefs://"+repoName+"/"+mainBranch, false, "lakectl_branch_create", destBranchVars)
+
+			commitMessage = "merge commit"
+			vars["MESSAGE"] = commitMessage
+			meta := "key1=value1,key2=value2"
+			squash := ""
+			if tc.Squash {
+				squash = "--squash"
+			}
+			destBranchVars["SOURCE_BRANCH"] = featureBranch
+			RunCmdAndVerifySuccessWithFile(t, Lakectl()+" merge lakefs://"+repoName+"/"+featureBranch+" lakefs://"+repoName+"/"+destBranch+" -m '"+commitMessage+"' --meta "+meta+" "+squash, false, "lakectl_merge_success", destBranchVars)
+
+			golden := "lakectl_merge_with_commit"
+			if tc.Squash {
+				golden = "lakectl_merge_with_squashed_commit"
+			}
+			RunCmdAndVerifySuccessWithFile(t, Lakectl()+" log --amount 1 lakefs://"+repoName+"/"+destBranch, false, golden, vars)
+		})
+	}
 }
 
 func TestLakectlMergeAndStrategies(t *testing.T) {

pkg/api/controller.go

@@ -4809,6 +4809,7 @@ func (c *Controller) MergeIntoBranch(w http.ResponseWriter, r *http.Request, bod
 		swag.StringValue(body.Strategy),
 		graveler.WithForce(swag.BoolValue(body.Force)),
 		graveler.WithAllowEmpty(swag.BoolValue(body.AllowEmpty)),
+		graveler.WithSquashMerge(swag.BoolValue(body.SquashMerge)),
 	)
 
 	if errors.Is(err, graveler.ErrConflictFound) {