Skip to content

Commit

Permalink
fix: use correct base in diff
Browse files Browse the repository at this point in the history
  • Loading branch information
didroe committed Aug 8, 2023
1 parent ad03bd4 commit f9a6551
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 58 deletions.
6 changes: 1 addition & 5 deletions pkg/commands/artifact/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,7 @@ func (r *runner) Scan(ctx context.Context, opts flag.Options) (*basebranchfindin

repository, err := gitrepository.New(ctx, r.scanSettings, targetPath, opts.DiffBaseBranch)
if err != nil {
return nil, fmt.Errorf("error opening git repository: %w", err)
}

if err := repository.FetchBaseIfNotPresent(); err != nil {
return nil, fmt.Errorf("error fetching base branch: %w", err)
return nil, fmt.Errorf("git repository error: %w", err)
}

fileList, err := filelist.Discover(repository, targetPath, r.goclocResult, r.scanSettings)
Expand Down
220 changes: 167 additions & 53 deletions pkg/commands/process/gitrepository/gitrepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"net/url"
"os"
"path/filepath"
"strings"
Expand All @@ -13,6 +14,7 @@ import (
"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/format/diff"
"github.com/go-git/go-git/v5/plumbing/object"
"github.com/google/go-github/github"
"github.com/hhatto/gocloc"
"github.com/rs/zerolog/log"

Expand All @@ -31,9 +33,10 @@ type Repository struct {
rootPath,
targetPath,
gitTargetPath string
baseBranch string
baseRemoteRefName plumbing.ReferenceName
headRef *plumbing.Reference
headCommit,
mergeBaseCommit *object.Commit
}

func New(
Expand All @@ -42,42 +45,54 @@ func New(
targetPath string,
baseBranch string,
) (*Repository, error) {
repository, err := git.PlainOpenWithOptions(targetPath, &git.PlainOpenOptions{
gitRepository, err := git.PlainOpenWithOptions(targetPath, &git.PlainOpenOptions{
DetectDotGit: true,
})
if err != nil {
return nil, translateOpenError(baseBranch, err)
}

worktree, err := repository.Worktree()
worktree, err := gitRepository.Worktree()
if err != nil {
return nil, fmt.Errorf("failed to get worktree: %w", err)
}

rootPath := worktree.Filesystem.Root()
gitTargetPath, err := filepath.Rel(rootPath, targetPath)
if err != nil {
return nil, fmt.Errorf("failed to get git relative target: %w", err)
return nil, fmt.Errorf("failed to get relative target: %w", err)
}

log.Debug().Msgf("git target: [%s/]%s", rootPath, gitTargetPath)

headRef, err := repository.Head()
headRef, err := gitRepository.Head()
if err != nil {
return nil, fmt.Errorf("error getting git head: %w", err)
return nil, fmt.Errorf("error getting head ref: %w", err)
}

return &Repository{
headCommit, err := gitRepository.CommitObject(headRef.Hash())
if err != nil {
return nil, fmt.Errorf("failed to get head commit: %w", err)
}

repository := &Repository{
ctx: ctx,
config: config,
git: repository,
git: gitRepository,
rootPath: rootPath,
targetPath: targetPath,
gitTargetPath: gitTargetPath,
baseBranch: baseBranch,
baseRemoteRefName: plumbing.NewRemoteReferenceName("origin", baseBranch),
headRef: headRef,
}, nil
headCommit: headCommit,
}

repository.mergeBaseCommit, err = repository.fetchMergeBaseCommit(baseBranch)
if err != nil {
return nil, err
}

return repository, nil
}

func (repository *Repository) ListFiles(
Expand All @@ -88,12 +103,12 @@ func (repository *Repository) ListFiles(
return nil, nil
}

headTree, err := repository.treeForRef(repository.headRef)
headTree, err := repository.headCommit.Tree()
if err != nil {
return nil, fmt.Errorf("failed to get head tree: %w", err)
}

if repository.baseBranch == "" {
if repository.mergeBaseCommit == nil {
return repository.getTreeFiles(ignore, goclocResult, headTree)
}

Expand All @@ -105,13 +120,49 @@ func (repository *Repository) ListFiles(
return repository.getDiffFiles(ignore, goclocResult, filePatches)
}

func (repository *Repository) treeForRef(ref *plumbing.Reference) (*object.Tree, error) {
commit, err := repository.git.CommitObject(ref.Hash())
func (repository *Repository) fetchMergeBaseCommit(baseBranch string) (*object.Commit, error) {
if baseBranch == "" {
return nil, nil
}

hash, err := repository.lookupMergeBaseHash(baseBranch)
if err != nil {
return nil, fmt.Errorf("failed to get commit for ref: %w", err)
return nil, fmt.Errorf("error looking up hash: %w", err)
}

if hash == nil {
return nil, fmt.Errorf(
"could not find common ancestor between the current and %s branch. "+
"please check that the base branch is correct, and that you have "+
"fetched enough git history to include the latest common ancestor",
baseBranch,
)
}

commit, err := repository.git.CommitObject(*hash)
if err == nil {
return commit, nil
}

if err != plumbing.ErrObjectNotFound {
return nil, fmt.Errorf("error looking up commit: %w", err)
}

log.Debug().Msgf("merge base commit not present, fetching")

if err := repository.git.FetchContext(repository.ctx, &git.FetchOptions{
RefSpecs: []config.RefSpec{config.RefSpec(
fmt.Sprintf("+%s:%s", hash.String(), repository.baseRemoteRefName),
)},
Depth: 1,
Tags: git.NoTags,
}); err != nil {
return nil, fmt.Errorf("error fetching: %w", err)
}

return commit.Tree()
log.Debug().Msgf("merge base commit fetched")

return commit, nil
}

func (repository *Repository) getTreeFiles(
Expand Down Expand Up @@ -139,12 +190,7 @@ func (repository *Repository) getTreeFiles(
}

func (repository *Repository) getDiffPatch(headTree *object.Tree) ([]diff.FilePatch, error) {
baseRef, err := repository.git.Reference(repository.baseRemoteRefName, true)
if err != nil {
return nil, fmt.Errorf("failed to get base ref %s: %w", repository.baseRemoteRefName, err)
}

baseTree, err := repository.treeForRef(baseRef)
baseTree, err := repository.mergeBaseCommit.Tree()
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -240,37 +286,8 @@ func (repository *Repository) getDiffFiles(
}, nil
}

func (repository *Repository) FetchBaseIfNotPresent() error {
if repository == nil || repository.baseBranch == "" {
return nil
}

ref, err := repository.git.Reference(repository.baseRemoteRefName, true)
if err != nil && err != plumbing.ErrReferenceNotFound {
return fmt.Errorf("invalid branch %s: %w", repository.baseBranch, err)
}

// Already exists
if ref != nil {
return nil
}

localRefName := plumbing.NewBranchReferenceName(repository.baseBranch)
if err := repository.git.FetchContext(repository.ctx, &git.FetchOptions{
RefSpecs: []config.RefSpec{config.RefSpec(
fmt.Sprintf("+%s:%s", localRefName, repository.baseRemoteRefName),
)},
Depth: 1,
Tags: git.NoTags,
}); err != nil {
return fmt.Errorf("error fetching branch %s: %w", repository.baseBranch, err)
}

return nil
}

func (repository *Repository) WithBaseBranch(body func() error) error {
if repository == nil || repository.baseBranch == "" {
if repository == nil || repository.mergeBaseCommit == nil {
return nil
}

Expand All @@ -282,7 +299,7 @@ func (repository *Repository) WithBaseBranch(body func() error) error {
defer repository.restoreHead(worktree)

if err := worktree.Checkout(&git.CheckoutOptions{
Branch: repository.baseRemoteRefName,
Hash: repository.mergeBaseCommit.Hash,
}); err != nil {
return fmt.Errorf("error checking out base branch: %w", err)
}
Expand Down Expand Up @@ -352,3 +369,100 @@ func translateOpenError(baseBranch string, err error) error {

return nil
}

func (repository *Repository) lookupMergeBaseHash(baseBranch string) (*plumbing.Hash, error) {
if hash := repository.lookupMergeBaseRefFromVariable(); hash != nil {
return hash, nil
}

if hash, err := repository.lookupMergeBaseRefFromGithub(baseBranch); hash != nil || err != nil {
return hash, err
}

log.Debug().Msg("finding merge base using local repository")

ref, err := repository.git.Reference(repository.baseRemoteRefName, true)
if err != nil {
return nil, fmt.Errorf("invalid ref: %w", err)
}

baseCommit, err := repository.git.CommitObject(ref.Hash())
if err != nil {
if err == plumbing.ErrObjectNotFound {
return nil, nil
}

return nil, fmt.Errorf("error looking up base commit: %w", err)
}

commonAncestors, err := repository.headCommit.MergeBase(baseCommit)
if err != nil {
if err == plumbing.ErrObjectNotFound {
return nil, nil
}

return nil, fmt.Errorf("error computing merge base: %w", err)
}
if len(commonAncestors) == 0 {
return nil, nil
}

return &commonAncestors[0].Hash, nil
}

func (repository *Repository) lookupMergeBaseRefFromVariable() *plumbing.Hash {
sha := os.Getenv("DIFF_BASE_COMMIT")
if sha == "" {
return nil
}

hash := plumbing.NewHash(sha)
return &hash
}

func (repository *Repository) lookupMergeBaseRefFromGithub(baseBranch string) (*plumbing.Hash, error) {
githubToken := os.Getenv("GITHUB_TOKEN")
if githubToken == "" {
return nil, nil
}

githubRepository := os.Getenv("GITHUB_REPOSITORY")
if githubRepository == "" {
return nil, nil
}

log.Debug().Msg("finding merge base using github api")

splitRepository := strings.SplitN(githubRepository, "/", 2)
if len(splitRepository) != 2 {
return nil, fmt.Errorf("invalid github repository name '%s'", githubRepository)
}

client := github.NewClient(nil)
if githubAPIURL := os.Getenv("GITHUB_API_URL"); githubAPIURL != "" {
parsedURL, err := url.Parse(githubAPIURL)
if err != nil {
return nil, fmt.Errorf("failed to parse github api url: %w", err)
}

client.BaseURL = parsedURL
}

comparison, _, err := client.Repositories.CompareCommits(
context.Background(),
splitRepository[0],
splitRepository[1],
baseBranch,
repository.headRef.Hash().String(),
)
if err != nil {
return nil, fmt.Errorf("error calling github compare api: %w", err)
}

if comparison.MergeBaseCommit == nil {
return nil, nil
}

hash := plumbing.NewHash(*comparison.MergeBaseCommit.SHA)
return &hash, nil
}

0 comments on commit f9a6551

Please sign in to comment.