From fba252b3ef35498d947e7a4bc1c6b11e2db3894c Mon Sep 17 00:00:00 2001 From: zeripath Date: Tue, 29 Mar 2022 18:12:33 +0100 Subject: [PATCH 1/2] Use full output of git show-ref --tags to get tags for PushUpdateAddTag (#19235) Strangely #19038 appears to relate to an issue whereby a tag appears to be listed in `git show-ref --tags` but then does not appear when `git show-ref --tags -- short_name` is called. As a solution though I propose to stop the second call as it is unnecessary and only likely to cause problems. I've also noticed that the tags calls are wildly inefficient and aren't using the common cat-files - so these have been added. I've also noticed that the git commit-graph is not being written on mirroring - so I've also added writing this to the migration which should improve mirror rendering somewhat. Fix #19038 Signed-off-by: Andrew Thornton Co-authored-by: 6543 <6543@obermui.de> --- modules/git/repo_branch_gogit.go | 44 +++++++++++- modules/git/repo_branch_nogogit.go | 35 +++++++--- modules/git/repo_commitgraph.go | 21 ++++++ modules/git/repo_tag.go | 91 ++++-------------------- modules/git/repo_tag_gogit.go | 82 ++++++++++++++++++++++ modules/git/repo_tag_nogogit.go | 108 +++++++++++++++++++++++++++++ modules/repository/repo.go | 35 ++++++---- services/mirror/mirror_pull.go | 11 ++- services/repository/branch.go | 2 +- 9 files changed, 325 insertions(+), 104 deletions(-) create mode 100644 modules/git/repo_commitgraph.go diff --git a/modules/git/repo_branch_gogit.go b/modules/git/repo_branch_gogit.go index d159aafd6f3b..e9e27a357053 100644 --- a/modules/git/repo_branch_gogit.go +++ b/modules/git/repo_branch_gogit.go @@ -13,6 +13,7 @@ import ( "strings" "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/storer" ) // IsObjectExist returns true if given reference exists in the repository. @@ -82,8 +83,9 @@ func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) { } // WalkReferences walks all the references from the repository -func WalkReferences(ctx context.Context, repoPath string, walkfn func(string) error) (int, error) { - repo, err := OpenRepositoryCtx(ctx, repoPath) +// refType should be empty, ObjectTag or ObjectBranch. All other values are equivalent to empty. +func WalkReferences(ctx context.Context, repoPath string, walkfn func(sha1, refname string) error) (int, error) { + repo, err = OpenRepositoryCtx(ctx, repoPath) if err != nil { return 0, err } @@ -97,9 +99,45 @@ func WalkReferences(ctx context.Context, repoPath string, walkfn func(string) er defer iter.Close() err = iter.ForEach(func(ref *plumbing.Reference) error { - err := walkfn(string(ref.Name())) + err := walkfn(ref.Hash().String(), string(ref.Name())) i++ return err }) return i, err } + +// WalkReferences walks all the references from the repository +func (repo *Repository) WalkReferences(arg ObjectType, skip, limit int, walkfn func(sha1, refname string) error) (int, error) { + i := 0 + var iter storer.ReferenceIter + var err error + switch arg { + case ObjectTag: + iter, err = repo.gogitRepo.Tags() + case ObjectBranch: + iter, err = repo.gogitRepo.Branches() + default: + iter, err = repo.gogitRepo.References() + } + if err != nil { + return i, err + } + defer iter.Close() + + err = iter.ForEach(func(ref *plumbing.Reference) error { + if i < skip { + i++ + return nil + } + err := walkfn(ref.Hash().String(), string(ref.Name())) + i++ + if err != nil { + return err + } + if limit != 0 && i >= skip+limit { + return storer.ErrStop + } + return nil + }) + return i, err +} diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index 55952acda4a3..080a7a3b2693 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/modules/git/repo_branch_nogogit.go @@ -68,13 +68,29 @@ func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) { } // WalkReferences walks all the references from the repository -func WalkReferences(ctx context.Context, repoPath string, walkfn func(string) error) (int, error) { +func WalkReferences(ctx context.Context, repoPath string, walkfn func(sha1, refname string) error) (int, error) { return walkShowRef(ctx, repoPath, "", 0, 0, walkfn) } +// WalkReferences walks all the references from the repository +// refType should be empty, ObjectTag or ObjectBranch. All other values are equivalent to empty. +func (repo *Repository) WalkReferences(refType ObjectType, skip, limit int, walkfn func(sha1, refname string) error) (int, error) { + var arg string + switch refType { + case ObjectTag: + arg = "--tags" + case ObjectBranch: + arg = "--heads" + default: + arg = "" + } + + return walkShowRef(repo.Ctx, repo.Path, arg, skip, limit, walkfn) +} + // callShowRef return refs, if limit = 0 it will not limit func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit int) (branchNames []string, countAll int, err error) { - countAll, err = walkShowRef(ctx, repoPath, arg, skip, limit, func(branchName string) error { + countAll, err = walkShowRef(ctx, repoPath, arg, skip, limit, func(_, branchName string) error { branchName = strings.TrimPrefix(branchName, prefix) branchNames = append(branchNames, branchName) @@ -83,7 +99,7 @@ func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit return } -func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, walkfn func(string) error) (countAll int, err error) { +func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) { stdoutReader, stdoutWriter := io.Pipe() defer func() { _ = stdoutReader.Close() @@ -125,11 +141,7 @@ func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, wal for limit == 0 || i < skip+limit { // The output of show-ref is simply a list: // SP LF - _, err := bufReader.ReadSlice(' ') - for err == bufio.ErrBufferFull { - // This shouldn't happen but we'll tolerate it for the sake of peace - _, err = bufReader.ReadSlice(' ') - } + sha, err := bufReader.ReadString(' ') if err == io.EOF { return i, nil } @@ -149,7 +161,12 @@ func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, wal if len(branchName) > 0 { branchName = branchName[:len(branchName)-1] } - err = walkfn(branchName) + + if len(sha) > 0 { + sha = sha[:len(sha)-1] + } + + err = walkfn(sha, branchName) if err != nil { return i, err } diff --git a/modules/git/repo_commitgraph.go b/modules/git/repo_commitgraph.go new file mode 100644 index 000000000000..44c41c0b42fb --- /dev/null +++ b/modules/git/repo_commitgraph.go @@ -0,0 +1,21 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package git + +import ( + "context" + "fmt" +) + +// WriteCommitGraph write commit graph to speed up repo access +// this requires git v2.18 to be installed +func WriteCommitGraph(ctx context.Context, repoPath string) error { + if CheckGitVersionAtLeast("2.18") == nil { + if _, err := NewCommandContext(ctx, "commit-graph", "write").RunInDir(repoPath); err != nil { + return fmt.Errorf("unable to write commit-graph for '%s' : %w", repoPath, err) + } + } + return nil +} diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index 6b5dbeef4823..4e4f0eae3334 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -10,7 +10,6 @@ import ( "fmt" "strings" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" ) @@ -34,69 +33,6 @@ func (repo *Repository) CreateAnnotatedTag(name, message, revision string) error return err } -func (repo *Repository) getTag(tagID SHA1, name string) (*Tag, error) { - t, ok := repo.tagCache.Get(tagID.String()) - if ok { - log.Debug("Hit cache: %s", tagID) - tagClone := *t.(*Tag) - tagClone.Name = name // This is necessary because lightweight tags may have same id - return &tagClone, nil - } - - tp, err := repo.GetTagType(tagID) - if err != nil { - return nil, err - } - - // Get the commit ID and tag ID (may be different for annotated tag) for the returned tag object - commitIDStr, err := repo.GetTagCommitID(name) - if err != nil { - // every tag should have a commit ID so return all errors - return nil, err - } - commitID, err := NewIDFromString(commitIDStr) - if err != nil { - return nil, err - } - - // If type is "commit, the tag is a lightweight tag - if ObjectType(tp) == ObjectCommit { - commit, err := repo.GetCommit(commitIDStr) - if err != nil { - return nil, err - } - tag := &Tag{ - Name: name, - ID: tagID, - Object: commitID, - Type: tp, - Tagger: commit.Committer, - Message: commit.Message(), - } - - repo.tagCache.Set(tagID.String(), tag) - return tag, nil - } - - // The tag is an annotated tag with a message. - data, err := NewCommandContext(repo.Ctx, "cat-file", "-p", tagID.String()).RunInDirBytes(repo.Path) - if err != nil { - return nil, err - } - - tag, err := parseTagData(data) - if err != nil { - return nil, err - } - - tag.Name = name - tag.ID = tagID - tag.Type = tp - - repo.tagCache.Set(tagID.String(), tag) - return tag, nil -} - // GetTagNameBySHA returns the name of a tag from its tag object SHA or commit SHA func (repo *Repository) GetTagNameBySHA(sha string) (string, error) { if len(sha) < 5 { @@ -159,6 +95,20 @@ func (repo *Repository) GetTag(name string) (*Tag, error) { return tag, nil } +// GetTagWithID returns a Git tag by given name and ID +func (repo *Repository) GetTagWithID(idStr, name string) (*Tag, error) { + id, err := NewIDFromString(idStr) + if err != nil { + return nil, err + } + + tag, err := repo.getTag(id, name) + if err != nil { + return nil, err + } + return tag, nil +} + // GetTagInfos returns all tag infos of the repository. func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) { // TODO this a slow implementation, makes one git command per tag @@ -192,19 +142,6 @@ func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) { return tags, tagsTotal, nil } -// GetTagType gets the type of the tag, either commit (simple) or tag (annotated) -func (repo *Repository) GetTagType(id SHA1) (string, error) { - // Get tag type - stdout, err := NewCommandContext(repo.Ctx, "cat-file", "-t", id.String()).RunInDir(repo.Path) - if err != nil { - return "", err - } - if len(stdout) == 0 { - return "", ErrNotExist{ID: id.String()} - } - return strings.TrimSpace(stdout), nil -} - // GetAnnotatedTag returns a Git tag by its SHA, must be an annotated tag func (repo *Repository) GetAnnotatedTag(sha string) (*Tag, error) { id, err := NewIDFromString(sha) diff --git a/modules/git/repo_tag_gogit.go b/modules/git/repo_tag_gogit.go index ff8a6d53eee6..5c87e914c063 100644 --- a/modules/git/repo_tag_gogit.go +++ b/modules/git/repo_tag_gogit.go @@ -11,6 +11,8 @@ package git import ( "strings" + "code.gitea.io/gitea/modules/log" + "github.com/go-git/go-git/v5/plumbing" ) @@ -53,3 +55,83 @@ func (repo *Repository) GetTags(skip, limit int) ([]string, error) { return tagNames, nil } + +// GetTagType gets the type of the tag, either commit (simple) or tag (annotated) +func (repo *Repository) GetTagType(id SHA1) (string, error) { + // Get tag type + obj, err := repo.gogitRepo.Object(plumbing.AnyObject, id) + if err != nil { + if err == plumbing.ErrReferenceNotFound { + return "", &ErrNotExist{ID: id.String()} + } + return "", err + } + + return obj.Type().String(), nil +} + +func (repo *Repository) getTag(tagID SHA1, name string) (*Tag, error) { + t, ok := repo.tagCache.Get(tagID.String()) + if ok { + log.Debug("Hit cache: %s", tagID) + tagClone := *t.(*Tag) + tagClone.Name = name // This is necessary because lightweight tags may have same id + return &tagClone, nil + } + + tp, err := repo.GetTagType(tagID) + if err != nil { + return nil, err + } + + // Get the commit ID and tag ID (may be different for annotated tag) for the returned tag object + commitIDStr, err := repo.GetTagCommitID(name) + if err != nil { + // every tag should have a commit ID so return all errors + return nil, err + } + commitID, err := NewIDFromString(commitIDStr) + if err != nil { + return nil, err + } + + // If type is "commit, the tag is a lightweight tag + if ObjectType(tp) == ObjectCommit { + commit, err := repo.GetCommit(commitIDStr) + if err != nil { + return nil, err + } + tag := &Tag{ + Name: name, + ID: tagID, + Object: commitID, + Type: tp, + Tagger: commit.Committer, + Message: commit.Message(), + } + + repo.tagCache.Set(tagID.String(), tag) + return tag, nil + } + + gogitTag, err := repo.gogitRepo.TagObject(tagID) + if err != nil { + if err == plumbing.ErrReferenceNotFound { + return nil, &ErrNotExist{ID: tagID.String()} + } + + return nil, err + } + + tag := &Tag{ + Name: name, + ID: tagID, + Object: gogitTag.Target, + Type: tp, + Tagger: &gogitTag.Tagger, + Message: gogitTag.Message, + } + + repo.tagCache.Set(tagID.String(), tag) + return tag, nil +} diff --git a/modules/git/repo_tag_nogogit.go b/modules/git/repo_tag_nogogit.go index 1a23755aa666..f2da7d88572b 100644 --- a/modules/git/repo_tag_nogogit.go +++ b/modules/git/repo_tag_nogogit.go @@ -8,6 +8,13 @@ package git +import ( + "errors" + "io" + + "code.gitea.io/gitea/modules/log" +) + // IsTagExist returns true if given tag exists in the repository. func (repo *Repository) IsTagExist(name string) bool { if name == "" { @@ -23,3 +30,104 @@ func (repo *Repository) GetTags(skip, limit int) (tags []string, err error) { tags, _, err = callShowRef(repo.Ctx, repo.Path, TagPrefix, "--tags", skip, limit) return } + +// GetTagType gets the type of the tag, either commit (simple) or tag (annotated) +func (repo *Repository) GetTagType(id SHA1) (string, error) { + wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx) + defer cancel() + _, err := wr.Write([]byte(id.String() + "\n")) + if err != nil { + return "", err + } + _, typ, _, err := ReadBatchLine(rd) + if IsErrNotExist(err) { + return "", ErrNotExist{ID: id.String()} + } + return typ, nil +} + +func (repo *Repository) getTag(tagID SHA1, name string) (*Tag, error) { + t, ok := repo.tagCache.Get(tagID.String()) + if ok { + log.Debug("Hit cache: %s", tagID) + tagClone := *t.(*Tag) + tagClone.Name = name // This is necessary because lightweight tags may have same id + return &tagClone, nil + } + + tp, err := repo.GetTagType(tagID) + if err != nil { + return nil, err + } + + // Get the commit ID and tag ID (may be different for annotated tag) for the returned tag object + commitIDStr, err := repo.GetTagCommitID(name) + if err != nil { + // every tag should have a commit ID so return all errors + return nil, err + } + commitID, err := NewIDFromString(commitIDStr) + if err != nil { + return nil, err + } + + // If type is "commit, the tag is a lightweight tag + if ObjectType(tp) == ObjectCommit { + commit, err := repo.GetCommit(commitIDStr) + if err != nil { + return nil, err + } + tag := &Tag{ + Name: name, + ID: tagID, + Object: commitID, + Type: tp, + Tagger: commit.Committer, + Message: commit.Message(), + } + + repo.tagCache.Set(tagID.String(), tag) + return tag, nil + } + + // The tag is an annotated tag with a message. + wr, rd, cancel := repo.CatFileBatch(repo.Ctx) + defer cancel() + + if _, err := wr.Write([]byte(tagID.String() + "\n")); err != nil { + return nil, err + } + _, typ, size, err := ReadBatchLine(rd) + if err != nil { + if errors.Is(err, io.EOF) || IsErrNotExist(err) { + return nil, ErrNotExist{ID: tagID.String()} + } + return nil, err + } + if typ != "tag" { + return nil, ErrNotExist{ID: tagID.String()} + } + + // then we need to parse the tag + // and load the commit + data, err := io.ReadAll(io.LimitReader(rd, size)) + if err != nil { + return nil, err + } + _, err = rd.Discard(1) + if err != nil { + return nil, err + } + + tag, err := parseTagData(data) + if err != nil { + return nil, err + } + + tag.Name = name + tag.ID = tagID + tag.Type = tp + + repo.tagCache.Set(tagID.String(), tag) + return tag, nil +} diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 83db165a5b2b..b79260384af1 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -80,6 +80,10 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, return repo, fmt.Errorf("Clone: %v", err) } + if err := git.WriteCommitGraph(ctx, repoPath); err != nil { + return repo, err + } + if opts.Wiki { wikiPath := repo_model.WikiPath(u.Name, opts.RepoName) wikiRemotePath := WikiRemoteURL(opts.CloneAddr) @@ -101,6 +105,9 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, } } } + if err := git.WriteCommitGraph(ctx, wikiPath); err != nil { + return repo, err + } } if repo.OwnerID == u.ID { @@ -278,23 +285,25 @@ func SyncReleasesWithTags(repo *repo_model.Repository, gitRepo *git.Repository) } } } - tags, err := gitRepo.GetTags(0, 0) - if err != nil { - return fmt.Errorf("unable to GetTags in Repo[%d:%s/%s]: %w", repo.ID, repo.OwnerName, repo.Name, err) - } - for _, tagName := range tags { - if _, ok := existingRelTags[strings.ToLower(tagName)]; !ok { - if err := PushUpdateAddTag(repo, gitRepo, tagName); err != nil { - return fmt.Errorf("unable to PushUpdateAddTag: %q to Repo[%d:%s/%s]: %w", tagName, repo.ID, repo.OwnerName, repo.Name, err) - } + + _, err := gitRepo.WalkReferences(git.ObjectTag, 0, 0, func(sha1, refname string) error { + tagName := strings.TrimPrefix(refname, git.TagPrefix) + if _, ok := existingRelTags[strings.ToLower(tagName)]; ok { + return nil } - } - return nil + + if err := PushUpdateAddTag(repo, gitRepo, tagName, sha1, refname); err != nil { + return fmt.Errorf("unable to PushUpdateAddTag: %q to Repo[%d:%s/%s]: %w", tagName, repo.ID, repo.OwnerName, repo.Name, err) + } + + return nil + }) + return err } // PushUpdateAddTag must be called for any push actions to add tag -func PushUpdateAddTag(repo *repo_model.Repository, gitRepo *git.Repository, tagName string) error { - tag, err := gitRepo.GetTag(tagName) +func PushUpdateAddTag(repo *repo_model.Repository, gitRepo *git.Repository, tagName, sha1, refname string) error { + tag, err := gitRepo.GetTagWithID(sha1, tagName) if err != nil { return fmt.Errorf("unable to GetTag: %w", err) } diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 000b076fb36a..754054d14d81 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -200,6 +200,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo timeout := time.Duration(setting.Git.Timeout.Mirror) * time.Second log.Trace("SyncMirrors [repo: %-v]: running git remote update...", m.Repo) + gitArgs := []string{"remote", "update"} if m.EnablePrune { gitArgs = append(gitArgs, "--prune") @@ -262,7 +263,11 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo } output := stderrBuilder.String() - gitRepo, err := git.OpenRepository(repoPath) + if err := git.WriteCommitGraph(ctx, repoPath); err != nil { + log.Error("SyncMirrors [repo: %-v]: %v", m.Repo, err) + } + + gitRepo, err := git.OpenRepositoryCtx(ctx, repoPath) if err != nil { log.Error("SyncMirrors [repo: %-v]: failed to OpenRepository: %v", m.Repo, err) return nil, false @@ -344,6 +349,10 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo } return nil, false } + + if err := git.WriteCommitGraph(ctx, wikiPath); err != nil { + log.Error("SyncMirrors [repo: %-v]: %v", m.Repo, err) + } } log.Trace("SyncMirrors [repo: %-v Wiki]: git remote update complete", m.Repo) } diff --git a/services/repository/branch.go b/services/repository/branch.go index e1775fc12b14..f16e3eca0a92 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -69,7 +69,7 @@ func GetBranches(repo *repo_model.Repository, skip, limit int) ([]*git.Branch, i // checkBranchName validates branch name with existing repository branches func checkBranchName(ctx context.Context, repo *repo_model.Repository, name string) error { - _, err := git.WalkReferences(ctx, repo.RepoPath(), func(refName string) error { + _, err := git.WalkReferences(ctx, repo.RepoPath(), func(_, refName string) error { branchRefName := strings.TrimPrefix(refName, git.BranchPrefix) switch { case branchRefName == name: From c37f30d8e623042f683bd3b13ff5a8ddc57e3e40 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 29 Mar 2022 19:33:11 +0200 Subject: [PATCH 2/2] fix rebase relict --- modules/git/repo_branch_gogit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_branch_gogit.go b/modules/git/repo_branch_gogit.go index e9e27a357053..13b42306a61c 100644 --- a/modules/git/repo_branch_gogit.go +++ b/modules/git/repo_branch_gogit.go @@ -85,7 +85,7 @@ func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) { // WalkReferences walks all the references from the repository // refType should be empty, ObjectTag or ObjectBranch. All other values are equivalent to empty. func WalkReferences(ctx context.Context, repoPath string, walkfn func(sha1, refname string) error) (int, error) { - repo, err = OpenRepositoryCtx(ctx, repoPath) + repo, err := OpenRepositoryCtx(ctx, repoPath) if err != nil { return 0, err }