Move git config/remote to gitrepo package and add global lock to resolve possible conflict when updating repository git config file (#35151)

Partially fix #32018 

`git config` and `git remote` write operations create a temporary file
named `config.lock`. Since these operations are not atomic, they must
not be run in parallel. If two requests attempt to modify the same
repository concurrently—such as during a compare operation—one may fail
due to the presence of an existing `config.lock` file.

In cases where `config.lock` is left behind due to an unexpected program
exit, a global lock mechanism could allow us to safely remove the stale
lock file when a related error is detected. While this behavior is not
yet implemented in this PR, it is planned for a future enhancement.

---------

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Lunny Xiao
2025-09-01 11:47:04 -07:00
committed by GitHub
parent 4e1b8db1fc
commit d2e994db2c
19 changed files with 312 additions and 225 deletions

View File

@@ -91,18 +91,12 @@ 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})
return err
return gitrepo.GitConfigSet(ctx, repo, "receive.advertisePushOptions", "true")
}
value, _, err := git.NewCommand("config", "receive.advertisePushOptions").RunStdString(ctx, &git.RunOpts{Dir: r.Path})
value, err := gitrepo.GitConfigGet(ctx, repo, "receive.advertisePushOptions")
if err != nil {
return err
}

View File

@@ -11,26 +11,28 @@ 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"
"code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/log"
"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.GitRemoteAdd(ctx, repo, tmpRemote, gitRepo.Path); err != nil {
return "", fmt.Errorf("GitRemoteAdd: %w", err)
}
defer func() {
if err := repo.RemoveRemote(tmpRemote); err != nil {
log.Error("getMergeBase: RemoveRemote: %v", err)
if err := gitrepo.GitRemoteRemove(graceful.GetManager().ShutdownContext(), repo, tmpRemote); err != nil {
log.Error("getMergeBase: GitRemoteRemove: %v", err)
}
}()
mergeBase, _, err := repo.GetMergeBase(tmpRemote, baseBranch, headBranch)
mergeBase, _, err := gitRepo.GetMergeBase(tmpRemote, baseBranch, headBranch)
return mergeBase, err
}
@@ -97,7 +99,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
}

View File

@@ -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.GitRemoteRemove(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.GitRemoteAdd(ctx, repo, remoteName, addr, gitrepo.RemoteOptionMirrorFetch)
if err != nil && !git.IsRemoteNotExistError(err) {
return err
}
if repo_service.HasWiki(ctx, m.Repo) {
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.GitRemoteRemove(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.GitRemoteAdd(ctx, repo.WikiStorageRepo(), remoteName, wikiRemotePath, gitrepo.RemoteOptionMirrorFetch)
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.GitRemotePrune(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,9 +261,9 @@ 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.GitRemoteGetURL(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
}
@@ -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.GitRemoteUpdatePrune(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.GitRemoteUpdatePrune(ctx, m.Repo.WikiStorageRepo(), m.GetRemoteName(),
timeout, &stdoutBuilder, &stderrBuilder); err != nil {
stdout := stdoutBuilder.String()
stderr := stderrBuilder.String()
stderrMessage = util.SanitizeCredentialURLs(stderr)

View File

@@ -30,28 +30,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.GitRemoteAdd(ctx, storageRepo, m.RemoteName, addr, gitrepo.RemoteOptionMirrorPush); 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.GitConfigAdd(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.GitConfigAdd(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 repo_service.HasWiki(ctx, m.Repo) {
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
}
}
@@ -62,15 +58,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.GitRemoteRemove(ctx, m.Repo, m.RemoteName); err != nil {
return err
}
if repo_service.HasWiki(ctx, m.Repo) {
if _, _, err := cmd.RunStdString(ctx, &git.RunOpts{Dir: m.Repo.WikiPath()}); 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)
}
@@ -129,25 +123,22 @@ 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.GitRemoteGetURL(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")
}
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")
@@ -185,14 +176,13 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error {
}
if repo_service.HasWiki(ctx, m.Repo) {
_, err := git.GetRemoteAddress(ctx, m.Repo.WikiPath(), m.RemoteName)
if err == nil {
if _, err := gitrepo.GitRemoteGetURL(ctx, m.Repo.WikiStorageRepo(), m.RemoteName); err == nil {
err := performPush(m.Repo, true)
if err != nil {
return err
}
} else {
log.Trace("Skipping wiki: No remote configured")
} else if !errors.Is(err, util.ErrNotExist) {
log.Error("GetRemote of wiki failed: %v", err)
}
}

95
services/pull/compare.go Normal file
View File

@@ -0,0 +1,95 @@
// 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"
"code.gitea.io/gitea/modules/graceful"
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, 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 headGitRepo.Path != baseRepo.RepoPath() {
// Add a temporary remote
tmpRemote = strconv.FormatInt(time.Now().UnixNano(), 10)
if err = gitrepo.GitRemoteAdd(ctx, headRepo, tmpRemote, baseRepo.RepoPath()); err != nil {
return nil, fmt.Errorf("GitRemoteAdd: %w", err)
}
defer func() {
if err := gitrepo.GitRemoteRemove(graceful.GetManager().ShutdownContext(), headRepo, tmpRemote); err != nil {
logger.Error("GetPullRequestInfo: GitRemoteRemove: %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 {
compareInfo.Commits, err = headGitRepo.ShowPrettyFormatLogToList(ctx, baseCommitID+separator+headBranch)
if err != nil {
return nil, fmt.Errorf("ShowPrettyFormatLogToList: %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
}

View File

@@ -141,7 +141,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
return err
}
compareInfo, err := baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(),
compareInfo, err := GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo,
git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false, false)
if err != nil {
return err
@@ -1074,7 +1074,7 @@ func GetPullCommits(ctx context.Context, baseGitRepo *git.Repository, doer *user
if pull.HasMerged {
baseBranch = pull.MergeBase
}
prInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), 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
}

View File

@@ -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 {
@@ -288,13 +274,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.GitRemoteRemove(ctx, repo, "origin")
if err != nil && !git.IsRemoteNotExistError(err) {
return repo, fmt.Errorf("CleanUpMigrateInfo: %w", err)
}
if hasWiki {
if err := cleanUpMigrateGitConfig(ctx, repo.WikiPath()); err != nil {
err = gitrepo.GitRemoteRemove(ctx, repo.WikiStorageRepo(), "origin")
if err != nil && !git.IsRemoteNotExistError(err) {
return repo, fmt.Errorf("cleanUpMigrateGitConfig (wiki): %w", err)
}
}