Skip to content

Commit d2e994d

Browse files
lunnywxiaoguang
andauthored
Move git config/remote to gitrepo package and add global lock to resolve possible conflict when updating repository git config file (go-gitea#35151)
Partially fix go-gitea#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 <[email protected]> Co-authored-by: wxiaoguang <[email protected]>
1 parent 4e1b8db commit d2e994d

File tree

19 files changed

+312
-225
lines changed

19 files changed

+312
-225
lines changed

modules/git/remote.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"net/url"
1010
"strings"
1111

12-
giturl "code.gitea.io/gitea/modules/git/url"
1312
"code.gitea.io/gitea/modules/util"
1413
)
1514

@@ -33,15 +32,6 @@ func GetRemoteAddress(ctx context.Context, repoPath, remoteName string) (string,
3332
return result, nil
3433
}
3534

36-
// GetRemoteURL returns the url of a specific remote of the repository.
37-
func GetRemoteURL(ctx context.Context, repoPath, remoteName string) (*giturl.GitURL, error) {
38-
addr, err := GetRemoteAddress(ctx, repoPath, remoteName)
39-
if err != nil {
40-
return nil, err
41-
}
42-
return giturl.ParseGitURL(addr)
43-
}
44-
4535
// ErrInvalidCloneAddr represents a "InvalidCloneAddr" kind of error.
4636
type ErrInvalidCloneAddr struct {
4737
Host string

modules/git/repo.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,17 @@ func (repo *Repository) GetAllCommitsCount() (int64, error) {
3838
return AllCommitsCount(repo.Ctx, repo.Path, false)
3939
}
4040

41+
func (repo *Repository) ShowPrettyFormatLogToList(ctx context.Context, revisionRange string) ([]*Commit, error) {
42+
// avoid: ambiguous argument 'refs/a...refs/b': unknown revision or path not in the working tree. Use '--': 'git <command> [<revision>...] -- [<file>...]'
43+
logs, _, err := NewCommand("log").AddArguments(prettyLogFormat).
44+
AddDynamicArguments(revisionRange).AddArguments("--").
45+
RunStdBytes(ctx, &RunOpts{Dir: repo.Path})
46+
if err != nil {
47+
return nil, err
48+
}
49+
return repo.parsePrettyFormatLogToList(logs)
50+
}
51+
4152
func (repo *Repository) parsePrettyFormatLogToList(logs []byte) ([]*Commit, error) {
4253
var commits []*Commit
4354
if len(logs) == 0 {

modules/git/repo_branch.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,6 @@ func (repo *Repository) AddRemote(name, url string, fetch bool) error {
7979
return err
8080
}
8181

82-
// RemoveRemote removes a remote from repository.
83-
func (repo *Repository) RemoveRemote(name string) error {
84-
_, _, err := NewCommand("remote", "rm").AddDynamicArguments(name).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path})
85-
return err
86-
}
87-
8882
// RenameBranch rename a branch
8983
func (repo *Repository) RenameBranch(from, to string) error {
9084
_, _, err := NewCommand("branch", "-m").AddDynamicArguments(from, to).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path})

modules/git/repo_compare.go

Lines changed: 0 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,8 @@ import (
1616
"regexp"
1717
"strconv"
1818
"strings"
19-
"time"
20-
21-
logger "code.gitea.io/gitea/modules/log"
2219
)
2320

