diff --git a/app/api/controller/repo/merge_check.go b/app/api/controller/repo/merge_check.go index 2cd18d496..9e1723971 100644 --- a/app/api/controller/repo/merge_check.go +++ b/app/api/controller/repo/merge_check.go @@ -21,6 +21,7 @@ import ( "github.com/harness/gitness/app/api/controller" "github.com/harness/gitness/app/auth" "github.com/harness/gitness/git" + "github.com/harness/gitness/git/api" "github.com/harness/gitness/types/enum" ) @@ -57,6 +58,14 @@ func (c *Controller) MergeCheck( HeadBranch: info.HeadRef, }) if err != nil { + // git.Merge works with commits and error is not user-friendly + // and here we are modify base-ref and head-ref with user input + // values. + if uErr := api.AsUnrelatedHistoriesError(err); uErr != nil { + uErr.BaseRef = info.BaseRef + uErr.HeadRef = info.HeadRef + return MergeCheck{}, uErr + } return MergeCheck{}, fmt.Errorf("merge check execution failed: %w", err) } if len(mergeOutput.ConflictFiles) > 0 { diff --git a/app/api/handler/repo/diff.go b/app/api/handler/repo/diff.go index bb35a3f3e..c6e86177f 100644 --- a/app/api/handler/repo/diff.go +++ b/app/api/handler/repo/diff.go @@ -23,6 +23,7 @@ import ( "github.com/harness/gitness/app/api/controller/repo" "github.com/harness/gitness/app/api/render" "github.com/harness/gitness/app/api/request" + "github.com/harness/gitness/app/api/usererror" "github.com/harness/gitness/errors" gittypes "github.com/harness/gitness/git/api" ) @@ -110,6 +111,13 @@ func HandleDiffStats(repoCtrl *repo.Controller) http.HandlerFunc { path := request.GetOptionalRemainderFromPath(r) output, err := repoCtrl.DiffStats(ctx, session, repoRef, path) + if uErr := gittypes.AsUnrelatedHistoriesError(err); uErr != nil { + render.JSON(w, http.StatusOK, &usererror.Error{ + Message: uErr.Error(), + Values: uErr.Map(), + }) + return + } if err != nil { render.TranslatedUserError(ctx, w, err) return diff --git a/app/api/render/render.go b/app/api/render/render.go index 1050b19b2..b32a105b6 100644 --- a/app/api/render/render.go +++ b/app/api/render/render.go @@ -22,7 +22,9 @@ import ( "os" "strconv" + "github.com/harness/gitness/app/api/usererror" "github.com/harness/gitness/errors" + "github.com/harness/gitness/git/api" "github.com/harness/gitness/types" "github.com/rs/zerolog/log" @@ -75,6 +77,15 @@ func JSONArrayDynamic[T comparable](ctx context.Context, w http.ResponseWriter, } if err != nil { + // based on discussion unrelated histories error should return + // StatusOK on diff. + if uErr := api.AsUnrelatedHistoriesError(err); uErr != nil { + JSON(w, http.StatusOK, &usererror.Error{ + Message: uErr.Error(), + Values: uErr.Map(), + }) + return + } // User canceled the request - no need to do anything if errors.Is(err, context.Canceled) { return diff --git a/app/api/usererror/translate.go b/app/api/usererror/translate.go index ee5b841a1..4d4ab2fc3 100644 --- a/app/api/usererror/translate.go +++ b/app/api/usererror/translate.go @@ -25,6 +25,7 @@ import ( "github.com/harness/gitness/app/services/webhook" "github.com/harness/gitness/blob" "github.com/harness/gitness/errors" + "github.com/harness/gitness/git/api" "github.com/harness/gitness/lock" "github.com/harness/gitness/store" "github.com/harness/gitness/types/check" @@ -37,6 +38,7 @@ func Translate(ctx context.Context, err error) *Error { rError *Error checkError *check.ValidationError appError *errors.Error + unrelatedHistoriesErr *api.UnrelatedHistoriesError maxBytesErr *http.MaxBytesError codeOwnersTooLargeError *codeowners.TooLargeError codeOwnersFileParseError *codeowners.FileParseError @@ -96,6 +98,12 @@ func Translate(ctx context.Context, err error) *Error { appError.Message, appError.Details, ) + case errors.As(err, &unrelatedHistoriesErr): + return NewWithPayload( + http.StatusBadRequest, + err.Error(), + unrelatedHistoriesErr.Map(), + ) // webhook errors case errors.Is(err, webhook.ErrWebhookNotRetriggerable): diff --git a/git/api/errors.go b/git/api/errors.go index 2c1527091..f9785757a 100644 --- a/git/api/errors.go +++ b/git/api/errors.go @@ -19,7 +19,6 @@ import ( "strings" "github.com/harness/gitness/errors" - "github.com/harness/gitness/git/enum" "github.com/rs/zerolog/log" ) @@ -133,33 +132,64 @@ func processGitErrorf(err error, format string, args ...interface{}) error { return errors.NotFound("repository not found") case strings.Contains(err.Error(), "reference already exists"): return errors.Conflict("reference already exists") + case strings.Contains(err.Error(), "no merge base"): + if len(args) >= 2 { + return &UnrelatedHistoriesError{ + BaseRef: strings.TrimSpace(args[0].(string)), + HeadRef: strings.TrimSpace(args[1].(string)), + } + } + return &UnrelatedHistoriesError{} default: return fallbackErr } } -// MergeUnrelatedHistoriesError represents an error if merging fails due to unrelated histories. -type MergeUnrelatedHistoriesError struct { - Method enum.MergeMethod - StdOut string - StdErr string - Err error +type UnrelatedHistoriesError struct { + BaseRef string + HeadRef string } -func IsMergeUnrelatedHistoriesError(err error) bool { - return errors.Is(err, &MergeUnrelatedHistoriesError{}) +func (e *UnrelatedHistoriesError) Map() map[string]any { + return map[string]any{ + "base_ref": e.BaseRef, + "head_ref": e.HeadRef, + } } -func (e *MergeUnrelatedHistoriesError) Error() string { - return fmt.Sprintf("Merge UnrelatedHistories Error: %v: %s\n%s", e.Err, e.StdErr, e.StdOut) +func (e *UnrelatedHistoriesError) Is(err error) bool { + var target *UnrelatedHistoriesError + ok := errors.As(err, &target) + if !ok { + return false + } + + return target.BaseRef == e.BaseRef && target.HeadRef == e.HeadRef } -func (e *MergeUnrelatedHistoriesError) Unwrap() error { - return e.Err +func (e *UnrelatedHistoriesError) Error() string { + if e.BaseRef == "" || e.HeadRef == "" { + return "unrelated commit histories error" + } + // remove branch and tag prefixes, original remains in struct fields + // we just need to remove first occurrence. + baseRef := strings.TrimPrefix(e.BaseRef, BranchPrefix) + baseRef = strings.TrimPrefix(baseRef, TagPrefix) + headRef := strings.TrimPrefix(e.HeadRef, BranchPrefix) + headRef = strings.TrimPrefix(headRef, TagPrefix) + return fmt.Sprintf("%s and %s have entirely different commit histories.", baseRef, headRef) } -//nolint:errorlint // the purpose of this method is to check whether the target itself if of this type. -func (e *MergeUnrelatedHistoriesError) Is(target error) bool { - _, ok := target.(*MergeUnrelatedHistoriesError) - return ok +// IsUnrelatedHistoriesError checks if an error is a UnrelatedHistoriesError. +func IsUnrelatedHistoriesError(err error) bool { + return AsUnrelatedHistoriesError(err) != nil +} + +func AsUnrelatedHistoriesError(err error) *UnrelatedHistoriesError { + var target *UnrelatedHistoriesError + ok := errors.As(err, &target) + if !ok { + return nil + } + return target } diff --git a/git/api/merge.go b/git/api/merge.go index 2c791d386..66e694874 100644 --- a/git/api/merge.go +++ b/git/api/merge.go @@ -56,15 +56,26 @@ func (g *Git) GetMergeBase( } } - stdout := &bytes.Buffer{} cmd := command.New("merge-base", command.WithArg(base, head), ) + + stdout, stderr := new(bytes.Buffer), new(bytes.Buffer) err := cmd.Run(ctx, command.WithDir(repoPath), command.WithStdout(stdout), + command.WithStderr(stderr), ) if err != nil { + // git merge-base rev1 rev2 + // if there is unrelated history then stderr is empty with + // exit code 1. This cannot be handled in processGitErrorf because stderr is empty. + if command.AsError(err).IsExitCode(1) && stderr.Len() == 0 { + return sha.None, "", &UnrelatedHistoriesError{ + BaseRef: base, + HeadRef: head, + } + } return sha.None, "", processGitErrorf(err, "failed to get merge-base [%s, %s]", base, head) } diff --git a/git/merge.go b/git/merge.go index 9053983e8..63cc21896 100644 --- a/git/merge.go +++ b/git/merge.go @@ -184,12 +184,12 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput, baseCommitSHA, err := s.git.GetFullCommitID(ctx, repoPath, params.BaseBranch) if err != nil { - return MergeOutput{}, fmt.Errorf("failed to get merge base branch commit SHA: %w", err) + return MergeOutput{}, fmt.Errorf("failed to get base branch commit SHA: %w", err) } headCommitSHA, err := s.git.GetFullCommitID(ctx, repoPath, params.HeadBranch) if err != nil { - return MergeOutput{}, fmt.Errorf("failed to get merge base branch commit SHA: %w", err) + return MergeOutput{}, fmt.Errorf("failed to get head branch commit SHA: %w", err) } if !params.HeadExpectedSHA.IsEmpty() && !params.HeadExpectedSHA.Equal(headCommitSHA) {