Skip to content

Commit 60c85e7

Browse files
lunnylunny
committed
Some improvements
1 parent 0a9a6c7 commit 60c85e7

File tree

8 files changed

+238
-256
lines changed

8 files changed

+238
-256
lines changed

‎models/git/branch.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,11 @@ func GetDeletedBranchByID(ctx context.Context, repoID, branchID int64) (*Branch,
223223
return &branch, nil
224224
}
225225

226+
func DeleteRepoBranches(ctx context.Context, repoID int64) error {
227+
_, err := db.GetEngine(ctx).Where("repo_id=?", repoID).Delete(new(Branch))
228+
return err
229+
}
230+
226231
func DeleteBranches(ctx context.Context, repoID, doerID int64, branchIDs []int64) error {
227232
return db.WithTx(ctx, func(ctx context.Context) error {
228233
branches := make([]*Branch, 0, len(branchIDs))

‎models/repo/release.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,3 +558,8 @@ func FindTagsByCommitIDs(ctx context.Context, repoID int64, commitIDs ...string)
558558
}
559559
return res, nil
560560
}
561+
562+
func DeleteRepoReleases(ctx context.Context, repoID int64) error {
563+
_, err := db.GetEngine(ctx).Where("repo_id = ?", repoID).Delete(new(Release))
564+
return err
565+
}

‎services/repository/adopt.go‎

Lines changed: 44 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,18 @@ import (
2828
"github.com/gobwas/glob"
2929
)
3030

31+
func deleteFailedAdoptRepository(ctx context.Context, repoID int64) error {
32+
return db.WithTx(ctx, func(ctx context.Context) error {
33+
if err := deleteDBRepository(ctx, repoID); err != nil {
34+
return fmt.Errorf("deleteDBRepository: %w", err)
35+
}
36+
if err := git_model.DeleteRepoBranches(ctx, repoID); err != nil {
37+
return fmt.Errorf("deleteRepoBranches: %w", err)
38+
}
39+
return repo_model.DeleteRepoReleases(ctx, repoID)
40+
})
41+
}
42+
3143
// AdoptRepository adopts pre-existing repository files for the user/organization.
3244
func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateRepoOptions) (*repo_model.Repository, error) {
3345
if !doer.IsAdmin && !u.CanCreateRepo() {
@@ -52,64 +64,50 @@ func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateR
5264
IsEmpty: !opts.AutoInit,
5365
}
5466

55-
isExist, err := gitrepo.IsRepositoryExist(ctx, repo)
67+
// 1 - create the repository database operations first
68+
err := db.WithTx(ctx, func(ctx context.Context) error {
69+
return CreateRepositoryInDB(ctx, doer, u, repo, true, false)
70+
})
5671
if err != nil {
57-
log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err)
5872
return nil, err
5973
}
60-
if !isExist {
61-
return nil, repo_model.ErrRepoNotExist{
62-
OwnerName: u.Name,
63-
Name: repo.Name,
74+
75+
// last - clean up if something goes wrong
76+
defer func() {
77+
if err != nil {
78+
if errDel := deleteFailedAdoptRepository(ctx, repo.ID); errDel != nil {
79+
log.Error("Failed to delete repository %s that could not be adopted: %v", repo.FullName(), errDel)
80+
}
6481
}
65-
}
82+
}()
6683

67-
// create the repository database operations first
68-
if err := db.WithTx(ctx, func(ctx context.Context) error {
69-
return CreateRepositoryByExample(ctx, doer, u, repo, true, false)
70-
}); err != nil {
71-
return nil, err
84+
// Re-fetch the repository from database before updating it (else it would
85+
// override changes that were done earlier with sql)
86+
if repo, err = repo_model.GetRepositoryByID(ctx, repo.ID); err != nil {
87+
return nil, fmt.Errorf("getRepositoryByID: %w", err)
7288
}
7389

74-
if err := db.WithTx(ctx, func(ctx context.Context) error {
75-
// Re-fetch the repository from database before updating it (else it would
76-
// override changes that were done earlier with sql)
77-
if repo, err = repo_model.GetRepositoryByID(ctx, repo.ID); err != nil {
78-
return fmt.Errorf("getRepositoryByID: %w", err)
79-
}
80-
return nil
81-
}); err != nil {
82-
return nil, err
90+
// 3 - adopt the repository from disk
91+
if err := adoptRepository(ctx, repo, opts.DefaultBranch); err != nil {
92+
return nil, fmt.Errorf("adoptRepository: %w", err)
8393
}
8494

85-
if err := func() error {
86-
if err := adoptRepository(ctx, repo, opts.DefaultBranch); err != nil {
87-
return fmt.Errorf("adoptRepository: %w", err)
88-
}
89-
90-
if err := repo_module.CheckDaemonExportOK(ctx, repo); err != nil {
91-
return fmt.Errorf("checkDaemonExportOK: %w", err)
92-
}
93-
94-
if stdout, _, err := git.NewCommand("update-server-info").
95-
RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil {
96-
log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
97-
return fmt.Errorf("CreateRepository(git update-server-info): %w", err)
98-
}
95+
if err := repo_module.CheckDaemonExportOK(ctx, repo); err != nil {
96+
return nil, fmt.Errorf("checkDaemonExportOK: %w", err)
97+
}
9998

100-
// update repository status
101-
repo.Status = repo_model.RepositoryReady
102-
if err = repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil {
103-
return fmt.Errorf("UpdateRepositoryCols: %w", err)
104-
}
99+
if stdout, _, err := git.NewCommand("update-server-info").
100+
RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil {
101+
log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
102+
return nil, fmt.Errorf("CreateRepository(git update-server-info): %w", err)
103+
}
105104

106-
return nil
107-
}(); err != nil {
108-
if errDel := DeleteRepository(ctx, doer, repo, false /* no notify */); errDel != nil {
109-
log.Error("Failed to delete repository %s that could not be adopted: %v", repo.FullName(), errDel)
110-
}
111-
return nil, err
105+
// 4 - update repository status
106+
repo.Status = repo_model.RepositoryReady
107+
if err = repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil {
108+
return nil, fmt.Errorf("UpdateRepositoryCols: %w", err)
112109
}
110+
113111
notify_service.AdoptRepository(ctx, doer, u, repo)
114112

115113
return repo, nil

‎services/repository/create.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt
248248
needsUpdateStatus := opts.Status != repo_model.RepositoryReady
249249

250250
if err := db.WithTx(ctx, func(ctx context.Context) error {
251-
return CreateRepositoryByExample(ctx, doer, u, repo, false, false)
251+
return CreateRepositoryInDB(ctx, doer, u, repo, false, false)
252252
}); err != nil {
253253
return nil, err
254254
}
@@ -344,8 +344,8 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt
344344
return repo, nil
345345
}
346346

347-
// CreateRepositoryByExample creates a repository for the user/organization.
348-
func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, overwriteOrAdopt, isFork bool) (err error) {
347+
// CreateRepositoryInDB creates a repository for the user/organization.
348+
func CreateRepositoryInDB(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, overwriteOrAdopt, isFork bool) (err error) {
349349
if err = repo_model.IsUsableRepoName(repo.Name); err != nil {
350350
return err
351351
}
@@ -365,7 +365,17 @@ func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, re
365365
log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err)
366366
return err
367367
}
368-
if !overwriteOrAdopt && isExist {
368+
if overwriteOrAdopt != isExist {
369+
if overwriteOrAdopt {
370+
// repo should exist but doesn't - We have two or three options.
371+
log.Error("Files do not exist in %s and we are not going to adopt or delete.", repo.FullName())
372+
return repo_model.ErrRepoNotExist{
373+
OwnerName: u.Name,
374+
Name: repo.Name,
375+
}
376+
}
377+
378+
// repo already exists - We have two or three options.
369379
log.Error("Files already exist in %s and we are not going to adopt or delete.", repo.FullName())
370380
return repo_model.ErrRepoFilesAlreadyExist{
371381
Uname: u.Name,

‎services/repository/delete.go‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,19 @@ import (
3232
"xorm.io/builder"
3333
)
3434

35+
func deleteDBRepository(ctx context.Context, repoID int64) error {
36+
if cnt, err := db.GetEngine(ctx).ID(repoID).Delete(&repo_model.Repository{}); err != nil {
37+
return err
38+
} else if cnt != 1 {
39+
return repo_model.ErrRepoNotExist{
40+
ID: repoID,
41+
OwnerName: "",
42+
Name: "",
43+
}
44+
}
45+
return nil
46+
}
47+
3548
// DeleteRepository deletes a repository for a user or organization.
3649
// make sure if you call this func to close open sessions (sqlite will otherwise get a deadlock)
3750
func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID int64, ignoreOrgTeams ...bool) error {
@@ -82,14 +95,8 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID
8295
}
8396
needRewriteKeysFile := deleted > 0
8497

85-
if cnt, err := sess.ID(repoID).Delete(&repo_model.Repository{}); err != nil {
98+
if err := deleteDBRepository(ctx, repoID); err != nil {
8699
return err
87-
} else if cnt != 1 {
88-
return repo_model.ErrRepoNotExist{
89-
ID: repoID,
90-
OwnerName: "",
91-
Name: "",
92-
}
93100
}
94101

95102
if org != nil && org.IsOrganization() {

‎services/repository/fork.go‎

Lines changed: 56 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -100,95 +100,76 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
100100
IsFork: true,
101101
ForkID: opts.BaseRepo.ID,
102102
ObjectFormatName: opts.BaseRepo.ObjectFormatName,
103+
Status: repo_model.RepositoryBeingMigrated,
103104
}
104105

105-
oldRepoPath := opts.BaseRepo.RepoPath()
106-
107-
needsRollback := false
108-
rollbackFn := func() {
109-
if !needsRollback {
110-
return
111-
}
112-
113-
if exists, _ := gitrepo.IsRepositoryExist(ctx, repo); !exists {
114-
return
115-
}
116-
117-
// As the transaction will be failed and hence database changes will be destroyed we only need
118-
// to delete the related repository on the filesystem
119-
if errDelete := gitrepo.DeleteRepository(ctx, repo); errDelete != nil {
120-
log.Error("Failed to remove fork repo")
121-
}
122-
}
123-
124-
needsRollbackInPanic := true
125-
defer func() {
126-
panicErr := recover()
127-
if panicErr == nil {
128-
return
129-
}
130-
131-
if needsRollbackInPanic {
132-
rollbackFn()
133-
}
134-
panic(panicErr)
135-
}()
136-
137-
err = db.WithTx(ctx, func(txCtx context.Context) error {
138-
if err = CreateRepositoryByExample(txCtx, doer, owner, repo, false, true); err != nil {
106+
// 1 - Create the repository in the database
107+
err = db.WithTx(ctx, func(ctx context.Context) error {
108+
if err = CreateRepositoryInDB(ctx, doer, owner, repo, false, true); err != nil {
139109
return err
140110
}
141-
142-
if err = repo_model.IncrementRepoForkNum(txCtx, opts.BaseRepo.ID); err != nil {
111+
if err = repo_model.IncrementRepoForkNum(ctx, opts.BaseRepo.ID); err != nil {
143112
return err
144113
}
145114

146115
// copy lfs files failure should not be ignored
147-
if err = git_model.CopyLFS(txCtx, repo, opts.BaseRepo); err != nil {
148-
return err
149-
}
150-
151-
needsRollback = true
116+
return git_model.CopyLFS(ctx, repo, opts.BaseRepo)
117+
})
118+
if err != nil {
119+
return nil, err
120+
}
152121

153-
cloneCmd := git.NewCommand("clone", "--bare")
154-
if opts.SingleBranch != "" {
155-
cloneCmd.AddArguments("--single-branch", "--branch").AddDynamicArguments(opts.SingleBranch)
156-
}
157-
if stdout, _, err := cloneCmd.AddDynamicArguments(oldRepoPath, repo.RepoPath()).
158-
RunStdBytes(txCtx, &git.RunOpts{Timeout: 10 * time.Minute}); err != nil {
159-
log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, opts.BaseRepo, stdout, err)
160-
return fmt.Errorf("git clone: %w", err)
122+
// last - clean up if something goes wrong
123+
defer func() {
124+
if err != nil {
125+
if errDelete := gitrepo.DeleteRepository(ctx, repo); errDelete != nil {
126+
log.Error("Failed to remove fork repo")
127+
}
161128
}
129+
}()
162130

163-
if err := repo_module.CheckDaemonExportOK(txCtx, repo); err != nil {
164-
return fmt.Errorf("checkDaemonExportOK: %w", err)
165-
}
131+
// 2 - Clone the repository
132+
cloneCmd := git.NewCommand("clone", "--bare")
133+
if opts.SingleBranch != "" {
134+
cloneCmd.AddArguments("--single-branch", "--branch").AddDynamicArguments(opts.SingleBranch)
135+
}
136+
if stdout, _, err := cloneCmd.AddDynamicArguments(opts.BaseRepo.RepoPath(), repo.RepoPath()).
137+
RunStdBytes(ctx, &git.RunOpts{Timeout: 10 * time.Minute}); err != nil {
138+
log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, opts.BaseRepo, stdout, err)
139+
return nil, fmt.Errorf("git clone: %w", err)
140+
}
166141

167-
if stdout, _, err := git.NewCommand("update-server-info").
168-
RunStdString(txCtx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil {
169-
log.Error("Fork Repository (git update-server-info) failed for %v:\nStdout: %s\nError: %v", repo, stdout, err)
170-
return fmt.Errorf("git update-server-info: %w", err)
171-
}
142+
// 3 - Update the repository
143+
if err := repo_module.CheckDaemonExportOK(ctx, repo); err != nil {
144+
return nil, fmt.Errorf("checkDaemonExportOK: %w", err)
145+
}
172146

173-
if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
174-
return fmt.Errorf("createDelegateHooks: %w", err)
175-
}
147+
if stdout, _, err := git.NewCommand("update-server-info").
148+
RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil {
149+
log.Error("Fork Repository (git update-server-info) failed for %v:\nStdout: %s\nError: %v", repo, stdout, err)
150+
return nil, fmt.Errorf("git update-server-info: %w", err)
151+
}
176152

177-
gitRepo, err := gitrepo.OpenRepository(txCtx, repo)
178-
if err != nil {
179-
return fmt.Errorf("OpenRepository: %w", err)
180-
}
181-
defer gitRepo.Close()
153+
// 4 - Create hooks
154+
if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
155+
return nil, fmt.Errorf("createDelegateHooks: %w", err)
156+
}
182157

183-
_, err = repo_module.SyncRepoBranchesWithRepo(txCtx, repo, gitRepo, doer.ID)
184-
return err
185-
})
186-
needsRollbackInPanic = false
158+
// 5 - Sync the repository branches and tags
159+
gitRepo, err := gitrepo.OpenRepository(ctx, repo)
187160
if err != nil {
188-
rollbackFn()
189-
return nil, err
161+
return nil, fmt.Errorf("OpenRepository: %w", err)
190162
}
163+
defer gitRepo.Close()
191164

165+
if _, err = repo_module.SyncRepoBranchesWithRepo(ctx, repo, gitRepo, doer.ID); err != nil {
166+
return nil, fmt.Errorf("SyncRepoBranchesWithRepo: %w", err)
167+
}
168+
if err := repo_module.SyncReleasesWithTags(ctx, repo, gitRepo); err != nil {
169+
return nil, fmt.Errorf("Sync releases from git tags failed: %v", err)
170+
}
171+
172+
// 6 - Update the repository
192173
// even if below operations failed, it could be ignored. And they will be retried
193174
if err := repo_module.UpdateRepoSize(ctx, repo); err != nil {
194175
log.Error("Failed to update size for repository: %v", err)
@@ -200,14 +181,10 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
200181
return nil, err
201182
}
202183

203-
gitRepo, err := gitrepo.OpenRepository(ctx, repo)
204-
if err != nil {
205-
log.Error("Open created git repository failed: %v", err)
206-
} else {
207-
defer gitRepo.Close()
208-
if err := repo_module.SyncReleasesWithTags(ctx, repo, gitRepo); err != nil {
209-
log.Error("Sync releases from git tags failed: %v", err)
210-
}
184+
// 7 - update repository status to be ready
185+
repo.Status = repo_model.RepositoryReady
186+
if err = repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil {
187+
return nil, fmt.Errorf("UpdateRepositoryCols: %w", err)
211188
}
212189

213190
notify_service.ForkRepository(ctx, doer, opts.BaseRepo, repo)

0 commit comments

Comments
 (0)