From 69055400881777148d5e5d3f531c563ebfdb9287 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 24 Feb 2024 14:55:19 +0800 Subject: [PATCH] Use the database object format name but not read from git repoisitory everytime and fix possible migration wrong objectformat when migrating a sha256 repository (#29294) Now we can get object format name from git command line or from the database repository table. Assume the column is right, we don't need to read from git command line every time. This also fixed a possible bug that the object format is wrong when migrating a sha256 repository from external. image (cherry picked from commit b79c30435f439af8243ee281310258cdf141e27b) Conflicts: routers/web/repo/blame.go services/agit/agit.go context --- modules/context/api.go | 9 ++------- modules/context/repo.go | 20 ++++++++------------ routers/api/v1/utils/git.go | 2 +- routers/private/hook_pre_receive.go | 2 +- routers/web/repo/blame.go | 5 +---- routers/web/repo/compare.go | 4 ++-- routers/web/repo/setting/lfs.go | 2 +- services/agit/agit.go | 5 +---- services/migrations/gitea_uploader.go | 16 +++++++++++++--- services/pull/check.go | 5 +---- services/pull/merge.go | 2 +- services/release/release.go | 2 +- services/repository/branch.go | 7 ++----- services/repository/files/commit.go | 6 ++---- services/repository/files/tree.go | 2 +- services/repository/lfs.go | 2 +- services/repository/push.go | 6 +----- 17 files changed, 40 insertions(+), 57 deletions(-) diff --git a/modules/context/api.go b/modules/context/api.go index 7557a5f43..fafd49fd4 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -309,12 +309,6 @@ func RepoRefForAPI(next http.Handler) http.Handler { return } - objectFormat, err := ctx.Repo.GitRepo.GetObjectFormat() - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetCommit", err) - return - } - if ref := ctx.FormTrim("ref"); len(ref) > 0 { commit, err := ctx.Repo.GitRepo.GetCommit(ref) if err != nil { @@ -333,6 +327,7 @@ func RepoRefForAPI(next http.Handler) http.Handler { } refName := getRefName(ctx.Base, ctx.Repo, RepoRefAny) + var err error if ctx.Repo.GitRepo.IsBranchExist(refName) { ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refName) @@ -348,7 +343,7 @@ func RepoRefForAPI(next http.Handler) http.Handler { return } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if len(refName) == objectFormat.FullLength() { + } else if len(refName) == ctx.Repo.GetObjectFormat().FullLength() { ctx.Repo.CommitID = refName ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName) if err != nil { diff --git a/modules/context/repo.go b/modules/context/repo.go index 490d798d5..e8ed1134f 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -83,6 +83,10 @@ func (r *Repository) CanCreateBranch() bool { return r.Permission.CanWrite(unit_model.TypeCode) && r.Repository.CanCreateBranch() } +func (r *Repository) GetObjectFormat() git.ObjectFormat { + return git.ObjectFormatFromName(r.Repository.ObjectFormatName) +} + // RepoMustNotBeArchived checks if a repo is archived func RepoMustNotBeArchived() func(ctx *Context) { return func(ctx *Context) { @@ -831,9 +835,8 @@ func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string { } // For legacy and API support only full commit sha parts := strings.Split(path, "/") - objectFormat, _ := repo.GitRepo.GetObjectFormat() - if len(parts) > 0 && len(parts[0]) == objectFormat.FullLength() { + if len(parts) > 0 && len(parts[0]) == git.ObjectFormatFromName(repo.Repository.ObjectFormatName).FullLength() { repo.TreePath = strings.Join(parts[1:], "/") return parts[0] } @@ -877,9 +880,8 @@ func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string { return getRefNameFromPath(ctx, repo, path, repo.GitRepo.IsTagExist) case RepoRefCommit: parts := strings.Split(path, "/") - objectFormat, _ := repo.GitRepo.GetObjectFormat() - if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= objectFormat.FullLength() { + if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= repo.GetObjectFormat().FullLength() { repo.TreePath = strings.Join(parts[1:], "/") return parts[0] } @@ -938,12 +940,6 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context } } - objectFormat, err := ctx.Repo.GitRepo.GetObjectFormat() - if err != nil { - log.Error("Cannot determine objectFormat for repository: %w", err) - ctx.Repo.Repository.MarkAsBrokenEmpty() - } - // Get default branch. if len(ctx.Params("*")) == 0 { refName = ctx.Repo.Repository.DefaultBranch @@ -1010,7 +1006,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context return cancel } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if len(refName) >= 7 && len(refName) <= objectFormat.FullLength() { + } else if len(refName) >= 7 && len(refName) <= ctx.Repo.GetObjectFormat().FullLength() { ctx.Repo.IsViewCommit = true ctx.Repo.CommitID = refName @@ -1020,7 +1016,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context return cancel } // If short commit ID add canonical link header - if len(refName) < objectFormat.FullLength() { + if len(refName) < ctx.Repo.GetObjectFormat().FullLength() { ctx.RespHeader().Set("Link", fmt.Sprintf("<%s>; rel=\"canonical\"", util.URLJoin(setting.AppURL, strings.Replace(ctx.Req.URL.RequestURI(), util.PathEscapeSegments(refName), url.PathEscape(ctx.Repo.Commit.ID.String()), 1)))) } diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index 2299cdc24..5e8019001 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -72,7 +72,7 @@ func searchRefCommitByType(ctx *context.APIContext, refType, filter string) (str // ConvertToObjectID returns a full-length SHA1 from a potential ID string func ConvertToObjectID(ctx gocontext.Context, repo *context.Repository, commitID string) (git.ObjectID, error) { - objectFormat, _ := repo.GitRepo.GetObjectFormat() + objectFormat := repo.GetObjectFormat() if len(commitID) == objectFormat.FullLength() && objectFormat.IsValid(commitID) { sha, err := git.NewIDFromString(commitID) if err == nil { diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 90d8287f0..f28ae4c0e 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -145,7 +145,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r repo := ctx.Repo.Repository gitRepo := ctx.Repo.GitRepo - objectFormat, _ := gitRepo.GetObjectFormat() + objectFormat := ctx.Repo.GetObjectFormat() if branchName == repo.DefaultBranch && newCommitID == objectFormat.EmptyObjectID().String() { log.Warn("Forbidden: Branch: %s is the default branch in %-v and cannot be deleted", branchName, repo) diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index 49443162e..2c938dc96 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -112,10 +112,7 @@ type blameResult struct { func performBlame(ctx *context.Context, commit *git.Commit, file string, bypassBlameIgnore bool) (*blameResult, error) { repoPath := ctx.Repo.Repository.RepoPath() - objectFormat, err := ctx.Repo.GitRepo.GetObjectFormat() - if err != nil { - return nil, err - } + objectFormat := ctx.Repo.GetObjectFormat() blameReader, err := git.CreateBlameReader(ctx, objectFormat, repoPath, commit, file, bypassBlameIgnore) if err != nil { diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index df41c750d..535487d5f 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -312,14 +312,14 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo { baseIsCommit := ctx.Repo.GitRepo.IsCommitExist(ci.BaseBranch) baseIsBranch := ctx.Repo.GitRepo.IsBranchExist(ci.BaseBranch) baseIsTag := ctx.Repo.GitRepo.IsTagExist(ci.BaseBranch) - objectFormat, _ := ctx.Repo.GitRepo.GetObjectFormat() + if !baseIsCommit && !baseIsBranch && !baseIsTag { // Check if baseBranch is short sha commit hash if baseCommit, _ := ctx.Repo.GitRepo.GetCommit(ci.BaseBranch); baseCommit != nil { ci.BaseBranch = baseCommit.ID.String() ctx.Data["BaseBranch"] = ci.BaseBranch baseIsCommit = true - } else if ci.BaseBranch == objectFormat.EmptyObjectID().String() { + } else if ci.BaseBranch == ctx.Repo.GetObjectFormat().EmptyObjectID().String() { if isSameRepo { ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(ci.HeadBranch)) } else { diff --git a/routers/web/repo/setting/lfs.go b/routers/web/repo/setting/lfs.go index e360ae2b8..53e7b22d1 100644 --- a/routers/web/repo/setting/lfs.go +++ b/routers/web/repo/setting/lfs.go @@ -388,7 +388,7 @@ func LFSFileFind(ctx *context.Context) { sha := ctx.FormString("sha") ctx.Data["Title"] = oid ctx.Data["PageIsSettingsLFS"] = true - objectFormat, _ := ctx.Repo.GitRepo.GetObjectFormat() + objectFormat := ctx.Repo.GetObjectFormat() var objectID git.ObjectID if len(sha) == 0 { pointer := lfs.Pointer{Oid: oid, Size: size} diff --git a/services/agit/agit.go b/services/agit/agit.go index 4c970ee78..e46a5771e 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -28,10 +28,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. title, hasTitle := opts.GitPushOptions["title"] description, hasDesc := opts.GitPushOptions["description"] - objectFormat, err := gitRepo.GetObjectFormat() - if err != nil { - return nil, fmt.Errorf("couldn't get object format of the repository: %w", err) - } + objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) pusher, err := user_model.GetUserByID(ctx, opts.UserID) if err != nil { diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index ebb1313c2..7920c0705 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -140,8 +140,18 @@ func (g *GiteaLocalUploader) CreateRepo(repo *base.Repository, opts base.Migrate if err != nil { return err } - g.gitRepo, err = gitrepo.OpenRepository(g.ctx, r) - return err + g.gitRepo, err = gitrepo.OpenRepository(g.ctx, g.repo) + if err != nil { + return err + } + + // detect object format from git repository and update to database + objectFormat, err := g.gitRepo.GetObjectFormat() + if err != nil { + return err + } + g.repo.ObjectFormatName = objectFormat.Name() + return repo_model.UpdateRepositoryCols(g.ctx, g.repo, "object_format_name") } // Close closes this uploader @@ -901,7 +911,7 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error { comment.UpdatedAt = comment.CreatedAt } - objectFormat, _ := g.gitRepo.GetObjectFormat() + objectFormat := git.ObjectFormatFromName(g.repo.ObjectFormatName) if !objectFormat.IsValid(comment.CommitID) { log.Warn("Invalid comment CommitID[%s] on comment[%d] in PR #%d of %s/%s replaced with %s", comment.CommitID, pr.Index, g.repoOwner, g.repoName, headCommitID) comment.CommitID = headCommitID diff --git a/services/pull/check.go b/services/pull/check.go index dd6c3ed23..f4dd332b1 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -222,10 +222,7 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com } defer gitRepo.Close() - objectFormat, err := gitRepo.GetObjectFormat() - if err != nil { - return nil, fmt.Errorf("%-v GetObjectFormat: %w", pr.BaseRepo, err) - } + objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName) // Get the commit from BaseBranch where the pull request got merged mergeCommit, _, err := git.NewCommand(ctx, "rev-list", "--ancestry-path", "--merges", "--reverse"). diff --git a/services/pull/merge.go b/services/pull/merge.go index f09067996..2112108d4 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -503,7 +503,7 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: repo_model.MergeStyleManuallyMerged} } - objectFormat, _ := baseGitRepo.GetObjectFormat() + objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName) if len(commitID) != objectFormat.FullLength() { return fmt.Errorf("Wrong commit ID") } diff --git a/services/release/release.go b/services/release/release.go index 4c522c18b..a359e5078 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -88,7 +88,7 @@ func createTag(ctx context.Context, gitRepo *git.Repository, rel *repo_model.Rel created = true rel.LowerTagName = strings.ToLower(rel.TagName) - objectFormat, _ := gitRepo.GetObjectFormat() + objectFormat := git.ObjectFormatFromName(rel.Repo.ObjectFormatName) commits := repository.NewPushCommits() commits.HeadCommit = repository.CommitToPushCommit(commit) commits.CompareURL = rel.Repo.ComposeCompareURL(objectFormat.EmptyObjectID().String(), commit.ID.String()) diff --git a/services/repository/branch.go b/services/repository/branch.go index 6add9422f..39be3984a 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -369,11 +369,6 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R return fmt.Errorf("GetBranch: %v", err) } - objectFormat, err := gitRepo.GetObjectFormat() - if err != nil { - return err - } - if rawBranch.IsDeleted { return nil } @@ -395,6 +390,8 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R return err } + objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) + // Don't return error below this if err := PushUpdate( &repo_module.PushUpdateOptions{ diff --git a/services/repository/files/commit.go b/services/repository/files/commit.go index 16a15e06a..512aec7c8 100644 --- a/services/repository/files/commit.go +++ b/services/repository/files/commit.go @@ -30,10 +30,8 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato } defer closer.Close() - objectFormat, err := gitRepo.GetObjectFormat() - if err != nil { - return fmt.Errorf("GetObjectFormat[%s]: %w", repoPath, err) - } + objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) + commit, err := gitRepo.GetCommit(sha) if err != nil { gitRepo.Close() diff --git a/services/repository/files/tree.go b/services/repository/files/tree.go index 9d3185c3f..e3a7f3b8b 100644 --- a/services/repository/files/tree.go +++ b/services/repository/files/tree.go @@ -37,7 +37,7 @@ func GetTreeBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git } apiURL := repo.APIURL() apiURLLen := len(apiURL) - objectFormat, _ := gitRepo.GetObjectFormat() + objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) hashLen := objectFormat.FullLength() const gitBlobsPath = "/git/blobs/" diff --git a/services/repository/lfs.go b/services/repository/lfs.go index 4504f796b..4d48881b8 100644 --- a/services/repository/lfs.go +++ b/services/repository/lfs.go @@ -79,7 +79,7 @@ func GarbageCollectLFSMetaObjectsForRepo(ctx context.Context, repo *repo_model.R store := lfs.NewContentStore() errStop := errors.New("STOPERR") - objectFormat, _ := gitRepo.GetObjectFormat() + objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) err = git_model.IterateLFSMetaObjectsForRepo(ctx, repo.ID, func(ctx context.Context, metaObject *git_model.LFSMetaObject, count int64) error { if opts.NumberToCheckPerRepo > 0 && total > opts.NumberToCheckPerRepo { diff --git a/services/repository/push.go b/services/repository/push.go index 5e2853b27..bb080e30c 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -93,11 +93,6 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } defer gitRepo.Close() - objectFormat, err := gitRepo.GetObjectFormat() - if err != nil { - return fmt.Errorf("unknown repository ObjectFormat [%s]: %w", repo.FullName(), err) - } - if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { return fmt.Errorf("Failed to update size for repository: %v", err) } @@ -105,6 +100,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { addTags := make([]string, 0, len(optsList)) delTags := make([]string, 0, len(optsList)) var pusher *user_model.User + objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) for _, opts := range optsList { log.Trace("pushUpdates: %-v %s %s %s", repo, opts.OldCommitID, opts.NewCommitID, opts.RefFullName)