History

Simplify how git repositories are opened (#28937)

## Purpose This is a refactor toward building an abstraction over managing git repositories. Afterwards, it does not matter anymore if they are stored on the local disk or somewhere remote. ## What this PR changes We used `git.OpenRepository` everywhere previously. Now, we should split them into two distinct functions: Firstly, there are temporary repositories which do not change: ```go git.OpenRepository(ctx, diskPath) ``` Gitea managed repositories having a record in the database in the `repository` table are moved into the new package `gitrepo`: ```go gitrepo.OpenRepository(ctx, repo_model.Repo) ``` Why is `repo_model.Repository` the second parameter instead of file path? Because then we can easily adapt our repository storage strategy. The repositories can be stored locally, however, they could just as well be stored on a remote server. ## Further changes in other PRs - A Git Command wrapper on package `gitrepo` could be created. i.e. `NewCommand(ctx, repo_model.Repository, commands...)`. `git.RunOpts{Dir: repo.RepoPath()}`, the directory should be empty before invoking this method and it can be filled in the function only. #28940 - Remove the `RepoPath()`/`WikiPath()` functions to reduce the possibility of mistakes. ---------

Lunny Xiao committed 2 years ago

Commit 5f82ead1 parent 60e4a9884 files · +426 −273Browse files

Showing the changes to services/pull (9 of 84 files). Show all

Unified · Split

9 files
  1. services/pull/check.go +2 −1
  2. services/pull/comment.go +2 −3
  3. services/pull/commit_status.go +2 −1
  4. services/pull/merge_rebase.go +2 −1
  5. services/pull/merge_squash.go +2 −1
  6. services/pull/patch.go +2 −1
  7. services/pull/pull.go +9 −8
  8. services/pull/pull_test.go +3 −2
  9. services/pull/review.go +2 −1
services/pull/check.go+2−1
@@ -20,6 +20,7 @@ import ( · 19 unmodified lines
20 "code.gitea.io/gitea/models/unit"20 "code.gitea.io/gitea/models/unit"
21 user_model "code.gitea.io/gitea/models/user"21 user_model "code.gitea.io/gitea/models/user"
22 "code.gitea.io/gitea/modules/git"22 "code.gitea.io/gitea/modules/git"
23 "code.gitea.io/gitea/modules/gitrepo"
23 "code.gitea.io/gitea/modules/graceful"24 "code.gitea.io/gitea/modules/graceful"
24 "code.gitea.io/gitea/modules/log"25 "code.gitea.io/gitea/modules/log"
25 "code.gitea.io/gitea/modules/process"26 "code.gitea.io/gitea/modules/process"
@@ -215,7 +216,7 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com · 189 unmodified lines
215 return nil, fmt.Errorf("GetFullCommitID(%s) in %s: %w", prHeadRef, pr.BaseRepo.FullName(), err)216 return nil, fmt.Errorf("GetFullCommitID(%s) in %s: %w", prHeadRef, pr.BaseRepo.FullName(), err)
216 }217 }
217 218
218 gitRepo, err := git.OpenRepository(ctx, pr.BaseRepo.RepoPath())219 gitRepo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo)
219 if err != nil {220 if err != nil {
220 return nil, fmt.Errorf("%-v OpenRepository: %w", pr.BaseRepo, err)221 return nil, fmt.Errorf("%-v OpenRepository: %w", pr.BaseRepo, err)
221 }222 }
services/pull/comment.go+2−3
@@ -9,7 +9,7 @@ import ( · 8 unmodified lines
9 issues_model "code.gitea.io/gitea/models/issues"9 issues_model "code.gitea.io/gitea/models/issues"
10 repo_model "code.gitea.io/gitea/models/repo"10 repo_model "code.gitea.io/gitea/models/repo"
11 user_model "code.gitea.io/gitea/models/user"11 user_model "code.gitea.io/gitea/models/user"
12 "code.gitea.io/gitea/modules/git"12 "code.gitea.io/gitea/modules/gitrepo"
13 "code.gitea.io/gitea/modules/json"13 "code.gitea.io/gitea/modules/json"
14)14)
15 15
@@ -17,8 +17,7 @@ import ( · 1 unmodified line
17// isForcePush will be true if oldCommit isn't on the branch17// isForcePush will be true if oldCommit isn't on the branch
18// Commit on baseBranch will skip18// Commit on baseBranch will skip
19func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldCommitID, newCommitID, baseBranch string) (commitIDs []string, isForcePush bool, err error) {19func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldCommitID, newCommitID, baseBranch string) (commitIDs []string, isForcePush bool, err error) {
20 repoPath := repo.RepoPath()20 gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
21 gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, repoPath)
22 if err != nil {21 if err != nil {
23 return nil, false, err22 return nil, false, err
24 }23 }
services/pull/commit_status.go+2−1
@@ -11,6 +11,7 @@ import ( · 10 unmodified lines
11 git_model "code.gitea.io/gitea/models/git"11 git_model "code.gitea.io/gitea/models/git"
12 issues_model "code.gitea.io/gitea/models/issues"12 issues_model "code.gitea.io/gitea/models/issues"
13 "code.gitea.io/gitea/modules/git"13 "code.gitea.io/gitea/modules/git"
14 "code.gitea.io/gitea/modules/gitrepo"
14 "code.gitea.io/gitea/modules/log"15 "code.gitea.io/gitea/modules/log"
15 "code.gitea.io/gitea/modules/structs"16 "code.gitea.io/gitea/modules/structs"
16 17
@@ -116,7 +117,7 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR · 99 unmodified lines
116 }117 }
117 118
118 // check if all required status checks are successful119 // check if all required status checks are successful
119 headGitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.HeadRepo.RepoPath())120 headGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
120 if err != nil {121 if err != nil {
121 return "", errors.Wrap(err, "OpenRepository")122 return "", errors.Wrap(err, "OpenRepository")
122 }123 }
services/pull/merge_rebase.go+2−1
@@ -9,6 +9,7 @@ import ( · 8 unmodified lines
9 9
10 repo_model "code.gitea.io/gitea/models/repo"10 repo_model "code.gitea.io/gitea/models/repo"
11 "code.gitea.io/gitea/modules/git"11 "code.gitea.io/gitea/modules/git"
12 "code.gitea.io/gitea/modules/gitrepo"
12 "code.gitea.io/gitea/modules/log"13 "code.gitea.io/gitea/modules/log"
13)14)
14 15
@@ -57,7 +58,7 @@ func doMergeRebaseFastForward(ctx *mergeContext) error { · 42 unmodified lines
57 }58 }
58 59
59 // Original repo to read template from.60 // Original repo to read template from.
60 baseGitRepo, err := git.OpenRepository(ctx, ctx.pr.BaseRepo.RepoPath())61 baseGitRepo, err := gitrepo.OpenRepository(ctx, ctx.pr.BaseRepo)
61 if err != nil {62 if err != nil {
62 log.Error("Unable to get Git repo for rebase: %v", err)63 log.Error("Unable to get Git repo for rebase: %v", err)
63 return err64 return err
services/pull/merge_squash.go+2−1
@@ -10,6 +10,7 @@ import ( · 9 unmodified lines
10 user_model "code.gitea.io/gitea/models/user"10 user_model "code.gitea.io/gitea/models/user"
11 "code.gitea.io/gitea/modules/container"11 "code.gitea.io/gitea/modules/container"
12 "code.gitea.io/gitea/modules/git"12 "code.gitea.io/gitea/modules/git"
13 "code.gitea.io/gitea/modules/gitrepo"
13 "code.gitea.io/gitea/modules/log"14 "code.gitea.io/gitea/modules/log"
14 "code.gitea.io/gitea/modules/setting"15 "code.gitea.io/gitea/modules/setting"
15)16)
@@ -24,7 +25,7 @@ func getAuthorSignatureSquash(ctx *mergeContext) (*git.Signature, error) { · 8 unmodified lines
24 // Try to get an signature from the same user in one of the commits, as the25 // Try to get an signature from the same user in one of the commits, as the
25 // poster email might be private or commits might have a different signature26 // poster email might be private or commits might have a different signature
26 // than the primary email address of the poster.27 // than the primary email address of the poster.
27 gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, ctx.tmpBasePath)28 gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpenPath(ctx, ctx.tmpBasePath)
28 if err != nil {29 if err != nil {
29 log.Error("%-v Unable to open base repository: %v", ctx.pr, err)30 log.Error("%-v Unable to open base repository: %v", ctx.pr, err)
30 return nil, err31 return nil, err
services/pull/patch.go+2−1
@@ -19,6 +19,7 @@ import ( · 18 unmodified lines
19 "code.gitea.io/gitea/models/unit"19 "code.gitea.io/gitea/models/unit"
20 "code.gitea.io/gitea/modules/container"20 "code.gitea.io/gitea/modules/container"
21 "code.gitea.io/gitea/modules/git"21 "code.gitea.io/gitea/modules/git"
22 "code.gitea.io/gitea/modules/gitrepo"
22 "code.gitea.io/gitea/modules/graceful"23 "code.gitea.io/gitea/modules/graceful"
23 "code.gitea.io/gitea/modules/log"24 "code.gitea.io/gitea/modules/log"
24 "code.gitea.io/gitea/modules/process"25 "code.gitea.io/gitea/modules/process"
@@ -35,7 +36,7 @@ func DownloadDiffOrPatch(ctx context.Context, pr *issues_model.PullRequest, w io · 10 unmodified lines
35 return err36 return err
36 }37 }
37 38
38 gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.BaseRepo.RepoPath())39 gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
39 if err != nil {40 if err != nil {
40 return fmt.Errorf("OpenRepository: %w", err)41 return fmt.Errorf("OpenRepository: %w", err)
41 }42 }
services/pull/pull.go+9−8
@@ -23,6 +23,7 @@ import ( · 22 unmodified lines
23 "code.gitea.io/gitea/modules/container"23 "code.gitea.io/gitea/modules/container"
24 gitea_context "code.gitea.io/gitea/modules/context"24 gitea_context "code.gitea.io/gitea/modules/context"
25 "code.gitea.io/gitea/modules/git"25 "code.gitea.io/gitea/modules/git"
26 "code.gitea.io/gitea/modules/gitrepo"
26 "code.gitea.io/gitea/modules/graceful"27 "code.gitea.io/gitea/modules/graceful"
27 "code.gitea.io/gitea/modules/json"28 "code.gitea.io/gitea/modules/json"
28 "code.gitea.io/gitea/modules/log"29 "code.gitea.io/gitea/modules/log"
@@ -62,7 +63,7 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss · 33 unmodified lines
62 assigneeCommentMap := make(map[int64]*issues_model.Comment)63 assigneeCommentMap := make(map[int64]*issues_model.Comment)
63 64
64 // add first push codes comment65 // add first push codes comment
65 baseGitRepo, err := git.OpenRepository(ctx, pr.BaseRepo.RepoPath())66 baseGitRepo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo)
66 if err != nil {67 if err != nil {
67 return err68 return err
68 }69 }
@@ -269,9 +270,9 @@ func checkForInvalidation(ctx context.Context, requests issues_model.PullRequest · 200 unmodified lines
269 if err != nil {270 if err != nil {
270 return fmt.Errorf("GetRepositoryByIDCtx: %w", err)271 return fmt.Errorf("GetRepositoryByIDCtx: %w", err)
271 }272 }
272 gitRepo, err := git.OpenRepository(ctx, repo.RepoPath())273 gitRepo, err := gitrepo.OpenRepository(ctx, repo)
273 if err != nil {274 if err != nil {
274 return fmt.Errorf("git.OpenRepository: %w", err)275 return fmt.Errorf("gitrepo.OpenRepository: %w", err)
275 }276 }
276 go func() {277 go func() {
277 // FIXME: graceful: We need to tell the manager we're doing something...278 // FIXME: graceful: We need to tell the manager we're doing something...
@@ -614,7 +615,7 @@ func CloseBranchPulls(ctx context.Context, doer *user_model.User, repoID int64, · 336 unmodified lines
614 615
615// CloseRepoBranchesPulls close all pull requests which head branches are in the given repository, but only whose base repo is not in the given repository616// CloseRepoBranchesPulls close all pull requests which head branches are in the given repository, but only whose base repo is not in the given repository
616func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *repo_model.Repository) error {617func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *repo_model.Repository) error {
617 branches, _, err := git.GetBranchesByPath(ctx, repo.RepoPath(), 0, 0)618 branches, _, err := gitrepo.GetBranchesByPath(ctx, repo, 0, 0)
618 if err != nil {619 if err != nil {
619 return err620 return err
620 }621 }
@@ -671,7 +672,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ · 50 unmodified lines
671 }672 }
672 }673 }
673 674
674 gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.HeadRepo.RepoPath())675 gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
675 if err != nil {676 if err != nil {
676 log.Error("Unable to open head repository: Error: %v", err)677 log.Error("Unable to open head repository: Error: %v", err)
677 return ""678 return ""
@@ -845,7 +846,7 @@ func GetIssuesAllCommitStatus(ctx context.Context, issues issues_model.IssueList · 167 unmodified lines
845 }846 }
846 gitRepo, ok := gitRepos[issue.RepoID]847 gitRepo, ok := gitRepos[issue.RepoID]
847 if !ok {848 if !ok {
848 gitRepo, err = git.OpenRepository(ctx, issue.Repo.RepoPath())849 gitRepo, err = gitrepo.OpenRepository(ctx, issue.Repo)
849 if err != nil {850 if err != nil {
850 log.Error("Cannot open git repository %-v for issue #%d[%d]. Error: %v", issue.Repo, issue.Index, issue.ID, err)851 log.Error("Cannot open git repository %-v for issue #%d[%d]. Error: %v", issue.Repo, issue.Index, issue.ID, err)
851 continue852 continue
@@ -882,7 +883,7 @@ func IsHeadEqualWithBranch(ctx context.Context, pr *issues_model.PullRequest, br · 30 unmodified lines
882 if err = pr.LoadBaseRepo(ctx); err != nil {883 if err = pr.LoadBaseRepo(ctx); err != nil {
883 return false, err884 return false, err
884 }885 }
885 baseGitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.BaseRepo.RepoPath())886 baseGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
886 if err != nil {887 if err != nil {
887 return false, err888 return false, err
888 }889 }
@@ -902,7 +903,7 @@ func IsHeadEqualWithBranch(ctx context.Context, pr *issues_model.PullRequest, br · 13 unmodified lines
902 } else {903 } else {
903 var closer io.Closer904 var closer io.Closer
904 905
905 headGitRepo, closer, err = git.RepositoryFromContextOrOpen(ctx, pr.HeadRepo.RepoPath())906 headGitRepo, closer, err = gitrepo.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
906 if err != nil {907 if err != nil {
907 return false, err908 return false, err
908 }909 }
services/pull/pull_test.go+3−2
@@ -13,6 +13,7 @@ import ( · 12 unmodified lines
13 "code.gitea.io/gitea/models/unit"13 "code.gitea.io/gitea/models/unit"
14 "code.gitea.io/gitea/models/unittest"14 "code.gitea.io/gitea/models/unittest"
15 "code.gitea.io/gitea/modules/git"15 "code.gitea.io/gitea/modules/git"
16 "code.gitea.io/gitea/modules/gitrepo"
16 17
17 "github.com/stretchr/testify/assert"18 "github.com/stretchr/testify/assert"
18)19)
@@ -41,7 +42,7 @@ func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) { · 22 unmodified lines
41 pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})42 pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
42 43
43 assert.NoError(t, pr.LoadBaseRepo(db.DefaultContext))44 assert.NoError(t, pr.LoadBaseRepo(db.DefaultContext))
44 gitRepo, err := git.OpenRepository(git.DefaultContext, pr.BaseRepo.RepoPath())45 gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, pr.BaseRepo)
45 assert.NoError(t, err)46 assert.NoError(t, err)
46 defer gitRepo.Close()47 defer gitRepo.Close()
47 48
@@ -71,7 +72,7 @@ func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) { · 23 unmodified lines
71 pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2, BaseRepo: baseRepo})72 pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2, BaseRepo: baseRepo})
72 73
73 assert.NoError(t, pr.LoadBaseRepo(db.DefaultContext))74 assert.NoError(t, pr.LoadBaseRepo(db.DefaultContext))
74 gitRepo, err := git.OpenRepository(git.DefaultContext, pr.BaseRepo.RepoPath())75 gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, pr.BaseRepo)
75 assert.NoError(t, err)76 assert.NoError(t, err)
76 defer gitRepo.Close()77 defer gitRepo.Close()
77 78
services/pull/review.go+2−1
@@ -16,6 +16,7 @@ import ( · 15 unmodified lines
16 repo_model "code.gitea.io/gitea/models/repo"16 repo_model "code.gitea.io/gitea/models/repo"
17 user_model "code.gitea.io/gitea/models/user"17 user_model "code.gitea.io/gitea/models/user"
18 "code.gitea.io/gitea/modules/git"18 "code.gitea.io/gitea/modules/git"
19 "code.gitea.io/gitea/modules/gitrepo"
19 "code.gitea.io/gitea/modules/log"20 "code.gitea.io/gitea/modules/log"
20 "code.gitea.io/gitea/modules/setting"21 "code.gitea.io/gitea/modules/setting"
21 "code.gitea.io/gitea/modules/util"22 "code.gitea.io/gitea/modules/util"
@@ -170,7 +171,7 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo · 148 unmodified lines
170 if err := pr.LoadBaseRepo(ctx); err != nil {171 if err := pr.LoadBaseRepo(ctx); err != nil {
171 return nil, fmt.Errorf("LoadBaseRepo: %w", err)172 return nil, fmt.Errorf("LoadBaseRepo: %w", err)
172 }173 }
173 gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.BaseRepo.RepoPath())174 gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
174 if err != nil {175 if err != nil {
175 return nil, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)176 return nil, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
176 }177 }

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t