24-
// CompareInfo represents needed information for comparing references.
25-
type CompareInfo struct {
26-
MergeBase string
27-
BaseCommitID string
28-
HeadCommitID string
29-
Commits []*Commit
30-
NumFiles int
31-
}
32-
3321
// GetMergeBase checks and returns merge base of two branches and the reference used as base.
3422
func (repo *Repository) GetMergeBase(tmpRemote, base, head string) (string, string, error) {
3523
if tmpRemote == "" {
@@ -49,83 +37,6 @@ func (repo *Repository) GetMergeBase(tmpRemote, base, head string) (string, stri
4937
return strings.TrimSpace(stdout), base, err
5038
}
5139

52-
// GetCompareInfo generates and returns compare information between base and head branches of repositories.
53-
func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string, directComparison, fileOnly bool) (_ *CompareInfo, err error) {
54-
var (
55-
remoteBranch string
56-
tmpRemote string
57-
)
58-
59-
// We don't need a temporary remote for same repository.
60-
if repo.Path != basePath {
61-
// Add a temporary remote
62-
tmpRemote = strconv.FormatInt(time.Now().UnixNano(), 10)
63-
if err = repo.AddRemote(tmpRemote, basePath, false); err != nil {
64-
return nil, fmt.Errorf("AddRemote: %w", err)
65-
}
66-
defer func() {
67-
if err := repo.RemoveRemote(tmpRemote); err != nil {
68-
logger.Error("GetPullRequestInfo: RemoveRemote: %v", err)
69-
}
70-
}()
71-
}
72-
73-
compareInfo := new(CompareInfo)
74-
75-
compareInfo.HeadCommitID, err = GetFullCommitID(repo.Ctx, repo.Path, headBranch)
76-
if err != nil {
77-
compareInfo.HeadCommitID = headBranch
78-
}
79-
80-
compareInfo.MergeBase, remoteBranch, err = repo.GetMergeBase(tmpRemote, baseBranch, headBranch)
81-
if err == nil {
82-
compareInfo.BaseCommitID, err = GetFullCommitID(repo.Ctx, repo.Path, remoteBranch)
83-
if err != nil {
84-
compareInfo.BaseCommitID = remoteBranch
85-
}
86-
separator := "..."
87-
baseCommitID := compareInfo.MergeBase
88-
if directComparison {
89-
separator = ".."
90-
baseCommitID = compareInfo.BaseCommitID
91-
}
92-
93-
// We have a common base - therefore we know that ... should work
94-
if !fileOnly {
95-
// avoid: ambiguous argument 'refs/a...refs/b': unknown revision or path not in the working tree. Use '--': 'git <command> [<revision>...] -- [<file>...]'
96-
var logs []byte
97-
logs, _, err = NewCommand("log").AddArguments(prettyLogFormat).
98-
AddDynamicArguments(baseCommitID+separator+headBranch).AddArguments("--").
99-
RunStdBytes(repo.Ctx, &RunOpts{Dir: repo.Path})
100-
if err != nil {
101-
return nil, err
102-
}
103-
compareInfo.Commits, err = repo.parsePrettyFormatLogToList(logs)
104-
if err != nil {
105-
return nil, fmt.Errorf("parsePrettyFormatLogToList: %w", err)
106-
}
107-
} else {
108-
compareInfo.Commits = []*Commit{}
109-
}
110-
} else {
111-
compareInfo.Commits = []*Commit{}
112-
compareInfo.MergeBase, err = GetFullCommitID(repo.Ctx, repo.Path, remoteBranch)
113-
if err != nil {
114-
compareInfo.MergeBase = remoteBranch
115-
}
116-
compareInfo.BaseCommitID = compareInfo.MergeBase
117-
}
118-
119-
// Count number of changed files.
120-
// This probably should be removed as we need to use shortstat elsewhere
121-
// Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly
122-
compareInfo.NumFiles, err = repo.GetDiffNumChangedFiles(remoteBranch, headBranch, directComparison)
123-
if err != nil {
124-
return nil, err
125-
}
126-
return compareInfo, nil
127-
}
128-
12940
type lineCountWriter struct {
13041
numLines int
13142
}

modules/gitrepo/config.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package gitrepo
5+
6+
import (
7+
"context"
8+
"strings"
9+
10+
"code.gitea.io/gitea/modules/git"
11+
"code.gitea.io/gitea/modules/globallock"
12+
)
13+
14+
func GitConfigGet(ctx context.Context, repo Repository, key string) (string, error) {
15+
result, _, err := git.NewCommand("config", "--get").
16+
AddDynamicArguments(key).
17+
RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)})
18+
if err != nil {
19+
return "", err
20+
}
21+
return strings.TrimSpace(result), nil
22+
}
23+
24+
func getRepoConfigLockKey(repoStoragePath string) string {
25+
return "repo-config:" + repoStoragePath
26+
}
27+
28+
// GitConfigAdd add a git configuration key to a specific value for the given repository.
29+
func GitConfigAdd(ctx context.Context, repo Repository, key, value string) error {
30+
return globallock.LockAndDo(ctx, getRepoConfigLockKey(repo.RelativePath()), func(ctx context.Context) error {
31+
_, _, err := git.NewCommand("config", "--add").
32+
AddDynamicArguments(key, value).
33+
RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)})
34+
return err
35+
})
36+
}
37+
38+
// GitConfigSet updates a git configuration key to a specific value for the given repository.
39+
// If the key does not exist, it will be created.
40+
// If the key exists, it will be updated to the new value.
41+
func GitConfigSet(ctx context.Context, repo Repository, key, value string) error {
42+
return globallock.LockAndDo(ctx, getRepoConfigLockKey(repo.RelativePath()), func(ctx context.Context) error {
43+
_, _, err := git.NewCommand("config").
44+
AddDynamicArguments(key, value).
45+
RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)})
46+
return err
47+
})
48+
}

modules/gitrepo/remote.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package gitrepo
5+
6+
import (
7+
"context"
8+
"errors"
9+
"io"
10+
"time"
11+
12+
"code.gitea.io/gitea/modules/git"
13+
giturl "code.gitea.io/gitea/modules/git/url"
14+
"code.gitea.io/gitea/modules/globallock"
15+
"code.gitea.io/gitea/modules/util"
16+
)
17+
18+
type RemoteOption string
19+
20+
const (
21+
RemoteOptionMirrorPush RemoteOption = "--mirror=push"
22+
RemoteOptionMirrorFetch RemoteOption = "--mirror=fetch"
23+
)
24+
25+
func GitRemoteAdd(ctx context.Context, repo Repository, remoteName, remoteURL string, options ...RemoteOption) error {
26+
return globallock.LockAndDo(ctx, getRepoConfigLockKey(repo.RelativePath()), func(ctx context.Context) error {
27+
cmd := git.NewCommand("remote", "add")
28+
if len(options) > 0 {
29+
switch options[0] {
30+
case RemoteOptionMirrorPush:
31+
cmd.AddArguments("--mirror=push")
32+
case RemoteOptionMirrorFetch:
33+
cmd.AddArguments("--mirror=fetch")
34+
default:
35+
return errors.New("unknown remote option: " + string(options[0]))
36+
}
37+
}
38+
_, _, err := cmd.
39+
AddDynamicArguments(remoteName, remoteURL).
40+
RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)})
41+
return err
42+
})
43+
}
44+
45+
func GitRemoteRemove(ctx context.Context, repo Repository, remoteName string) error {
46+
return globallock.LockAndDo(ctx, getRepoConfigLockKey(repo.RelativePath()), func(ctx context.Context) error {
47+
cmd := git.NewCommand("remote", "rm").AddDynamicArguments(remoteName)
48+
_, _, err := cmd.RunStdString(ctx, &git.RunOpts{Dir: repoPath(repo)})
49+
return err
50+
})
51+
}
52+
53+
// GitRemoteGetURL returns the url of a specific remote of the repository.
54+
func GitRemoteGetURL(ctx context.Context, repo Repository, remoteName string) (*giturl.GitURL, error) {
55+
addr, err := git.GetRemoteAddress(ctx, repoPath(repo), remoteName)
56+
if err != nil {
57+
return nil, err
58+
}
59+
if addr == "" {
60+
return nil, util.NewNotExistErrorf("remote '%s' does not exist", remoteName)
61+
}
62+
return giturl.ParseGitURL(addr)
63+
}
64+
65+
// GitRemotePrune prunes the remote branches that no longer exist in the remote repository.
66+
func GitRemotePrune(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error {
67+
return git.NewCommand("remote", "prune").AddDynamicArguments(remoteName).
68+
Run(ctx, &git.RunOpts{
69+
Timeout: timeout,
70+
Dir: repoPath(repo),
71+
Stdout: stdout,
72+
Stderr: stderr,
73+
})
74+
}
75+
76+
// GitRemoteUpdatePrune updates the remote branches and prunes the ones that no longer exist in the remote repository.
77+
func GitRemoteUpdatePrune(ctx context.Context, repo Repository, remoteName string, timeout time.Duration, stdout, stderr io.Writer) error {
78+
return git.NewCommand("remote", "update", "--prune").AddDynamicArguments(remoteName).
79+
Run(ctx, &git.RunOpts{
80+
Timeout: timeout,
81+
Dir: repoPath(repo),
82+
Stdout: stdout,
83+
Stderr: stderr,
84+
})
85+
}

modules/templates/util_misc.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ import (
1414

1515
activities_model "code.gitea.io/gitea/models/activities"
1616
repo_model "code.gitea.io/gitea/models/repo"
17-
"code.gitea.io/gitea/modules/git"
18-
giturl "code.gitea.io/gitea/modules/git/url"
17+
"code.gitea.io/gitea/modules/gitrepo"
1918
"code.gitea.io/gitea/modules/json"
2019
"code.gitea.io/gitea/modules/log"
2120
"code.gitea.io/gitea/modules/repository"
@@ -145,18 +144,12 @@ type remoteAddress struct {
145144

146145
func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string) remoteAddress {
147146
ret := remoteAddress{}
148-
remoteURL, err := git.GetRemoteAddress(ctx, m.RepoPath(), remoteName)
147+
u, err := gitrepo.GitRemoteGetURL(ctx, m, remoteName)
149148
if err != nil {
150149
log.Error("GetRemoteURL %v", err)
151150
return ret
152151
}
153152

154-
u, err := giturl.ParseGitURL(remoteURL)
155-
if err != nil {
156-
log.Error("giturl.Parse %v", err)
157-
return ret
158-
}
159-
160153
if u.Scheme != "ssh" && u.Scheme != "file" {
161154
if u.User != nil {
162155
ret.Username = u.User.Username()

routers/api/v1/repo/pull.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,7 @@ func MergePullRequest(ctx *context.APIContext) {
10851085
type parseCompareInfoResult struct {
10861086
headRepo *repo_model.Repository
10871087
headGitRepo *git.Repository
1088-
compareInfo *git.CompareInfo
1088+
compareInfo *pull_service.CompareInfo
10891089
baseRef git.RefName
10901090
headRef git.RefName
10911091
}
@@ -1201,7 +1201,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
12011201
return nil, nil
12021202
}
12031203

1204-
compareInfo, err := headGitRepo.GetCompareInfo(repo_model.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseRef.ShortName(), headRef.ShortName(), false, false)
1204+
compareInfo, err := pull_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef.ShortName(), headRef.ShortName(), false, false)
12051205
if err != nil {
12061206
ctx.APIErrorInternal(err)
12071207
return nil, nil
@@ -1452,7 +1452,7 @@ func GetPullRequestCommits(ctx *context.APIContext) {
14521452
return
14531453
}
14541454

1455-
var prInfo *git.CompareInfo
1455+
var prInfo *pull_service.CompareInfo
14561456
baseGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
14571457
if err != nil {
14581458
ctx.APIErrorInternal(err)
@@ -1461,9 +1461,9 @@ func GetPullRequestCommits(ctx *context.APIContext) {
14611461
defer closer.Close()
14621462

14631463
if pr.HasMerged {
1464-
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitHeadRefName(), false, false)
1464+
prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), false, false)
14651465
} else {
1466-
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitHeadRefName(), false, false)
1466+
prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), false, false)
14671467
}
14681468
if err != nil {
14691469
ctx.APIErrorInternal(err)
@@ -1582,11 +1582,11 @@ func GetPullRequestFiles(ctx *context.APIContext) {
15821582

15831583
baseGitRepo := ctx.Repo.GitRepo
15841584

1585-
var prInfo *git.CompareInfo
1585+
var prInfo *pull_service.CompareInfo
15861586
if pr.HasMerged {
1587-
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitHeadRefName(), true, false)
1587+
prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), true, false)
15881588
} else {
1589-
prInfo, err = baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitHeadRefName(), true, false)
1589+
prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), true, false)
15901590
}
15911591
if err != nil {
15921592
ctx.APIErrorInternal(err)

routers/common/compare.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@ import (
77
repo_model "code.gitea.io/gitea/models/repo"
88
user_model "code.gitea.io/gitea/models/user"
99
"code.gitea.io/gitea/modules/git"
10+
pull_service "code.gitea.io/gitea/services/pull"
1011
)
1112

1213
// CompareInfo represents the collected results from ParseCompareInfo
1314
type CompareInfo struct {
1415
HeadUser *user_model.User
1516
HeadRepo *repo_model.Repository
1617
HeadGitRepo *git.Repository
17-
CompareInfo *git.CompareInfo
18+
CompareInfo *pull_service.CompareInfo
1819
BaseBranch string
1920
HeadBranch string
2021
DirectComparison bool

0 commit comments

Comments
 (0)