From a38ff9ae004f0589803c393203c403428de25e79 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 23 Jul 2025 22:00:28 -0700 Subject: [PATCH 1/8] Move git config/remote to gitrepo package and add global lock to resolve possible conflict when updating repository git config file --- modules/git/remote.go | 10 --- modules/git/repo.go | 6 +- modules/git/repo_branch.go | 6 -- modules/git/repo_commit.go | 14 +-- modules/git/repo_compare.go | 89 ------------------- modules/gitrepo/config.go | 132 ++++++++++++++++++++++++++++ routers/api/v1/repo/pull.go | 16 ++-- routers/common/compare.go | 3 +- routers/web/repo/compare.go | 3 +- routers/web/repo/pull.go | 12 +-- routers/web/repo/setting/setting.go | 3 +- services/doctor/misc.go | 9 +- services/issue/pull.go | 15 ++-- services/migrations/dump.go | 1 + services/mirror/mirror_pull.go | 52 ++++------- services/mirror/mirror_push.go | 33 +++---- services/pull/compare.go | 102 +++++++++++++++++++++ services/pull/pull.go | 4 +- services/repository/migrate.go | 19 +--- 19 files changed, 312 insertions(+), 217 deletions(-) create mode 100644 modules/gitrepo/config.go create mode 100644 services/pull/compare.go diff --git a/modules/git/remote.go b/modules/git/remote.go index 876c3d6acb8..283d26484bb 100644 --- a/modules/git/remote.go +++ b/modules/git/remote.go @@ -9,7 +9,6 @@ import ( "net/url" "strings" - giturl "code.gitea.io/gitea/modules/git/url" "code.gitea.io/gitea/modules/util" ) @@ -33,15 +32,6 @@ func GetRemoteAddress(ctx context.Context, repoPath, remoteName string) (string, return result, nil } -// GetRemoteURL returns the url of a specific remote of the repository. -func GetRemoteURL(ctx context.Context, repoPath, remoteName string) (*giturl.GitURL, error) { - addr, err := GetRemoteAddress(ctx, repoPath, remoteName) - if err != nil { - return nil, err - } - return giturl.ParseGitURL(addr) -} - // ErrInvalidCloneAddr represents a "InvalidCloneAddr" kind of error. type ErrInvalidCloneAddr struct { Host string diff --git a/modules/git/repo.go b/modules/git/repo.go index f1f6902773a..ddc96d036cf 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -31,13 +31,17 @@ type GPGSettings struct { Format string } -const prettyLogFormat = `--pretty=format:%H` +const PrettyLogFormat = `--pretty=format:%H` // GetAllCommitsCount returns count of all commits in repository func (repo *Repository) GetAllCommitsCount() (int64, error) { return AllCommitsCount(repo.Ctx, repo.Path, false) } +func (repo *Repository) ParsePrettyFormatLogToList(logs []byte) ([]*Commit, error) { + return repo.parsePrettyFormatLogToList(logs) +} + func (repo *Repository) parsePrettyFormatLogToList(logs []byte) ([]*Commit, error) { var commits []*Commit if len(logs) == 0 { diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index e7ecf53f51f..eac20310035 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -79,12 +79,6 @@ func (repo *Repository) AddRemote(name, url string, fetch bool) error { return err } -// RemoveRemote removes a remote from repository. -func (repo *Repository) RemoveRemote(name string) error { - _, _, err := NewCommand("remote", "rm").AddDynamicArguments(name).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) - return err -} - // RenameBranch rename a branch func (repo *Repository) RenameBranch(from, to string) error { _, _, err := NewCommand("branch", "-m").AddDynamicArguments(from, to).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 4066a1ca7ba..269538beabc 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -59,7 +59,7 @@ func (repo *Repository) getCommitByPathWithID(id ObjectID, relpath string) (*Com relpath = `\` + relpath } - stdout, _, runErr := NewCommand("log", "-1", prettyLogFormat).AddDynamicArguments(id.String()).AddDashesAndList(relpath).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) + stdout, _, runErr := NewCommand("log", "-1", PrettyLogFormat).AddDynamicArguments(id.String()).AddDashesAndList(relpath).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) if runErr != nil { return nil, runErr } @@ -74,7 +74,7 @@ func (repo *Repository) getCommitByPathWithID(id ObjectID, relpath string) (*Com // GetCommitByPath returns the last commit of relative path. func (repo *Repository) GetCommitByPath(relpath string) (*Commit, error) { - stdout, _, runErr := NewCommand("log", "-1", prettyLogFormat).AddDashesAndList(relpath).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path}) + stdout, _, runErr := NewCommand("log", "-1", PrettyLogFormat).AddDashesAndList(relpath).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path}) if runErr != nil { return nil, runErr } @@ -94,7 +94,7 @@ func (repo *Repository) commitsByRangeWithTime(id ObjectID, page, pageSize int, cmd := NewCommand("log"). AddOptionFormat("--skip=%d", (page-1)*pageSize). AddOptionFormat("--max-count=%d", pageSize). - AddArguments(prettyLogFormat). + AddArguments(PrettyLogFormat). AddDynamicArguments(id.String()) if not != "" { @@ -141,7 +141,7 @@ func (repo *Repository) searchCommits(id ObjectID, opts SearchCommitsOptions) ([ } // create new git log command with limit of 100 commits - cmd := NewCommand("log", "-100", prettyLogFormat).AddDynamicArguments(id.String()) + cmd := NewCommand("log", "-100", PrettyLogFormat).AddDynamicArguments(id.String()) // pretend that all refs along with HEAD were listed on command line as // https://git-scm.com/docs/git-log#Documentation/git-log.txt---all @@ -175,7 +175,7 @@ func (repo *Repository) searchCommits(id ObjectID, opts SearchCommitsOptions) ([ // ignore anything not matching a valid sha pattern if id.Type().IsValid(v) { // create new git log command with 1 commit limit - hashCmd := NewCommand("log", "-1", prettyLogFormat) + hashCmd := NewCommand("log", "-1", PrettyLogFormat) // add previous arguments except for --grep and --all addCommonSearchArgs(hashCmd) // add keyword as @@ -410,7 +410,7 @@ func (repo *Repository) CommitsCountBetween(start, end string) (int64, error) { // commitsBefore the limit is depth, not total number of returned commits. func (repo *Repository) commitsBefore(id ObjectID, limit int) ([]*Commit, error) { - cmd := NewCommand("log", prettyLogFormat) + cmd := NewCommand("log", PrettyLogFormat) if limit > 0 { cmd.AddOptionFormat("-%d", limit) } @@ -536,7 +536,7 @@ func (repo *Repository) AddLastCommitCache(cacheKey, fullName, sha string) error // GetCommitBranchStart returns the commit where the branch diverged func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID string) (string, error) { - cmd := NewCommand("log", prettyLogFormat) + cmd := NewCommand("log", PrettyLogFormat) cmd.AddDynamicArguments(endCommitID) stdout, _, runErr := cmd.RunStdBytes(repo.Ctx, &RunOpts{ diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index ff44506e13c..56251a87a0f 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -16,20 +16,8 @@ import ( "regexp" "strconv" "strings" - "time" - - logger "code.gitea.io/gitea/modules/log" ) -// CompareInfo represents needed information for comparing references. -type CompareInfo struct { - MergeBase string - BaseCommitID string - HeadCommitID string - Commits []*Commit - NumFiles int -} - // GetMergeBase checks and returns merge base of two branches and the reference used as base. func (repo *Repository) GetMergeBase(tmpRemote, base, head string) (string, string, error) { if tmpRemote == "" { @@ -49,83 +37,6 @@ func (repo *Repository) GetMergeBase(tmpRemote, base, head string) (string, stri return strings.TrimSpace(stdout), base, err } -// GetCompareInfo generates and returns compare information between base and head branches of repositories. -func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string, directComparison, fileOnly bool) (_ *CompareInfo, err error) { - var ( - remoteBranch string - tmpRemote string - ) - - // We don't need a temporary remote for same repository. - if repo.Path != basePath { - // Add a temporary remote - tmpRemote = strconv.FormatInt(time.Now().UnixNano(), 10) - if err = repo.AddRemote(tmpRemote, basePath, false); err != nil { - return nil, fmt.Errorf("AddRemote: %w", err) - } - defer func() { - if err := repo.RemoveRemote(tmpRemote); err != nil { - logger.Error("GetPullRequestInfo: RemoveRemote: %v", err) - } - }() - } - - compareInfo := new(CompareInfo) - - compareInfo.HeadCommitID, err = GetFullCommitID(repo.Ctx, repo.Path, headBranch) - if err != nil { - compareInfo.HeadCommitID = headBranch - } - - compareInfo.MergeBase, remoteBranch, err = repo.GetMergeBase(tmpRemote, baseBranch, headBranch) - if err == nil { - compareInfo.BaseCommitID, err = GetFullCommitID(repo.Ctx, repo.Path, remoteBranch) - if err != nil { - compareInfo.BaseCommitID = remoteBranch - } - separator := "..." - baseCommitID := compareInfo.MergeBase - if directComparison { - separator = ".." - baseCommitID = compareInfo.BaseCommitID - } - - // We have a common base - therefore we know that ... should work - if !fileOnly { - // avoid: ambiguous argument 'refs/a...refs/b': unknown revision or path not in the working tree. Use '--': 'git [...] -- [...]' - var logs []byte - logs, _, err = NewCommand("log").AddArguments(prettyLogFormat). - AddDynamicArguments(baseCommitID+separator+headBranch).AddArguments("--"). - RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path}) - if err != nil { - return nil, err - } - compareInfo.Commits, err = repo.parsePrettyFormatLogToList(logs) - if err != nil { - return nil, fmt.Errorf("parsePrettyFormatLogToList: %w", err) - } - } else { - compareInfo.Commits = []*Commit{} - } - } else { - compareInfo.Commits = []*Commit{} - compareInfo.MergeBase, err = GetFullCommitID(repo.Ctx, repo.Path, remoteBranch) - if err != nil { - compareInfo.MergeBase = remoteBranch - } - compareInfo.BaseCommitID = compareInfo.MergeBase - } - - // Count number of changed files. - // This probably should be removed as we need to use shortstat elsewhere - // Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly - compareInfo.NumFiles, err = repo.GetDiffNumChangedFiles(remoteBranch, headBranch, directComparison) - if err != nil { - return nil, err - } - return compareInfo, nil -} - type lineCountWriter struct { numLines int } diff --git a/modules/gitrepo/config.go b/modules/gitrepo/config.go new file mode 100644 index 00000000000..e2f35343b31 --- /dev/null +++ b/modules/gitrepo/config.go @@ -0,0 +1,132 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitrepo + +import ( + "context" + "io" + "time" + + "code.gitea.io/gitea/modules/git" + giturl "code.gitea.io/gitea/modules/git/url" + "code.gitea.io/gitea/modules/globallock" +) + +func GetGitConfig(ctx context.Context, repo Repository, key string) (string, error) { + result, _, err := git.NewCommand("config", "--get"). + AddDynamicArguments(key). + RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)}) + if err != nil { + return "", err + } + if len(result) > 0 { + result = result[:len(result)-1] // remove trailing newline + } + return result, nil +} + +func getRepoConfigLockKey(repoStoragePath string) string { + return "repo-config:" + repoStoragePath +} + +// AddGitConfig add a git configuration key to a specific value for the given repository. +func AddGitConfig(ctx context.Context, repo Repository, key, value string) error { + releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) + if err != nil { + return err + } + defer releaser() + + _, _, err = git.NewCommand("config", "--add"). + AddDynamicArguments(key, value). + RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)}) + return err +} + +// UpdateGitConfig updates a git configuration key to a specific value for the given repository. +// If the key does not exist, it will be created. +// If the key exists, it will be updated to the new value. +func UpdateGitConfig(ctx context.Context, repo Repository, key, value string) (string, error) { + releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) + if err != nil { + return "", err + } + defer releaser() + + value, _, err1 := git.NewCommand("config"). + AddDynamicArguments(key, value). + RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)}) + return value, err1 +} + +func AddGitRemote(ctx context.Context, repo Repository, remoteName, remoteURL string, options ...string) error { + releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) + if err != nil { + return err + } + defer releaser() + + cmd := git.NewCommand("remote", "add") + if len(options) > 0 { + cmd.AddDynamicArguments(options...) + } + _, _, err = cmd. + AddDynamicArguments(remoteName, remoteURL). + RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)}) + return err +} + +func RemoveGitRemote(ctx context.Context, repo Repository, remoteName string) error { + releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) + if err != nil { + return err + } + defer releaser() + + cmd := git.NewCommand("remote", "rm").AddDynamicArguments(remoteName) + _, _, err = cmd.RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)}) + return err +} + +// GetRemoteURL returns the url of a specific remote of the repository. +func GetRemoteURL(ctx context.Context, repo Repository, remoteName string) (*giturl.GitURL, error) { + addr, err := git.GetRemoteAddress(ctx, repoPath(repo), remoteName) + if err != nil { + return nil, err + } + return giturl.ParseGitURL(addr) +} + +// PruneRemote prunes the remote branches that no longer exist in the remote repository. +func PruneRemote(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error { + releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) + if err != nil { + return err + } + defer releaser() + + return git.NewCommand("remote", "prune").AddDynamicArguments(remoteName). + Run(ctx, &git.RunOpts{ + Timeout: timeout, + Dir: repoPath(repo), + Stdout: stdout, + Stderr: stderr, + }) +} + +func UpdateRemotePrune(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error { + releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) + if err != nil { + return err + } + defer releaser() + + return git.NewCommand("remote", "update", "--prune").AddDynamicArguments(remoteName). + Run(ctx, &git.RunOpts{ + Timeout: timeout, + Dir: repoPath(repo), + Stdout: stdout, + Stderr: stderr, + }) +} diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 09729200d52..74f01452e6c 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1085,7 +1085,7 @@ func MergePullRequest(ctx *context.APIContext) { type parseCompareInfoResult struct { headRepo *repo_model.Repository headGitRepo *git.Repository - compareInfo *git.CompareInfo + compareInfo *pull_service.CompareInfo baseRef git.RefName headRef git.RefName } @@ -1201,7 +1201,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) return nil, nil } - compareInfo, err := headGitRepo.GetCompareInfo(repo_model.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseRef.ShortName(), headRef.ShortName(), false, false) + compareInfo, err := pull_service.GetCompareInfo(ctx, baseRepo, headGitRepo, baseRef.ShortName(), headRef.ShortName(), false, false) if err != nil { ctx.APIErrorInternal(err) return nil, nil @@ -1452,7 +1452,7 @@ func GetPullRequestCommits(ctx *context.APIContext) { return } - var prInfo *git.CompareInfo + var prInfo *pull_service.CompareInfo baseGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo) if err != nil { ctx.APIErrorInternal(err) @@ -1461,9 +1461,9 @@ func GetPullRequestCommits(ctx *context.APIContext) { defer closer.Close() if pr.HasMerged { - prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitHeadRefName(), false, false) + prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), false, false) } else { - prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitHeadRefName(), false, false) + prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), false, false) } if err != nil { ctx.APIErrorInternal(err) @@ -1582,11 +1582,11 @@ func GetPullRequestFiles(ctx *context.APIContext) { baseGitRepo := ctx.Repo.GitRepo - var prInfo *git.CompareInfo + var prInfo *pull_service.CompareInfo if pr.HasMerged { - prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitHeadRefName(), true, false) + prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), true, false) } else { - prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitHeadRefName(), true, false) + prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), true, false) } if err != nil { ctx.APIErrorInternal(err) diff --git a/routers/common/compare.go b/routers/common/compare.go index 4d1cc2f0d89..fda31a07ba7 100644 --- a/routers/common/compare.go +++ b/routers/common/compare.go @@ -7,6 +7,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" + pull_service "code.gitea.io/gitea/services/pull" ) // CompareInfo represents the collected results from ParseCompareInfo @@ -14,7 +15,7 @@ type CompareInfo struct { HeadUser *user_model.User HeadRepo *repo_model.Repository HeadGitRepo *git.Repository - CompareInfo *git.CompareInfo + CompareInfo *pull_service.CompareInfo BaseBranch string HeadBranch string DirectComparison bool diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index c771b30e5ff..6980a6e0dc2 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -41,6 +41,7 @@ import ( "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/gitdiff" + pull_service "code.gitea.io/gitea/services/pull" ) const ( @@ -550,7 +551,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { headBranchRef = git.TagPrefix + ci.HeadBranch } - ci.CompareInfo, err = ci.HeadGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranchRef, headBranchRef, ci.DirectComparison, fileOnly) + ci.CompareInfo, err = pull_service.GetCompareInfo(ctx, baseRepo, ci.HeadGitRepo, baseBranchRef, headBranchRef, ci.DirectComparison, fileOnly) if err != nil { ctx.ServerError("GetCompareInfo", err) return nil diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index c5302dd50f5..5af33b40bb4 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -254,7 +254,7 @@ func GetMergedBaseCommitID(ctx *context.Context, issue *issues_model.Issue) stri return baseCommit } -func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { +func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_service.CompareInfo { if !issue.IsPull { return nil } @@ -265,7 +265,7 @@ func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *g } // prepareMergedViewPullInfo show meta information for a merged pull request view page -func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { +func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_service.CompareInfo { pull := issue.PullRequest setMergeTarget(ctx, pull) @@ -273,7 +273,7 @@ func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) baseCommit := GetMergedBaseCommitID(ctx, issue) - compareInfo, err := ctx.Repo.GitRepo.GetCompareInfo(ctx.Repo.Repository.RepoPath(), + compareInfo, err := pull_service.GetCompareInfo(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, baseCommit, pull.GetGitHeadRefName(), false, false) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "unknown revision or path not in the working tree") { @@ -311,7 +311,7 @@ func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) } // prepareViewPullInfo show meta information for a pull request preview page -func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { +func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_service.CompareInfo { ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes repo := ctx.Repo.Repository @@ -373,7 +373,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(commitStatuses) } - compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), + compareInfo, err := pull_service.GetCompareInfo(ctx, pull.BaseRepo, baseGitRepo, pull.MergeBase, pull.GetGitHeadRefName(), false, false) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") { @@ -521,7 +521,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C } } - compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), + compareInfo, err := pull_service.GetCompareInfo(ctx, pull.BaseRepo, baseGitRepo, git.BranchPrefix+pull.BaseBranch, pull.GetGitHeadRefName(), false, false) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") { diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 0865d9d7c0f..db090f02df1 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -16,6 +16,7 @@ import ( unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/indexer/code" issue_indexer "code.gitea.io/gitea/modules/indexer/issues" "code.gitea.io/gitea/modules/indexer/stats" @@ -258,7 +259,7 @@ func handleSettingsPostMirror(ctx *context.Context) { return } - u, err := git.GetRemoteURL(ctx, ctx.Repo.Repository.RepoPath(), pullMirror.GetRemoteName()) + u, err := gitrepo.GetRemoteURL(ctx, ctx.Repo.Repository, pullMirror.GetRemoteName()) if err != nil { ctx.Data["Err_MirrorAddress"] = true handleSettingRemoteAddrError(ctx, err, form) diff --git a/services/doctor/misc.go b/services/doctor/misc.go index 1269d088c31..84de1673c40 100644 --- a/services/doctor/misc.go +++ b/services/doctor/misc.go @@ -91,18 +91,13 @@ func checkEnablePushOptions(ctx context.Context, logger log.Logger, autofix bool if err := iterateRepositories(ctx, func(repo *repo_model.Repository) error { numRepos++ - r, err := gitrepo.OpenRepository(ctx, repo) - if err != nil { - return err - } - defer r.Close() if autofix { - _, _, err := git.NewCommand("config", "receive.advertisePushOptions", "true").RunStdString(ctx, &git.RunOpts{Dir: r.Path}) + _, err := gitrepo.UpdateGitConfig(ctx, repo, "receive.advertisePushOptions", "true") return err } - value, _, err := git.NewCommand("config", "receive.advertisePushOptions").RunStdString(ctx, &git.RunOpts{Dir: r.Path}) + value, err := gitrepo.GetGitConfig(ctx, repo, "receive.advertisePushOptions") if err != nil { return err } diff --git a/services/issue/pull.go b/services/issue/pull.go index 512fdf78e84..b21923d539c 100644 --- a/services/issue/pull.go +++ b/services/issue/pull.go @@ -11,6 +11,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" org_model "code.gitea.io/gitea/models/organization" + repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" @@ -18,19 +19,19 @@ import ( "code.gitea.io/gitea/modules/setting" ) -func getMergeBase(repo *git.Repository, pr *issues_model.PullRequest, baseBranch, headBranch string) (string, error) { +func getMergeBase(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, pr *issues_model.PullRequest, baseBranch, headBranch string) (string, error) { // Add a temporary remote tmpRemote := fmt.Sprintf("mergebase-%d-%d", pr.ID, time.Now().UnixNano()) - if err := repo.AddRemote(tmpRemote, repo.Path, false); err != nil { - return "", fmt.Errorf("AddRemote: %w", err) + if err := gitrepo.AddGitRemote(ctx, repo, tmpRemote, gitRepo.Path); err != nil { + return "", fmt.Errorf("AddGitRemote: %w", err) } defer func() { - if err := repo.RemoveRemote(tmpRemote); err != nil { - log.Error("getMergeBase: RemoveRemote: %v", err) + if err := gitrepo.RemoveGitRemote(ctx, repo, tmpRemote); err != nil { + log.Error("getMergeBase: RemoveGitRemote: %v", err) } }() - mergeBase, _, err := repo.GetMergeBase(tmpRemote, baseBranch, headBranch) + mergeBase, _, err := gitRepo.GetMergeBase(tmpRemote, baseBranch, headBranch) return mergeBase, err } @@ -97,7 +98,7 @@ func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullReque } // get the mergebase - mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName()) + mergeBase, err := getMergeBase(ctx, pr.BaseRepo, repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName()) if err != nil { return nil, err } diff --git a/services/migrations/dump.go b/services/migrations/dump.go index 8edd567b082..b4eb3aa690c 100644 --- a/services/migrations/dump.go +++ b/services/migrations/dump.go @@ -506,6 +506,7 @@ func (g *RepositoryDumper) handlePullRequest(ctx context.Context, pr *base.PullR // ... let's try something less nice remote = "head-pr-" + strconv.FormatInt(pr.Number, 10) } + // ... now add the remote err := g.gitRepo.AddRemote(remote, pr.Head.CloneURL, true) if err != nil { diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index cb90af5894d..47f6d9da99f 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -39,30 +39,27 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error } remoteName := m.GetRemoteName() - repoPath := m.GetRepository(ctx).RepoPath() + repo := m.GetRepository(ctx) // Remove old remote - _, _, err = git.NewCommand("remote", "rm").AddDynamicArguments(remoteName).RunStdString(ctx, &git.RunOpts{Dir: repoPath}) + err = gitrepo.RemoveGitRemote(ctx, repo, remoteName) if err != nil && !git.IsRemoteNotExistError(err) { return err } - cmd := git.NewCommand("remote", "add").AddDynamicArguments(remoteName).AddArguments("--mirror=fetch").AddDynamicArguments(addr) - _, _, err = cmd.RunStdString(ctx, &git.RunOpts{Dir: repoPath}) + err = gitrepo.AddGitRemote(ctx, repo, remoteName, addr, "--mirror=fetch") if err != nil && !git.IsRemoteNotExistError(err) { return err } if m.Repo.HasWiki() { - wikiPath := m.Repo.WikiPath() wikiRemotePath := repo_module.WikiRemoteURL(ctx, addr) // Remove old remote of wiki - _, _, err = git.NewCommand("remote", "rm").AddDynamicArguments(remoteName).RunStdString(ctx, &git.RunOpts{Dir: wikiPath}) + err = gitrepo.RemoveGitRemote(ctx, repo.WikiStorageRepo(), remoteName) if err != nil && !git.IsRemoteNotExistError(err) { return err } - cmd = git.NewCommand("remote", "add").AddDynamicArguments(remoteName).AddArguments("--mirror=fetch").AddDynamicArguments(wikiRemotePath) - _, _, err = cmd.RunStdString(ctx, &git.RunOpts{Dir: wikiPath}) + err = gitrepo.AddGitRemote(ctx, repo.WikiStorageRepo(), remoteName, wikiRemotePath, "--mirror=fetch") if err != nil && !git.IsRemoteNotExistError(err) { return err } @@ -197,25 +194,21 @@ func parseRemoteUpdateOutput(output, remoteName string) []*mirrorSyncResult { func pruneBrokenReferences(ctx context.Context, m *repo_model.Mirror, - repoPath string, timeout time.Duration, stdoutBuilder, stderrBuilder *strings.Builder, isWiki bool, ) error { wiki := "" + var storageRepo gitrepo.Repository = m.Repo if isWiki { wiki = "Wiki " + storageRepo = m.Repo.WikiStorageRepo() } stderrBuilder.Reset() stdoutBuilder.Reset() - pruneErr := git.NewCommand("remote", "prune").AddDynamicArguments(m.GetRemoteName()). - Run(ctx, &git.RunOpts{ - Timeout: timeout, - Dir: repoPath, - Stdout: stdoutBuilder, - Stderr: stderrBuilder, - }) + + pruneErr := gitrepo.PruneRemote(ctx, storageRepo, m.GetRemoteName(), timeout, stdoutBuilder, stderrBuilder) if pruneErr != nil { stdout := stdoutBuilder.String() stderr := stderrBuilder.String() @@ -226,7 +219,7 @@ func pruneBrokenReferences(ctx context.Context, stdoutMessage := util.SanitizeCredentialURLs(stdout) log.Error("Failed to prune mirror repository %s%-v references:\nStdout: %s\nStderr: %s\nErr: %v", wiki, m.Repo, stdoutMessage, stderrMessage, pruneErr) - desc := fmt.Sprintf("Failed to prune mirror repository %s'%s' references: %s", wiki, repoPath, stderrMessage) + desc := fmt.Sprintf("Failed to prune mirror repository %s'%s' references: %s", wiki, storageRepo.RelativePath(), stderrMessage) if err := system_model.CreateRepositoryNotice(desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } @@ -268,7 +261,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo } cmd.AddArguments("--tags").AddDynamicArguments(m.GetRemoteName()) - remoteURL, remoteErr := git.GetRemoteURL(ctx, repoPath, m.GetRemoteName()) + remoteURL, remoteErr := gitrepo.GetRemoteURL(ctx, m.Repo, m.GetRemoteName()) if remoteErr != nil { log.Error("SyncMirrors [repo: %-v]: GetRemoteAddress Error %v", m.Repo, remoteErr) return nil, false @@ -298,7 +291,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo err = nil // Attempt prune - pruneErr := pruneBrokenReferences(ctx, m, repoPath, timeout, &stdoutBuilder, &stderrBuilder, false) + pruneErr := pruneBrokenReferences(ctx, m, timeout, &stdoutBuilder, &stderrBuilder, false) if pruneErr == nil { // Successful prune - reattempt mirror stderrBuilder.Reset() @@ -371,13 +364,9 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo log.Trace("SyncMirrors [repo: %-v Wiki]: running git remote update...", m.Repo) stderrBuilder.Reset() stdoutBuilder.Reset() - if err := git.NewCommand("remote", "update", "--prune").AddDynamicArguments(m.GetRemoteName()). - Run(ctx, &git.RunOpts{ - Timeout: timeout, - Dir: wikiPath, - Stdout: &stdoutBuilder, - Stderr: &stderrBuilder, - }); err != nil { + + if err := gitrepo.UpdateRemotePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(), + timeout, &stdoutBuilder, &stderrBuilder); err != nil { stdout := stdoutBuilder.String() stderr := stderrBuilder.String() @@ -391,19 +380,14 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo err = nil // Attempt prune - pruneErr := pruneBrokenReferences(ctx, m, repoPath, timeout, &stdoutBuilder, &stderrBuilder, true) + pruneErr := pruneBrokenReferences(ctx, m, timeout, &stdoutBuilder, &stderrBuilder, true) if pruneErr == nil { // Successful prune - reattempt mirror stderrBuilder.Reset() stdoutBuilder.Reset() - if err = git.NewCommand("remote", "update", "--prune").AddDynamicArguments(m.GetRemoteName()). - Run(ctx, &git.RunOpts{ - Timeout: timeout, - Dir: wikiPath, - Stdout: &stdoutBuilder, - Stderr: &stderrBuilder, - }); err != nil { + if err = gitrepo.UpdateRemotePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(), + timeout, &stdoutBuilder, &stderrBuilder); err != nil { stdout := stdoutBuilder.String() stderr := stderrBuilder.String() stderrMessage = util.SanitizeCredentialURLs(stderr) diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index 9b57427d980..00bdfff8de5 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -29,28 +29,24 @@ var stripExitStatus = regexp.MustCompile(`exit status \d+ - `) // AddPushMirrorRemote registers the push mirror remote. func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr string) error { - addRemoteAndConfig := func(addr, path string) error { - cmd := git.NewCommand("remote", "add", "--mirror=push").AddDynamicArguments(m.RemoteName, addr) - if _, _, err := cmd.RunStdString(ctx, &git.RunOpts{Dir: path}); err != nil { + addRemoteAndConfig := func(storageRepo gitrepo.Repository, addr string) error { + if err := gitrepo.AddGitRemote(ctx, storageRepo, m.RemoteName, addr, "--mirror=push"); err != nil { return err } - if _, _, err := git.NewCommand("config", "--add").AddDynamicArguments("remote."+m.RemoteName+".push", "+refs/heads/*:refs/heads/*").RunStdString(ctx, &git.RunOpts{Dir: path}); err != nil { + if err := gitrepo.AddGitConfig(ctx, storageRepo, "remote."+m.RemoteName+".push", "+refs/heads/*:refs/heads/*"); err != nil { return err } - if _, _, err := git.NewCommand("config", "--add").AddDynamicArguments("remote."+m.RemoteName+".push", "+refs/tags/*:refs/tags/*").RunStdString(ctx, &git.RunOpts{Dir: path}); err != nil { - return err - } - return nil + return gitrepo.AddGitConfig(ctx, storageRepo, "remote."+m.RemoteName+".push", "+refs/tags/*:refs/tags/*") } - if err := addRemoteAndConfig(addr, m.Repo.RepoPath()); err != nil { + if err := addRemoteAndConfig(m.Repo, addr); err != nil { return err } if m.Repo.HasWiki() { wikiRemoteURL := repository.WikiRemoteURL(ctx, addr) if len(wikiRemoteURL) > 0 { - if err := addRemoteAndConfig(wikiRemoteURL, m.Repo.WikiPath()); err != nil { + if err := addRemoteAndConfig(m.Repo.WikiStorageRepo(), wikiRemoteURL); err != nil { return err } } @@ -61,15 +57,13 @@ func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr str // RemovePushMirrorRemote removes the push mirror remote. func RemovePushMirrorRemote(ctx context.Context, m *repo_model.PushMirror) error { - cmd := git.NewCommand("remote", "rm").AddDynamicArguments(m.RemoteName) _ = m.GetRepository(ctx) - - if _, _, err := cmd.RunStdString(ctx, &git.RunOpts{Dir: m.Repo.RepoPath()}); err != nil { + if err := gitrepo.RemoveGitRemote(ctx, m.Repo, m.RemoteName); err != nil { return err } if m.Repo.HasWiki() { - if _, _, err := cmd.RunStdString(ctx, &git.RunOpts{Dir: m.Repo.WikiPath()}); err != nil { + if err := gitrepo.RemoveGitRemote(ctx, m.Repo.WikiStorageRepo(), m.RemoteName); err != nil { // The wiki remote may not exist log.Warn("Wiki Remote[%d] could not be removed: %v", m.ID, err) } @@ -128,11 +122,13 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error { timeout := time.Duration(setting.Git.Timeout.Mirror) * time.Second performPush := func(repo *repo_model.Repository, isWiki bool) error { + var storageRepo gitrepo.Repository = repo path := repo.RepoPath() if isWiki { + storageRepo = repo.WikiStorageRepo() path = repo.WikiPath() } - remoteURL, err := git.GetRemoteURL(ctx, path, m.RemoteName) + remoteURL, err := gitrepo.GetRemoteURL(ctx, storageRepo, m.RemoteName) if err != nil { log.Error("GetRemoteAddress(%s) Error %v", path, err) return errors.New("Unexpected error") @@ -141,12 +137,7 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error { if setting.LFS.StartServer { log.Trace("SyncMirrors [repo: %-v]: syncing LFS objects...", m.Repo) - var gitRepo *git.Repository - if isWiki { - gitRepo, err = gitrepo.OpenRepository(ctx, repo.WikiStorageRepo()) - } else { - gitRepo, err = gitrepo.OpenRepository(ctx, repo) - } + gitRepo, err := gitrepo.OpenRepository(ctx, storageRepo) if err != nil { log.Error("OpenRepository: %v", err) return errors.New("Unexpected error") diff --git a/services/pull/compare.go b/services/pull/compare.go new file mode 100644 index 00000000000..a687ee572f2 --- /dev/null +++ b/services/pull/compare.go @@ -0,0 +1,102 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "context" + "fmt" + "strconv" + "time" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" + logger "code.gitea.io/gitea/modules/log" +) + +// CompareInfo represents needed information for comparing references. +type CompareInfo struct { + MergeBase string + BaseCommitID string + HeadCommitID string + Commits []*git.Commit + NumFiles int +} + +// GetCompareInfo generates and returns compare information between base and head branches of repositories. +func GetCompareInfo(ctx context.Context, baseRepo *repo_model.Repository, headGitRepo *git.Repository, baseBranch, headBranch string, directComparison, fileOnly bool) (_ *CompareInfo, err error) { + var ( + remoteBranch string + tmpRemote string + ) + + // We don't need a temporary remote for same repository. + if baseRepo.RepoPath() != headGitRepo.Path { + // Add a temporary remote + tmpRemote = strconv.FormatInt(time.Now().UnixNano(), 10) + if err = gitrepo.AddGitRemote(ctx, baseRepo, tmpRemote, headGitRepo.Path); err != nil { + return nil, fmt.Errorf("AddRemote: %w", err) + } + defer func() { + if err := gitrepo.RemoveGitRemote(ctx, baseRepo, tmpRemote); err != nil { + logger.Error("GetPullRequestInfo: RemoveGitRemote: %v", err) + } + }() + } + + compareInfo := new(CompareInfo) + + compareInfo.HeadCommitID, err = git.GetFullCommitID(ctx, headGitRepo.Path, headBranch) + if err != nil { + compareInfo.HeadCommitID = headBranch + } + + compareInfo.MergeBase, remoteBranch, err = headGitRepo.GetMergeBase(tmpRemote, baseBranch, headBranch) + if err == nil { + compareInfo.BaseCommitID, err = git.GetFullCommitID(ctx, headGitRepo.Path, remoteBranch) + if err != nil { + compareInfo.BaseCommitID = remoteBranch + } + separator := "..." + baseCommitID := compareInfo.MergeBase + if directComparison { + separator = ".." + baseCommitID = compareInfo.BaseCommitID + } + + // We have a common base - therefore we know that ... should work + if !fileOnly { + // avoid: ambiguous argument 'refs/a...refs/b': unknown revision or path not in the working tree. Use '--': 'git [...] -- [...]' + var logs []byte + logs, _, err = git.NewCommand("log").AddArguments(git.PrettyLogFormat). + AddDynamicArguments(baseCommitID+separator+headBranch).AddArguments("--"). + RunStdBytes(ctx, &git.RunOpts{Dir: headGitRepo.Path}) + if err != nil { + return nil, err + } + compareInfo.Commits, err = headGitRepo.ParsePrettyFormatLogToList(logs) + if err != nil { + return nil, fmt.Errorf("parsePrettyFormatLogToList: %w", err) + } + } else { + compareInfo.Commits = []*git.Commit{} + } + } else { + compareInfo.Commits = []*git.Commit{} + compareInfo.MergeBase, err = git.GetFullCommitID(ctx, headGitRepo.Path, remoteBranch) + if err != nil { + compareInfo.MergeBase = remoteBranch + } + compareInfo.BaseCommitID = compareInfo.MergeBase + } + + // Count number of changed files. + // This probably should be removed as we need to use shortstat elsewhere + // Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly + compareInfo.NumFiles, err = headGitRepo.GetDiffNumChangedFiles(remoteBranch, headBranch, directComparison) + if err != nil { + return nil, err + } + return compareInfo, nil +} diff --git a/services/pull/pull.go b/services/pull/pull.go index 2829e154410..f5de728e3d5 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -142,7 +142,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { return err } - compareInfo, err := baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), + compareInfo, err := GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false, false) if err != nil { return err @@ -1077,7 +1077,7 @@ func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]Co if pull.HasMerged { baseBranch = pull.MergeBase } - prInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), baseBranch, pull.GetGitHeadRefName(), true, false) + prInfo, err := GetCompareInfo(ctx, pull.BaseRepo, baseGitRepo, baseBranch, pull.GetGitHeadRefName(), true, false) if err != nil { return nil, "", err } diff --git a/services/repository/migrate.go b/services/repository/migrate.go index 0a3dc45339f..bc29785371e 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -261,20 +261,6 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, return repo, committer.Commit() } -// cleanUpMigrateGitConfig removes mirror info which prevents "push --all". -// This also removes possible user credentials. -func cleanUpMigrateGitConfig(ctx context.Context, repoPath string) error { - cmd := git.NewCommand("remote", "rm", "origin") - // if the origin does not exist - _, _, err := cmd.RunStdString(ctx, &git.RunOpts{ - Dir: repoPath, - }) - if err != nil && !git.IsRemoteNotExistError(err) { - return err - } - return nil -} - // CleanUpMigrateInfo finishes migrating repository and/or wiki with things that don't need to be done for mirrors. func CleanUpMigrateInfo(ctx context.Context, repo *repo_model.Repository) (*repo_model.Repository, error) { if err := gitrepo.CreateDelegateHooks(ctx, repo); err != nil { @@ -286,13 +272,14 @@ func CleanUpMigrateInfo(ctx context.Context, repo *repo_model.Repository) (*repo } } - _, _, err := git.NewCommand("remote", "rm", "origin").RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}) + err := gitrepo.RemoveGitRemote(ctx, repo, "origin") if err != nil && !git.IsRemoteNotExistError(err) { return repo, fmt.Errorf("CleanUpMigrateInfo: %w", err) } if repo.HasWiki() { - if err := cleanUpMigrateGitConfig(ctx, repo.WikiPath()); err != nil { + err = gitrepo.RemoveGitRemote(ctx, repo.WikiStorageRepo(), "origin") + if err != nil && !git.IsRemoteNotExistError(err) { return repo, fmt.Errorf("cleanUpMigrateGitConfig (wiki): %w", err) } } From 2e69ad399db3cb3b1604c95f5c49867b9d3d0991 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 23 Jul 2025 22:10:53 -0700 Subject: [PATCH 2/8] remove unnecessary change --- services/migrations/dump.go | 1 - 1 file changed, 1 deletion(-) diff --git a/services/migrations/dump.go b/services/migrations/dump.go index b4eb3aa690c..8edd567b082 100644 --- a/services/migrations/dump.go +++ b/services/migrations/dump.go @@ -506,7 +506,6 @@ func (g *RepositoryDumper) handlePullRequest(ctx context.Context, pr *base.PullR // ... let's try something less nice remote = "head-pr-" + strconv.FormatInt(pr.Number, 10) } - // ... now add the remote err := g.gitRepo.AddRemote(remote, pr.Head.CloneURL, true) if err != nil { From 2f89a0454a10c05e0100e4f1759cc2a6395cdf03 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 24 Jul 2025 16:53:19 -0700 Subject: [PATCH 3/8] Fix bug --- routers/api/v1/repo/pull.go | 10 +++++----- routers/web/repo/compare.go | 2 +- routers/web/repo/pull.go | 6 +++--- services/pull/compare.go | 8 ++++---- services/pull/pull.go | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 74f01452e6c..ff6ddbce2d4 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1201,7 +1201,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) return nil, nil } - compareInfo, err := pull_service.GetCompareInfo(ctx, baseRepo, headGitRepo, baseRef.ShortName(), headRef.ShortName(), false, false) + compareInfo, err := pull_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef.ShortName(), headRef.ShortName(), false, false) if err != nil { ctx.APIErrorInternal(err) return nil, nil @@ -1461,9 +1461,9 @@ func GetPullRequestCommits(ctx *context.APIContext) { defer closer.Close() if pr.HasMerged { - prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), false, false) + prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), false, false) } else { - prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), false, false) + prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), false, false) } if err != nil { ctx.APIErrorInternal(err) @@ -1584,9 +1584,9 @@ func GetPullRequestFiles(ctx *context.APIContext) { var prInfo *pull_service.CompareInfo if pr.HasMerged { - prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), true, false) + prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), true, false) } else { - prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), true, false) + prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), true, false) } if err != nil { ctx.APIErrorInternal(err) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 6980a6e0dc2..8f36b1dca36 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -551,7 +551,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { headBranchRef = git.TagPrefix + ci.HeadBranch } - ci.CompareInfo, err = pull_service.GetCompareInfo(ctx, baseRepo, ci.HeadGitRepo, baseBranchRef, headBranchRef, ci.DirectComparison, fileOnly) + ci.CompareInfo, err = pull_service.GetCompareInfo(ctx, baseRepo, ci.HeadRepo, ci.HeadGitRepo, baseBranchRef, headBranchRef, ci.DirectComparison, fileOnly) if err != nil { ctx.ServerError("GetCompareInfo", err) return nil diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 5af33b40bb4..1d18f0b30d8 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -273,7 +273,7 @@ func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) baseCommit := GetMergedBaseCommitID(ctx, issue) - compareInfo, err := pull_service.GetCompareInfo(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, + compareInfo, err := pull_service.GetCompareInfo(ctx, ctx.Repo.Repository, ctx.Repo.Repository, ctx.Repo.GitRepo, baseCommit, pull.GetGitHeadRefName(), false, false) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "unknown revision or path not in the working tree") { @@ -373,7 +373,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_ ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(commitStatuses) } - compareInfo, err := pull_service.GetCompareInfo(ctx, pull.BaseRepo, baseGitRepo, + compareInfo, err := pull_service.GetCompareInfo(ctx, pull.BaseRepo, pull.BaseRepo, baseGitRepo, pull.MergeBase, pull.GetGitHeadRefName(), false, false) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") { @@ -521,7 +521,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_ } } - compareInfo, err := pull_service.GetCompareInfo(ctx, pull.BaseRepo, baseGitRepo, + compareInfo, err := pull_service.GetCompareInfo(ctx, pull.BaseRepo, pull.BaseRepo, baseGitRepo, git.BranchPrefix+pull.BaseBranch, pull.GetGitHeadRefName(), false, false) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") { diff --git a/services/pull/compare.go b/services/pull/compare.go index a687ee572f2..f8b5ba20a88 100644 --- a/services/pull/compare.go +++ b/services/pull/compare.go @@ -25,21 +25,21 @@ type CompareInfo struct { } // GetCompareInfo generates and returns compare information between base and head branches of repositories. -func GetCompareInfo(ctx context.Context, baseRepo *repo_model.Repository, headGitRepo *git.Repository, baseBranch, headBranch string, directComparison, fileOnly bool) (_ *CompareInfo, err error) { +func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Repository, headGitRepo *git.Repository, baseBranch, headBranch string, directComparison, fileOnly bool) (_ *CompareInfo, err error) { var ( remoteBranch string tmpRemote string ) // We don't need a temporary remote for same repository. - if baseRepo.RepoPath() != headGitRepo.Path { + if headGitRepo.Path != baseRepo.RepoPath() { // Add a temporary remote tmpRemote = strconv.FormatInt(time.Now().UnixNano(), 10) - if err = gitrepo.AddGitRemote(ctx, baseRepo, tmpRemote, headGitRepo.Path); err != nil { + if err = gitrepo.AddGitRemote(ctx, headRepo, tmpRemote, baseRepo.RepoPath()); err != nil { return nil, fmt.Errorf("AddRemote: %w", err) } defer func() { - if err := gitrepo.RemoveGitRemote(ctx, baseRepo, tmpRemote); err != nil { + if err := gitrepo.RemoveGitRemote(ctx, headRepo, tmpRemote); err != nil { logger.Error("GetPullRequestInfo: RemoveGitRemote: %v", err) } }() diff --git a/services/pull/pull.go b/services/pull/pull.go index f5de728e3d5..4df46e4c380 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -142,7 +142,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { return err } - compareInfo, err := GetCompareInfo(ctx, pr.BaseRepo, baseGitRepo, + compareInfo, err := GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false, false) if err != nil { return err @@ -1077,7 +1077,7 @@ func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]Co if pull.HasMerged { baseBranch = pull.MergeBase } - prInfo, err := GetCompareInfo(ctx, pull.BaseRepo, baseGitRepo, baseBranch, pull.GetGitHeadRefName(), true, false) + prInfo, err := GetCompareInfo(ctx, pull.BaseRepo, pull.BaseRepo, baseGitRepo, baseBranch, pull.GetGitHeadRefName(), true, false) if err != nil { return nil, "", err } From 183cde56e09af0ee7b9c3ba83f826873ff16091c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 25 Jul 2025 10:39:51 +0800 Subject: [PATCH 4/8] fix --- modules/git/repo.go | 11 +++++++++-- modules/git/repo_commit.go | 14 +++++++------- services/pull/compare.go | 12 ++---------- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/modules/git/repo.go b/modules/git/repo.go index ddc96d036cf..ed1f8e835a4 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -31,14 +31,21 @@ type GPGSettings struct { Format string } -const PrettyLogFormat = `--pretty=format:%H` +const prettyLogFormat = `--pretty=format:%H` // GetAllCommitsCount returns count of all commits in repository func (repo *Repository) GetAllCommitsCount() (int64, error) { return AllCommitsCount(repo.Ctx, repo.Path, false) } -func (repo *Repository) ParsePrettyFormatLogToList(logs []byte) ([]*Commit, error) { +func (repo *Repository) ShowPrettyFormatLogToList(ctx context.Context, revisionRange string) ([]*Commit, error) { + // avoid: ambiguous argument 'refs/a...refs/b': unknown revision or path not in the working tree. Use '--': 'git [...] -- [...]' + logs, _, err := NewCommand("log").AddArguments(prettyLogFormat). + AddDynamicArguments(revisionRange).AddArguments("--"). + RunStdBytes(ctx, &RunOpts{Dir: repo.Path}) + if err != nil { + return nil, err + } return repo.parsePrettyFormatLogToList(logs) } diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 269538beabc..4066a1ca7ba 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -59,7 +59,7 @@ func (repo *Repository) getCommitByPathWithID(id ObjectID, relpath string) (*Com relpath = `\` + relpath } - stdout, _, runErr := NewCommand("log", "-1", PrettyLogFormat).AddDynamicArguments(id.String()).AddDashesAndList(relpath).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) + stdout, _, runErr := NewCommand("log", "-1", prettyLogFormat).AddDynamicArguments(id.String()).AddDashesAndList(relpath).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) if runErr != nil { return nil, runErr } @@ -74,7 +74,7 @@ func (repo *Repository) getCommitByPathWithID(id ObjectID, relpath string) (*Com // GetCommitByPath returns the last commit of relative path. func (repo *Repository) GetCommitByPath(relpath string) (*Commit, error) { - stdout, _, runErr := NewCommand("log", "-1", PrettyLogFormat).AddDashesAndList(relpath).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path}) + stdout, _, runErr := NewCommand("log", "-1", prettyLogFormat).AddDashesAndList(relpath).RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path}) if runErr != nil { return nil, runErr } @@ -94,7 +94,7 @@ func (repo *Repository) commitsByRangeWithTime(id ObjectID, page, pageSize int, cmd := NewCommand("log"). AddOptionFormat("--skip=%d", (page-1)*pageSize). AddOptionFormat("--max-count=%d", pageSize). - AddArguments(PrettyLogFormat). + AddArguments(prettyLogFormat). AddDynamicArguments(id.String()) if not != "" { @@ -141,7 +141,7 @@ func (repo *Repository) searchCommits(id ObjectID, opts SearchCommitsOptions) ([ } // create new git log command with limit of 100 commits - cmd := NewCommand("log", "-100", PrettyLogFormat).AddDynamicArguments(id.String()) + cmd := NewCommand("log", "-100", prettyLogFormat).AddDynamicArguments(id.String()) // pretend that all refs along with HEAD were listed on command line as // https://git-scm.com/docs/git-log#Documentation/git-log.txt---all @@ -175,7 +175,7 @@ func (repo *Repository) searchCommits(id ObjectID, opts SearchCommitsOptions) ([ // ignore anything not matching a valid sha pattern if id.Type().IsValid(v) { // create new git log command with 1 commit limit - hashCmd := NewCommand("log", "-1", PrettyLogFormat) + hashCmd := NewCommand("log", "-1", prettyLogFormat) // add previous arguments except for --grep and --all addCommonSearchArgs(hashCmd) // add keyword as @@ -410,7 +410,7 @@ func (repo *Repository) CommitsCountBetween(start, end string) (int64, error) { // commitsBefore the limit is depth, not total number of returned commits. func (repo *Repository) commitsBefore(id ObjectID, limit int) ([]*Commit, error) { - cmd := NewCommand("log", PrettyLogFormat) + cmd := NewCommand("log", prettyLogFormat) if limit > 0 { cmd.AddOptionFormat("-%d", limit) } @@ -536,7 +536,7 @@ func (repo *Repository) AddLastCommitCache(cacheKey, fullName, sha string) error // GetCommitBranchStart returns the commit where the branch diverged func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID string) (string, error) { - cmd := NewCommand("log", PrettyLogFormat) + cmd := NewCommand("log", prettyLogFormat) cmd.AddDynamicArguments(endCommitID) stdout, _, runErr := cmd.RunStdBytes(repo.Ctx, &RunOpts{ diff --git a/services/pull/compare.go b/services/pull/compare.go index f8b5ba20a88..4c782f066eb 100644 --- a/services/pull/compare.go +++ b/services/pull/compare.go @@ -67,17 +67,9 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito // We have a common base - therefore we know that ... should work if !fileOnly { - // avoid: ambiguous argument 'refs/a...refs/b': unknown revision or path not in the working tree. Use '--': 'git [...] -- [...]' - var logs []byte - logs, _, err = git.NewCommand("log").AddArguments(git.PrettyLogFormat). - AddDynamicArguments(baseCommitID+separator+headBranch).AddArguments("--"). - RunStdBytes(ctx, &git.RunOpts{Dir: headGitRepo.Path}) + compareInfo.Commits, err = headGitRepo.ShowPrettyFormatLogToList(ctx, baseCommitID+separator+headBranch) if err != nil { - return nil, err - } - compareInfo.Commits, err = headGitRepo.ParsePrettyFormatLogToList(logs) - if err != nil { - return nil, fmt.Errorf("parsePrettyFormatLogToList: %w", err) + return nil, fmt.Errorf("ShowPrettyFormatLogToList: %w", err) } } else { compareInfo.Commits = []*git.Commit{} From 70ef12612dc09df69ffbfd915bd9a8936b110672 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 25 Jul 2025 10:47:19 +0800 Subject: [PATCH 5/8] fix --- modules/gitrepo/config.go | 45 +++++++++++++++-------------- routers/web/repo/setting/setting.go | 2 +- services/doctor/misc.go | 4 +-- services/issue/pull.go | 8 ++--- services/mirror/mirror_pull.go | 16 +++++----- services/mirror/mirror_push.go | 12 ++++---- services/pull/compare.go | 6 ++-- services/repository/migrate.go | 4 +-- 8 files changed, 49 insertions(+), 48 deletions(-) diff --git a/modules/gitrepo/config.go b/modules/gitrepo/config.go index e2f35343b31..2d10e82e50c 100644 --- a/modules/gitrepo/config.go +++ b/modules/gitrepo/config.go @@ -13,7 +13,7 @@ import ( "code.gitea.io/gitea/modules/globallock" ) -func GetGitConfig(ctx context.Context, repo Repository, key string) (string, error) { +func GitConfigGet(ctx context.Context, repo Repository, key string) (string, error) { result, _, err := git.NewCommand("config", "--get"). AddDynamicArguments(key). RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)}) @@ -26,13 +26,13 @@ func GetGitConfig(ctx context.Context, repo Repository, key string) (string, err return result, nil } -func getRepoConfigLockKey(repoStoragePath string) string { +func repoGitConfigLockKey(repoStoragePath string) string { return "repo-config:" + repoStoragePath } -// AddGitConfig add a git configuration key to a specific value for the given repository. -func AddGitConfig(ctx context.Context, repo Repository, key, value string) error { - releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) +// GitConfigAdd add a git configuration key to a specific value for the given repository. +func GitConfigAdd(ctx context.Context, repo Repository, key, value string) error { + releaser, err := globallock.Lock(ctx, repoGitConfigLockKey(repo.RelativePath())) if err != nil { return err } @@ -44,24 +44,24 @@ func AddGitConfig(ctx context.Context, repo Repository, key, value string) error return err } -// UpdateGitConfig updates a git configuration key to a specific value for the given repository. +// GitConfigSet updates a git configuration key to a specific value for the given repository. // If the key does not exist, it will be created. // If the key exists, it will be updated to the new value. -func UpdateGitConfig(ctx context.Context, repo Repository, key, value string) (string, error) { - releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) +func GitConfigSet(ctx context.Context, repo Repository, key, value string) error { + releaser, err := globallock.Lock(ctx, repoGitConfigLockKey(repo.RelativePath())) if err != nil { - return "", err + return err } defer releaser() - value, _, err1 := git.NewCommand("config"). + _, _, err = git.NewCommand("config"). AddDynamicArguments(key, value). RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)}) - return value, err1 + return err } -func AddGitRemote(ctx context.Context, repo Repository, remoteName, remoteURL string, options ...string) error { - releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) +func GitRemoteAdd(ctx context.Context, repo Repository, remoteName, remoteURL string, options ...string) error { + releaser, err := globallock.Lock(ctx, repoGitConfigLockKey(repo.RelativePath())) if err != nil { return err } @@ -77,8 +77,8 @@ func AddGitRemote(ctx context.Context, repo Repository, remoteName, remoteURL st return err } -func RemoveGitRemote(ctx context.Context, repo Repository, remoteName string) error { - releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) +func GitRemoteRemove(ctx context.Context, repo Repository, remoteName string) error { + releaser, err := globallock.Lock(ctx, repoGitConfigLockKey(repo.RelativePath())) if err != nil { return err } @@ -89,8 +89,8 @@ func RemoveGitRemote(ctx context.Context, repo Repository, remoteName string) er return err } -// GetRemoteURL returns the url of a specific remote of the repository. -func GetRemoteURL(ctx context.Context, repo Repository, remoteName string) (*giturl.GitURL, error) { +// GitRemoteGetURL returns the url of a specific remote of the repository. +func GitRemoteGetURL(ctx context.Context, repo Repository, remoteName string) (*giturl.GitURL, error) { addr, err := git.GetRemoteAddress(ctx, repoPath(repo), remoteName) if err != nil { return nil, err @@ -98,9 +98,9 @@ func GetRemoteURL(ctx context.Context, repo Repository, remoteName string) (*git return giturl.ParseGitURL(addr) } -// PruneRemote prunes the remote branches that no longer exist in the remote repository. -func PruneRemote(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error { - releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) +// FIXME: config related? long-time running? +func GitRemotePrune(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error { + releaser, err := globallock.Lock(ctx, repoGitConfigLockKey(repo.RelativePath())) if err != nil { return err } @@ -115,8 +115,9 @@ func PruneRemote(ctx context.Context, repo Repository, remoteName string, timeou }) } -func UpdateRemotePrune(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error { - releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) +// FIXME: config related? long-time running? +func GitRemoteUpdatePrune(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error { + releaser, err := globallock.Lock(ctx, repoGitConfigLockKey(repo.RelativePath())) if err != nil { return err } diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index db090f02df1..1a4f590e106 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -259,7 +259,7 @@ func handleSettingsPostMirror(ctx *context.Context) { return } - u, err := gitrepo.GetRemoteURL(ctx, ctx.Repo.Repository, pullMirror.GetRemoteName()) + u, err := gitrepo.GitRemoteGetURL(ctx, ctx.Repo.Repository, pullMirror.GetRemoteName()) if err != nil { ctx.Data["Err_MirrorAddress"] = true handleSettingRemoteAddrError(ctx, err, form) diff --git a/services/doctor/misc.go b/services/doctor/misc.go index 84de1673c40..dcdef03738d 100644 --- a/services/doctor/misc.go +++ b/services/doctor/misc.go @@ -93,11 +93,11 @@ func checkEnablePushOptions(ctx context.Context, logger log.Logger, autofix bool numRepos++ if autofix { - _, err := gitrepo.UpdateGitConfig(ctx, repo, "receive.advertisePushOptions", "true") + err := gitrepo.GitConfigSet(ctx, repo, "receive.advertisePushOptions", "true") return err } - value, err := gitrepo.GetGitConfig(ctx, repo, "receive.advertisePushOptions") + value, err := gitrepo.GitConfigGet(ctx, repo, "receive.advertisePushOptions") if err != nil { return err } diff --git a/services/issue/pull.go b/services/issue/pull.go index b21923d539c..0a4b4328d2c 100644 --- a/services/issue/pull.go +++ b/services/issue/pull.go @@ -22,12 +22,12 @@ import ( func getMergeBase(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, pr *issues_model.PullRequest, baseBranch, headBranch string) (string, error) { // Add a temporary remote tmpRemote := fmt.Sprintf("mergebase-%d-%d", pr.ID, time.Now().UnixNano()) - if err := gitrepo.AddGitRemote(ctx, repo, tmpRemote, gitRepo.Path); err != nil { - return "", fmt.Errorf("AddGitRemote: %w", err) + if err := gitrepo.GitRemoteAdd(ctx, repo, tmpRemote, gitRepo.Path); err != nil { + return "", fmt.Errorf("GitRemoteAdd: %w", err) } defer func() { - if err := gitrepo.RemoveGitRemote(ctx, repo, tmpRemote); err != nil { - log.Error("getMergeBase: RemoveGitRemote: %v", err) + if err := gitrepo.GitRemoteRemove(ctx, repo, tmpRemote); err != nil { + log.Error("getMergeBase: GitRemoteRemove: %v", err) } }() diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 47f6d9da99f..f9e47444764 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -41,12 +41,12 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error remoteName := m.GetRemoteName() repo := m.GetRepository(ctx) // Remove old remote - err = gitrepo.RemoveGitRemote(ctx, repo, remoteName) + err = gitrepo.GitRemoteRemove(ctx, repo, remoteName) if err != nil && !git.IsRemoteNotExistError(err) { return err } - err = gitrepo.AddGitRemote(ctx, repo, remoteName, addr, "--mirror=fetch") + err = gitrepo.GitRemoteAdd(ctx, repo, remoteName, addr, "--mirror=fetch") if err != nil && !git.IsRemoteNotExistError(err) { return err } @@ -54,12 +54,12 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error if m.Repo.HasWiki() { wikiRemotePath := repo_module.WikiRemoteURL(ctx, addr) // Remove old remote of wiki - err = gitrepo.RemoveGitRemote(ctx, repo.WikiStorageRepo(), remoteName) + err = gitrepo.GitRemoteRemove(ctx, repo.WikiStorageRepo(), remoteName) if err != nil && !git.IsRemoteNotExistError(err) { return err } - err = gitrepo.AddGitRemote(ctx, repo.WikiStorageRepo(), remoteName, wikiRemotePath, "--mirror=fetch") + err = gitrepo.GitRemoteAdd(ctx, repo.WikiStorageRepo(), remoteName, wikiRemotePath, "--mirror=fetch") if err != nil && !git.IsRemoteNotExistError(err) { return err } @@ -208,7 +208,7 @@ func pruneBrokenReferences(ctx context.Context, stderrBuilder.Reset() stdoutBuilder.Reset() - pruneErr := gitrepo.PruneRemote(ctx, storageRepo, m.GetRemoteName(), timeout, stdoutBuilder, stderrBuilder) + pruneErr := gitrepo.GitRemotePrune(ctx, storageRepo, m.GetRemoteName(), timeout, stdoutBuilder, stderrBuilder) if pruneErr != nil { stdout := stdoutBuilder.String() stderr := stderrBuilder.String() @@ -261,7 +261,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo } cmd.AddArguments("--tags").AddDynamicArguments(m.GetRemoteName()) - remoteURL, remoteErr := gitrepo.GetRemoteURL(ctx, m.Repo, m.GetRemoteName()) + remoteURL, remoteErr := gitrepo.GitRemoteGetURL(ctx, m.Repo, m.GetRemoteName()) if remoteErr != nil { log.Error("SyncMirrors [repo: %-v]: GetRemoteAddress Error %v", m.Repo, remoteErr) return nil, false @@ -365,7 +365,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stderrBuilder.Reset() stdoutBuilder.Reset() - if err := gitrepo.UpdateRemotePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(), + if err := gitrepo.GitRemoteUpdatePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(), timeout, &stdoutBuilder, &stderrBuilder); err != nil { stdout := stdoutBuilder.String() stderr := stderrBuilder.String() @@ -386,7 +386,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stderrBuilder.Reset() stdoutBuilder.Reset() - if err = gitrepo.UpdateRemotePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(), + if err = gitrepo.GitRemoteUpdatePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(), timeout, &stdoutBuilder, &stderrBuilder); err != nil { stdout := stdoutBuilder.String() stderr := stderrBuilder.String() diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index 00bdfff8de5..98159811ee2 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -30,13 +30,13 @@ var stripExitStatus = regexp.MustCompile(`exit status \d+ - `) // AddPushMirrorRemote registers the push mirror remote. func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr string) error { addRemoteAndConfig := func(storageRepo gitrepo.Repository, addr string) error { - if err := gitrepo.AddGitRemote(ctx, storageRepo, m.RemoteName, addr, "--mirror=push"); err != nil { + if err := gitrepo.GitRemoteAdd(ctx, storageRepo, m.RemoteName, addr, "--mirror=push"); err != nil { return err } - if err := gitrepo.AddGitConfig(ctx, storageRepo, "remote."+m.RemoteName+".push", "+refs/heads/*:refs/heads/*"); err != nil { + if err := gitrepo.GitConfigAdd(ctx, storageRepo, "remote."+m.RemoteName+".push", "+refs/heads/*:refs/heads/*"); err != nil { return err } - return gitrepo.AddGitConfig(ctx, storageRepo, "remote."+m.RemoteName+".push", "+refs/tags/*:refs/tags/*") + return gitrepo.GitConfigAdd(ctx, storageRepo, "remote."+m.RemoteName+".push", "+refs/tags/*:refs/tags/*") } if err := addRemoteAndConfig(m.Repo, addr); err != nil { @@ -58,12 +58,12 @@ func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr str // RemovePushMirrorRemote removes the push mirror remote. func RemovePushMirrorRemote(ctx context.Context, m *repo_model.PushMirror) error { _ = m.GetRepository(ctx) - if err := gitrepo.RemoveGitRemote(ctx, m.Repo, m.RemoteName); err != nil { + if err := gitrepo.GitRemoteRemove(ctx, m.Repo, m.RemoteName); err != nil { return err } if m.Repo.HasWiki() { - if err := gitrepo.RemoveGitRemote(ctx, m.Repo.WikiStorageRepo(), m.RemoteName); err != nil { + if err := gitrepo.GitRemoteRemove(ctx, m.Repo.WikiStorageRepo(), m.RemoteName); err != nil { // The wiki remote may not exist log.Warn("Wiki Remote[%d] could not be removed: %v", m.ID, err) } @@ -128,7 +128,7 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error { storageRepo = repo.WikiStorageRepo() path = repo.WikiPath() } - remoteURL, err := gitrepo.GetRemoteURL(ctx, storageRepo, m.RemoteName) + remoteURL, err := gitrepo.GitRemoteGetURL(ctx, storageRepo, m.RemoteName) if err != nil { log.Error("GetRemoteAddress(%s) Error %v", path, err) return errors.New("Unexpected error") diff --git a/services/pull/compare.go b/services/pull/compare.go index 4c782f066eb..152676597d7 100644 --- a/services/pull/compare.go +++ b/services/pull/compare.go @@ -35,12 +35,12 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito if headGitRepo.Path != baseRepo.RepoPath() { // Add a temporary remote tmpRemote = strconv.FormatInt(time.Now().UnixNano(), 10) - if err = gitrepo.AddGitRemote(ctx, headRepo, tmpRemote, baseRepo.RepoPath()); err != nil { + if err = gitrepo.GitRemoteAdd(ctx, headRepo, tmpRemote, baseRepo.RepoPath()); err != nil { return nil, fmt.Errorf("AddRemote: %w", err) } defer func() { - if err := gitrepo.RemoveGitRemote(ctx, headRepo, tmpRemote); err != nil { - logger.Error("GetPullRequestInfo: RemoveGitRemote: %v", err) + if err := gitrepo.GitRemoteRemove(ctx, headRepo, tmpRemote); err != nil { + logger.Error("GetPullRequestInfo: GitRemoteRemove: %v", err) } }() } diff --git a/services/repository/migrate.go b/services/repository/migrate.go index bc29785371e..91e3072df67 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -272,13 +272,13 @@ func CleanUpMigrateInfo(ctx context.Context, repo *repo_model.Repository) (*repo } } - err := gitrepo.RemoveGitRemote(ctx, repo, "origin") + err := gitrepo.GitRemoteRemove(ctx, repo, "origin") if err != nil && !git.IsRemoteNotExistError(err) { return repo, fmt.Errorf("CleanUpMigrateInfo: %w", err) } if repo.HasWiki() { - err = gitrepo.RemoveGitRemote(ctx, repo.WikiStorageRepo(), "origin") + err = gitrepo.GitRemoteRemove(ctx, repo.WikiStorageRepo(), "origin") if err != nil && !git.IsRemoteNotExistError(err) { return repo, fmt.Errorf("cleanUpMigrateGitConfig (wiki): %w", err) } From 98bc0df9b87c90bb7733cc51bd7aa10b645a14df Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 25 Jul 2025 11:07:37 -0700 Subject: [PATCH 6/8] some improvements --- modules/gitrepo/config.go | 43 +++++++++++++++++------ modules/templates/util_misc.go | 11 +++--- services/mirror/mirror_pull.go | 64 +++++++++++++++++++++++++++++++++- services/mirror/mirror_push.go | 6 ++-- 4 files changed, 103 insertions(+), 21 deletions(-) diff --git a/modules/gitrepo/config.go b/modules/gitrepo/config.go index e2f35343b31..d19dcfea212 100644 --- a/modules/gitrepo/config.go +++ b/modules/gitrepo/config.go @@ -5,6 +5,7 @@ package gitrepo import ( "context" + "errors" "io" "time" @@ -60,7 +61,14 @@ func UpdateGitConfig(ctx context.Context, repo Repository, key, value string) (s return value, err1 } -func AddGitRemote(ctx context.Context, repo Repository, remoteName, remoteURL string, options ...string) error { +type RemoteOption string + +const ( + RemoteOptionMirrorPush RemoteOption = "--mirror=push" + RemoteOptionMirrorFetch RemoteOption = "--mirror=fetch" +) + +func AddGitRemote(ctx context.Context, repo Repository, remoteName, remoteURL string, options ...RemoteOption) error { releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) if err != nil { return err @@ -69,7 +77,14 @@ func AddGitRemote(ctx context.Context, repo Repository, remoteName, remoteURL st cmd := git.NewCommand("remote", "add") if len(options) > 0 { - cmd.AddDynamicArguments(options...) + switch options[0] { + case RemoteOptionMirrorPush: + cmd.AddArguments("--mirror=push") + case RemoteOptionMirrorFetch: + cmd.AddArguments("--mirror=fetch") + default: + return errors.New("unknown remote option: " + string(options[0])) + } } _, _, err = cmd. AddDynamicArguments(remoteName, remoteURL). @@ -95,17 +110,28 @@ func GetRemoteURL(ctx context.Context, repo Repository, remoteName string) (*git if err != nil { return nil, err } + if addr == "" { + return nil, nil + } return giturl.ParseGitURL(addr) } -// PruneRemote prunes the remote branches that no longer exist in the remote repository. -func PruneRemote(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error { +func SetRemoteURL(ctx context.Context, repo Repository, remoteName, remoteURL string) error { releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) if err != nil { return err } defer releaser() + cmd := git.NewCommand("remote", "set-url").AddDynamicArguments(remoteName, remoteURL) + _, _, err = cmd.RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)}) + return err +} + +// PruneRemote prunes the remote branches that no longer exist in the remote repository. +// No lock is needed because the remote remoteName will be checked before invoking this function. +// Then it will not update the remote automatically if the remote does not exist. +func PruneRemote(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error { return git.NewCommand("remote", "prune").AddDynamicArguments(remoteName). Run(ctx, &git.RunOpts{ Timeout: timeout, @@ -115,13 +141,10 @@ func PruneRemote(ctx context.Context, repo Repository, remoteName string, timeou }) } +// UpdateRemotePrune updates the remote branches and prunes the ones that no longer exist in the remote repository. +// No lock is needed because the remote remoteName will be checked before invoking this function. +// Then it will not update the remote automatically if the remote does not exist. func UpdateRemotePrune(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error { - releaser, err := globallock.Lock(ctx, getRepoConfigLockKey(repo.RelativePath())) - if err != nil { - return err - } - defer releaser() - return git.NewCommand("remote", "update", "--prune").AddDynamicArguments(remoteName). Run(ctx, &git.RunOpts{ Timeout: timeout, diff --git a/modules/templates/util_misc.go b/modules/templates/util_misc.go index cc5bf67b42b..8e01cc109c1 100644 --- a/modules/templates/util_misc.go +++ b/modules/templates/util_misc.go @@ -14,8 +14,7 @@ import ( activities_model "code.gitea.io/gitea/models/activities" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/modules/git" - giturl "code.gitea.io/gitea/modules/git/url" + "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/repository" @@ -145,15 +144,13 @@ type remoteAddress struct { func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string) remoteAddress { ret := remoteAddress{} - remoteURL, err := git.GetRemoteAddress(ctx, m.RepoPath(), remoteName) + u, err := gitrepo.GetRemoteURL(ctx, m, remoteName) if err != nil { log.Error("GetRemoteURL %v", err) return ret } - - u, err := giturl.ParseGitURL(remoteURL) - if err != nil { - log.Error("giturl.Parse %v", err) + if u == nil { + log.Error("GetRemoteURL %s returned nil", remoteName) return ret } diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 47f6d9da99f..026d4d7b19b 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -208,6 +208,24 @@ func pruneBrokenReferences(ctx context.Context, stderrBuilder.Reset() stdoutBuilder.Reset() + // check whether the remote still exists before pruning to avoid prune creating a new remote + // this is needed because prune will not fail if the remote does not exist + u, err := gitrepo.GetRemoteURL(ctx, storageRepo, m.GetRemoteName()) + if err != nil { + return err + } + if u == nil { + return fmt.Errorf("remote %s does not exist for %srepository %s", m.GetRemoteName(), wiki, storageRepo.RelativePath()) + } + + fetchConfig, err := gitrepo.GetGitConfig(ctx, storageRepo, "remote.origin.fetch") + if err != nil { + return err + } + if fetchConfig == "" { + return fmt.Errorf("remote %s has no fetch config for %srepository %s", m.GetRemoteName(), wiki, storageRepo.RelativePath()) + } + pruneErr := gitrepo.PruneRemote(ctx, storageRepo, m.GetRemoteName(), timeout, stdoutBuilder, stderrBuilder) if pruneErr != nil { stdout := stdoutBuilder.String() @@ -263,7 +281,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo remoteURL, remoteErr := gitrepo.GetRemoteURL(ctx, m.Repo, m.GetRemoteName()) if remoteErr != nil { - log.Error("SyncMirrors [repo: %-v]: GetRemoteAddress Error %v", m.Repo, remoteErr) + log.Error("SyncMirrors [repo: %-v]: GetRemoteURL Error %v", m.Repo, remoteErr) return nil, false } @@ -365,6 +383,28 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stderrBuilder.Reset() stdoutBuilder.Reset() + // check whether the remote still exists before pruning to avoid prune creating a new remote + // this is needed because prune will not fail if the remote does not exist + u, err := gitrepo.GetRemoteURL(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName()) + if err != nil { + log.Error("SyncMirrors [repo: %-v Wiki]: GetRemoteURL Error %v", m.Repo, err) + return nil, false + } + if u == nil { + log.Error("remote %s does not exist for repository %s", m.GetRemoteName(), m.Repo.WikiStorageRepo().RelativePath()) + return nil, false + } + + fetchConfig, err := gitrepo.GetGitConfig(ctx, m.Repo.WikiStorageRepo(), "remote.origin.fetch") + if err != nil { + log.Error("SyncMirrors [repo: %-v Wiki]: GetGitConfig Error %v", m.Repo, err) + return nil, false + } + if fetchConfig == "" { + log.Error("remote %s has no fetch config for repository %s", m.GetRemoteName(), m.Repo.WikiStorageRepo().RelativePath()) + return nil, false + } + if err := gitrepo.UpdateRemotePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(), timeout, &stdoutBuilder, &stderrBuilder); err != nil { stdout := stdoutBuilder.String() @@ -386,6 +426,28 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stderrBuilder.Reset() stdoutBuilder.Reset() + // check whether the remote still exists before pruning to avoid prune creating a new remote + // this is needed because prune will not fail if the remote does not exist + u, err := gitrepo.GetRemoteURL(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName()) + if err != nil { + log.Error("SyncMirrors [repo: %-v Wiki]: GetRemoteURL Error %v", m.Repo, err) + return nil, false + } + if u == nil { + log.Error("remote %s does not exist for repository %s", m.GetRemoteName(), m.Repo.WikiStorageRepo().RelativePath()) + return nil, false + } + + fetchConfig, err := gitrepo.GetGitConfig(ctx, m.Repo.WikiStorageRepo(), "remote.origin.fetch") + if err != nil { + log.Error("SyncMirrors [repo: %-v Wiki]: GetGitConfig Error %v", m.Repo, err) + return nil, false + } + if fetchConfig == "" { + log.Error("remote %s has no fetch config for repository %s", m.GetRemoteName(), m.Repo.WikiStorageRepo().RelativePath()) + return nil, false + } + if err = gitrepo.UpdateRemotePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(), timeout, &stdoutBuilder, &stderrBuilder); err != nil { stdout := stdoutBuilder.String() diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index 00bdfff8de5..bb0ea5bc7a1 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -130,7 +130,7 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error { } remoteURL, err := gitrepo.GetRemoteURL(ctx, storageRepo, m.RemoteName) if err != nil { - log.Error("GetRemoteAddress(%s) Error %v", path, err) + log.Error("GetRemoteURL(%s) Error %v", path, err) return errors.New("Unexpected error") } @@ -175,8 +175,8 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error { } if m.Repo.HasWiki() { - _, err := git.GetRemoteAddress(ctx, m.Repo.WikiPath(), m.RemoteName) - if err == nil { + u, err := gitrepo.GetRemoteURL(ctx, m.Repo.WikiStorageRepo(), m.RemoteName) + if err == nil && u != nil { err := performPush(m.Repo, true) if err != nil { return err From 7a8fdc2963f2914f4177445f2296db73442f1ca2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 25 Jul 2025 11:24:49 -0700 Subject: [PATCH 7/8] improvements --- routers/web/repo/setting/setting.go | 5 +++++ services/mirror/mirror_pull.go | 22 ---------------------- 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 1a4f590e106..3ce719f8e1a 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -265,6 +265,11 @@ func handleSettingsPostMirror(ctx *context.Context) { handleSettingRemoteAddrError(ctx, err, form) return } + if u == nil { + ctx.Data["Err_MirrorAddress"] = true + handleSettingRemoteAddrError(ctx, err, form) + return + } if u.User != nil && form.MirrorPassword == "" && form.MirrorUsername == u.User.Username() { form.MirrorPassword, _ = u.User.Password() } diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 8bd9a68200e..334b1965153 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -408,28 +408,6 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo stderrBuilder.Reset() stdoutBuilder.Reset() - // check whether the remote still exists before pruning to avoid prune creating a new remote - // this is needed because prune will not fail if the remote does not exist - u, err := gitrepo.GitRemoteGetURL(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName()) - if err != nil { - log.Error("SyncMirrors [repo: %-v Wiki]: GetRemoteURL Error %v", m.Repo, err) - return nil, false - } - if u == nil { - log.Error("remote %s does not exist for repository %s", m.GetRemoteName(), m.Repo.WikiStorageRepo().RelativePath()) - return nil, false - } - - fetchConfig, err := gitrepo.GitConfigGet(ctx, m.Repo.WikiStorageRepo(), "remote.origin.fetch") - if err != nil { - log.Error("SyncMirrors [repo: %-v Wiki]: GetGitConfig Error %v", m.Repo, err) - return nil, false - } - if fetchConfig == "" { - log.Error("remote %s has no fetch config for repository %s", m.GetRemoteName(), m.Repo.WikiStorageRepo().RelativePath()) - return nil, false - } - if err = gitrepo.GitRemoteUpdatePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(), timeout, &stdoutBuilder, &stderrBuilder); err != nil { stdout := stdoutBuilder.String() From 869f3aedd3913caf3a7b619eba4b5ad58000082c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 25 Jul 2025 11:27:38 -0700 Subject: [PATCH 8/8] improvements --- services/doctor/misc.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/doctor/misc.go b/services/doctor/misc.go index dcdef03738d..ce7eea1dcc8 100644 --- a/services/doctor/misc.go +++ b/services/doctor/misc.go @@ -93,8 +93,7 @@ func checkEnablePushOptions(ctx context.Context, logger log.Logger, autofix bool numRepos++ if autofix { - err := gitrepo.GitConfigSet(ctx, repo, "receive.advertisePushOptions", "true") - return err + return gitrepo.GitConfigSet(ctx, repo, "receive.advertisePushOptions", "true") } value, err := gitrepo.GitConfigGet(ctx, repo, "receive.advertisePushOptions")