From 889a8c268ca6a54ff5be19e61b29b10feb4a12e8 Mon Sep 17 00:00:00 2001 From: zeripath Date: Tue, 29 Mar 2022 18:12:33 +0100 Subject: [PATCH] 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 | 42 ++++++++++- 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 | 9 +++ services/repository/branch.go | 2 +- 9 files changed, 323 insertions(+), 102 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 57952bcc6..59ae0eaa0 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,7 +83,8 @@ 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) { +// 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 := RepositoryFromContext(ctx, repoPath) if repo == nil { var err error @@ -101,9 +103,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 66990add6..f595b6d9a 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() @@ -130,11 +146,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 } @@ -154,7 +166,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 000000000..5549c9259 --- /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 := NewCommand(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 afeb7f5df..d1b076ffc 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 := NewCommand(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 := NewCommand(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 ff8a6d53e..5c87e914c 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 1a23755aa..f2da7d885 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 dbf4b527b..7170bed8b 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -81,6 +81,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(ctx, opts.CloneAddr) @@ -102,6 +106,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 { @@ -279,23 +286,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 d142a48ca..a8a646a51 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -204,6 +204,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") @@ -276,6 +277,10 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo } output := stderrBuilder.String() + 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) @@ -368,6 +373,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 b1a6dafb5..6667cdee6 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -55,7 +55,7 @@ func GetBranches(ctx context.Context, repo *repo_model.Repository, skip, limit 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: