Skip to content

Commit 3ec97d0

Browse files
committed
fix get branch
1 parent ca32157 commit 3ec97d0

File tree

2 files changed

+13
-6
lines changed

2 files changed

+13
-6
lines changed

models/git/branch.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,12 +166,10 @@ func GetBranch(ctx context.Context, repoID int64, branchName string) (*Branch, e
166166
RepoID: repoID,
167167
BranchName: branchName,
168168
}
169-
} else if branch.IsDeleted {
170-
return nil, ErrBranchNotExist{
171-
RepoID: repoID,
172-
BranchName: branchName,
173-
}
174169
}
170+
// FIXME: this design is not right: it doesn't check `branch.IsDeleted`, it doesn't make sense to make callers to check IsDeleted again and again.
171+
// It causes inconsistency with `GetBranches` and `git.GetBranch`, and will lead to strange bugs
172+
// In the future, there should be 2 functions: `GetBranchExisting` and `GetBranchWithDeleted`
175173
return &branch, nil
176174
}
177175

services/repository/branch.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,11 +684,20 @@ func GetBranchDivergingInfo(ctx reqctx.RequestContext, baseRepo *repo_model.Repo
684684
if err != nil {
685685
return nil, err
686686
}
687-
687+
if headGitBranch.IsDeleted {
688+
return nil, git_model.ErrBranchNotExist{
689+
BranchName: headBranch,
690+
}
691+
}
688692
baseGitBranch, err := git_model.GetBranch(ctx, baseRepo.ID, baseBranch)
689693
if err != nil {
690694
return nil, err
691695
}
696+
if baseGitBranch.IsDeleted {
697+
return nil, git_model.ErrBranchNotExist{
698+
BranchName: baseBranch,
699+
}
700+
}
692701

693702
info := &BranchDivergingInfo{}
694703
if headGitBranch.CommitID == baseGitBranch.CommitID {

0 commit comments

Comments
 (0)