Mirrors/gitea
0
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-10-31 11:41:32 +01:00
Code Issues Activity

Fix GetCommitBranchStart bug (#33298) (#33421)

Browse Source 
Backport #33298 by Zettat123

Fix #33265
Fix #33370

This PR also fixes some bugs in `TestGitGeneral`.

---------

Co-authored-by: Zettat123 <zettat123@gmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Giteabot 2025-02-01 16:43:10 +08:00 committed by GitHub
parent 7df1204795
commit ebac324ff2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 50 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
modules/git/commit_test.go
View File

@ -360,5 +360,5 @@ func Test_GetCommitBranchStart(t *testing.T) {
startCommitID, err := repo.GetCommitBranchStart(os.Environ(), "branch1", commit.ID.String())
assert.NoError(t, err)
assert.NotEmpty(t, startCommitID)
assert.EqualValues(t, "9c9aef8dd84e02bc7ec12641deb4c930a7c30185", startCommitID)
assert.EqualValues(t, "95bb4d39648ee7e325106df01a621c530863a653", startCommitID)
}

8
modules/git/repo_commit.go
View File

@ -519,6 +519,7 @@ func (repo *Repository) AddLastCommitCache(cacheKey, fullName, sha string) error
return nil
}
// GetCommitBranchStart returns the commit where the branch diverged
func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID string) (string, error) {
cmd := NewCommand(repo.Ctx, "log", prettyLogFormat)
cmd.AddDynamicArguments(endCommitID)
@ -533,7 +534,8 @@ func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID s
parts := bytes.Split(bytes.TrimSpace(stdout), []byte{'\n'})
var startCommitID string
// check the commits one by one until we find a commit contained by another branch
// and we think this commit is the divergence point
for _, commitID := range parts {
branches, err := repo.getBranches(env, string(commitID), 2)
if err != nil {
@ -541,11 +543,9 @@ func (repo *Repository) GetCommitBranchStart(env []string, branch, endCommitID s
}
for _, b := range branches {
if b != branch {
return startCommitID, nil
return string(commitID), nil
}
}
startCommitID = string(commitID)
}
return "", nil

71
tests/integration/git_general_test.go
View File

@ -80,6 +80,7 @@ func testGitGeneral(t *testing.T, u *url.URL) {
mediaTest(t, &httpContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1])
t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &httpContext, "test/head"))
t.Run("CreateProtectedBranch", doCreateProtectedBranch(&httpContext, dstPath))
t.Run("BranchProtectMerge", doBranchProtectPRMerge(&httpContext, dstPath))
t.Run("AutoMerge", doAutoPRMerge(&httpContext, dstPath))
t.Run("CreatePRAndSetManuallyMerged", doCreatePRAndSetManuallyMerged(httpContext, httpContext, dstPath, "master", "test-manually-merge"))
@ -121,6 +122,7 @@ func testGitGeneral(t *testing.T, u *url.URL) {
mediaTest(t, &sshContext, pushedFilesStandard[0], pushedFilesStandard[1], pushedFilesLFS[0], pushedFilesLFS[1])
t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &sshContext, "test/head2"))
t.Run("CreateProtectedBranch", doCreateProtectedBranch(&sshContext, dstPath))
t.Run("BranchProtectMerge", doBranchProtectPRMerge(&sshContext, dstPath))
t.Run("MergeFork", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
@ -325,6 +327,34 @@ func generateCommitWithNewData(size int, repoPath, email, fullName, prefix strin
return filepath.Base(tmpFile.Name()), err
}
func doCreateProtectedBranch(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
return func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
ctx := NewAPITestContext(t, baseCtx.Username, baseCtx.Reponame, auth_model.AccessTokenScopeWriteRepository)
t.Run("ProtectBranchWithFilePatterns", doProtectBranch(ctx, "release-*", baseCtx.Username, "", "", "config*"))
// push a new branch without any new commits
t.Run("CreateProtectedBranch-NoChanges", doGitCreateBranch(dstPath, "release-v1.0"))
t.Run("PushProtectedBranch-NoChanges", doGitPushTestRepository(dstPath, "origin", "release-v1.0"))
t.Run("CheckoutMaster-NoChanges", doGitCheckoutBranch(dstPath, "master"))
// push a new branch with a new unprotected file
t.Run("CreateProtectedBranch-UnprotectedFile", doGitCreateBranch(dstPath, "release-v2.0"))
_, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "abc.txt")
assert.NoError(t, err)
t.Run("PushProtectedBranch-UnprotectedFile", doGitPushTestRepository(dstPath, "origin", "release-v2.0"))
t.Run("CheckoutMaster-UnprotectedFile", doGitCheckoutBranch(dstPath, "master"))
// push a new branch with a new protected file
t.Run("CreateProtectedBranch-ProtectedFile", doGitCreateBranch(dstPath, "release-v3.0"))
_, err = generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "config")
assert.NoError(t, err)
t.Run("PushProtectedBranch-ProtectedFile", doGitPushTestRepositoryFail(dstPath, "origin", "release-v3.0"))
t.Run("CheckoutMaster-ProtectedFile", doGitCheckoutBranch(dstPath, "master"))
}
}
func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
return func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
@ -334,27 +364,23 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
ctx := NewAPITestContext(t, baseCtx.Username, baseCtx.Reponame, auth_model.AccessTokenScopeWriteRepository)
// Protect branch without any whitelisting
t.Run("ProtectBranchNoWhitelist", func(t *testing.T) {
doProtectBranch(ctx, "protected", "", "", "")
})
t.Run("ProtectBranchNoWhitelist", doProtectBranch(ctx, "protected", "", "", "", ""))
// Try to push without permissions, which should fail
t.Run("TryPushWithoutPermissions", func(t *testing.T) {
_, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-")
assert.NoError(t, err)
doGitPushTestRepositoryFail(dstPath, "origin", "protected")
doGitPushTestRepositoryFail(dstPath, "origin", "protected")(t)
})
// Set up permissions for normal push but not force push
t.Run("SetupNormalPushPermissions", func(t *testing.T) {
doProtectBranch(ctx, "protected", baseCtx.Username, "", "")
})
t.Run("SetupNormalPushPermissions", doProtectBranch(ctx, "protected", baseCtx.Username, "", "", ""))
// Normal push should work
t.Run("NormalPushWithPermissions", func(t *testing.T) {
_, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-")
assert.NoError(t, err)
doGitPushTestRepository(dstPath, "origin", "protected")
doGitPushTestRepository(dstPath, "origin", "protected")(t)
})
// Try to force push without force push permissions, which should fail
@ -364,30 +390,22 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
_, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-new")
assert.NoError(t, err)
})
doGitPushTestRepositoryFail(dstPath, "-f", "origin", "protected")
doGitPushTestRepositoryFail(dstPath, "-f", "origin", "protected")(t)
})
// Set up permissions for force push but not normal push
t.Run("SetupForcePushPermissions", func(t *testing.T) {
doProtectBranch(ctx, "protected", "", baseCtx.Username, "")
})
t.Run("SetupForcePushPermissions", doProtectBranch(ctx, "protected", "", baseCtx.Username, "", ""))
// Try to force push without normal push permissions, which should fail
t.Run("ForcePushWithoutNormalPermissions", func(t *testing.T) {
doGitPushTestRepositoryFail(dstPath, "-f", "origin", "protected")
})
t.Run("ForcePushWithoutNormalPermissions", doGitPushTestRepositoryFail(dstPath, "-f", "origin", "protected"))
// Set up permissions for normal and force push (both are required to force push)
t.Run("SetupNormalAndForcePushPermissions", func(t *testing.T) {
doProtectBranch(ctx, "protected", baseCtx.Username, baseCtx.Username, "")
})
t.Run("SetupNormalAndForcePushPermissions", doProtectBranch(ctx, "protected", baseCtx.Username, baseCtx.Username, "", ""))
// Force push should now work
t.Run("ForcePushWithPermissions", func(t *testing.T) {
doGitPushTestRepository(dstPath, "-f", "origin", "protected")
})
t.Run("ForcePushWithPermissions", doGitPushTestRepository(dstPath, "-f", "origin", "protected"))
t.Run("ProtectProtectedBranchNoWhitelist", doProtectBranch(ctx, "protected", "", "", ""))
t.Run("ProtectProtectedBranchNoWhitelist", doProtectBranch(ctx, "protected", "", "", "", ""))
t.Run("PushToUnprotectedBranch", doGitPushTestRepository(dstPath, "origin", "protected:unprotected"))
var pr api.PullRequest
var err error
@ -409,14 +427,14 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
t.Run("MergePR", doAPIMergePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index))
t.Run("PullProtected", doGitPull(dstPath, "origin", "protected"))
t.Run("ProtectProtectedBranchUnprotectedFilePaths", doProtectBranch(ctx, "protected", "", "", "unprotected-file-*"))
t.Run("ProtectProtectedBranchUnprotectedFilePaths", doProtectBranch(ctx, "protected", "", "", "unprotected-file-*", ""))
t.Run("GenerateCommit", func(t *testing.T) {
_, err := generateCommitWithNewData(testFileSizeSmall, dstPath, "user2@example.com", "User Two", "unprotected-file-")
assert.NoError(t, err)
})
t.Run("PushUnprotectedFilesToProtectedBranch", doGitPushTestRepository(dstPath, "origin", "protected"))
t.Run("ProtectProtectedBranchWhitelist", doProtectBranch(ctx, "protected", baseCtx.Username, "", ""))
t.Run("ProtectProtectedBranchWhitelist", doProtectBranch(ctx, "protected", baseCtx.Username, "", "", ""))
t.Run("CheckoutMaster", doGitCheckoutBranch(dstPath, "master"))
t.Run("CreateBranchForced", doGitCreateBranch(dstPath, "toforce"))
@ -431,7 +449,7 @@ func doBranchProtectPRMerge(baseCtx *APITestContext, dstPath string) func(t *tes
}
}
func doProtectBranch(ctx APITestContext, branch, userToWhitelistPush, userToWhitelistForcePush, unprotectedFilePatterns string) func(t *testing.T) {
func doProtectBranch(ctx APITestContext, branch, userToWhitelistPush, userToWhitelistForcePush, unprotectedFilePatterns, protectedFilePatterns string) func(t *testing.T) {
// We are going to just use the owner to set the protection.
return func(t *testing.T) {
csrf := GetUserCSRFToken(t, ctx.Session)
@ -440,6 +458,7 @@ func doProtectBranch(ctx APITestContext, branch, userToWhitelistPush, userToWhit
"_csrf": csrf,
"rule_name": branch,
"unprotected_file_patterns": unprotectedFilePatterns,
"protected_file_patterns": protectedFilePatterns,
}
if userToWhitelistPush != "" {
@ -465,7 +484,7 @@ func doProtectBranch(ctx APITestContext, branch, userToWhitelistPush, userToWhit
// Check if master branch has been locked successfully
flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.EqualValues(t, "success%3DBranch%2Bprotection%2Bfor%2Brule%2B%2522"+url.QueryEscape(branch)+"%2522%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value)
assert.EqualValues(t, "success%3DBranch%2Bprotection%2Bfor%2Brule%2B%2522"+url.QueryEscape(url.QueryEscape(branch))+"%2522%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value)
}
}