3 Commits

Author SHA256 Message Date
a8337693ce pr: use relative paths in new submodules
All checks were successful
go-generate-check / go-generate-check (pull_request) Successful in 23s
Integration tests / t (pull_request) Successful in 7m54s
2026-03-04 08:45:24 +01:00
45275cb004 direct: use relative paths when adding repositories and repocheck 2026-03-04 08:23:17 +01:00
d023627f42 common: Add relative path repository resolution helper 2026-03-04 08:23:09 +01:00
6 changed files with 179 additions and 117 deletions

View File

@@ -44,6 +44,42 @@ type NewRepos struct {
const maintainership_line = "MAINTAINER"
var true_lines []string = []string{"1", "TRUE", "YES", "OK", "T"}
var InvalidUrlError error = errors.New("PrjGit or PackageGit URLs cannot be empty.")
var AbsoluteUrlError error = errors.New("PrjGit or PackageGit URLs cannot be relative.")
var HostsNotEqualError error = errors.New("PrjGit or PackageGit are not the same hosts.")
var AbsoluteUrlWithQuery error = errors.New("PrjGit or PackageGit with query parameter. Unsupported.")
var InvalidPath error = errors.New("PrjGit or PackageGit path has unsupported format.")
func RelativeRepositoryPath(prjgit_org, packagegit string) (string, error) {
if len(packagegit) == 0 {
return "", InvalidUrlError
}
pkggiturl, err := url.Parse(packagegit)
if err != nil {
return "", err
}
if !pkggiturl.IsAbs() {
return "", AbsoluteUrlError
}
if len(pkggiturl.RawQuery) != 0 {
return "", AbsoluteUrlWithQuery
}
pkggitpath := SplitStringNoEmpty(pkggiturl.Path, "/")
if len(pkggitpath) != 2 {
return "", InvalidPath
}
pkggitpath[1] = strings.TrimSuffix(pkggitpath[1], ".git")
if prjgit_org == pkggitpath[0] {
return "../" + pkggitpath[1], nil
}
return "../../" + pkggitpath[0] + "/" + pkggitpath[1], nil
}
func HasSpace(s string) bool {
return strings.IndexFunc(s, unicode.IsSpace) >= 0

View File

@@ -8,6 +8,82 @@ import (
"src.opensuse.org/autogits/common"
)
func TestRelativeRepositoryPath(t *testing.T) {
tests := []struct {
name string
prjorg, repo string
hasError bool
relative string
}{
{
name: "Empty packagegit",
prjorg: "org1",
repo: "",
hasError: true,
},
{
name: "Invalid URL",
prjorg: "org1",
repo: ":",
hasError: true,
},
{
name: "Relative packagegit",
prjorg: "org1",
repo: "/path/to/repo",
hasError: true,
},
{
name: "Packagegit with query",
prjorg: "org1",
repo: "https://host/org1/repo?query=1",
hasError: true,
},
{
name: "Invalid path (too short)",
prjorg: "org1",
repo: "https://host/repo",
hasError: true,
},
{
name: "Invalid path (too long)",
prjorg: "org1",
repo: "https://host/org/repo/extra",
hasError: true,
},
{
name: "Same org",
prjorg: "org1",
repo: "https://host/org1/repo.git",
relative: "../repo",
},
{
name: "Different org",
prjorg: "org1",
repo: "https://host/org2/repo.git",
relative: "../../org2/repo",
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
r, err := common.RelativeRepositoryPath(test.prjorg, test.repo)
if err != nil && !test.hasError {
t.Error("Expected no error but have one", err)
}
if err == nil && test.hasError {
t.Error("Expected an error but had none. Returned:", r)
}
if err == nil && test.relative != r {
t.Error("Expected", test.relative, "but have", r)
}
})
}
}
func TestGitUrlParse(t *testing.T) {
tests := []struct {
name string

View File

@@ -137,108 +137,6 @@ index 0000000..e69de29
assert project_merged, f"Project PR myproducts/mySLFO#{project_pr_number} was not merged after 'merge ok'."
print("Both PRs merged successfully after 'merge ok'.")
@pytest.mark.t003
def test_003_refuse_manual_merge(manual_merge_env, test_user_client, ownerB_client, staging_bot_client):
"""
Test scenario TC-MERGE-003:
1. Create a PackageGit PR with ManualMergeOnly set to true.
2. Ensure all mandatory reviews are completed on both project and package PRs.
3. Comment "merge ok" on the package PR from the account of a not requested reviewer.
4. Verify the PR is not merged.
"""
gitea_env, test_full_repo_name, merge_branch_name = manual_merge_env
# 1. Create a package PR
diff = """diff --git a/manual_merge_test.txt b/manual_merge_test.txt
new file mode 100644
index 0000000..e69de29
"""
print(f"--- Creating package PR in mypool/pkgA on branch {merge_branch_name} ---")
package_pr = test_user_client.create_gitea_pr("mypool/pkgA", diff, "Test Manual Merge Fixture", False, base_branch=merge_branch_name)
package_pr_number = package_pr["number"]
print(f"Created package PR mypool/pkgA#{package_pr_number}")
# 2. Make sure the workflow-pr service created related project PR
project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number)
assert project_pr_number is not None, "Workflow bot did not create a project PR."
print(f"Found project PR: myproducts/mySLFO#{project_pr_number}")
# 3. Approve reviews and verify NOT merged
print("Waiting for all expected review requests and approving them...")
# Expected reviewers based on manual-merge branch config and pkgA maintainership
expected_reviewers = {"usera", "userb", "ownerA", "ownerX", "ownerY"}
# ManualMergeOnly still requires regular reviews to be satisfied.
# We poll until all expected reviewers are requested, then approve them.
all_requested = False
for _ in range(30):
# Trigger approvals for whatever is already requested
gitea_env.approve_requested_reviews("mypool/pkgA", package_pr_number)
gitea_env.approve_requested_reviews("myproducts/mySLFO", project_pr_number)
# Explicitly handle staging bot if it is requested or pending
prj_reviews = gitea_env.list_reviews("myproducts/mySLFO", project_pr_number)
if any(r["user"]["login"] == "autogits_obs_staging_bot" and r["state"] in ["REQUEST_REVIEW", "PENDING"] for r in prj_reviews):
print("Staging bot has a pending/requested review. Approving...")
staging_bot_client.create_review("myproducts/mySLFO", project_pr_number, event="APPROVED", body="Staging bot approves")
# Check if all expected reviewers have at least one review record (any state)
pkg_reviews = gitea_env.list_reviews("mypool/pkgA", package_pr_number)
current_reviewers = {r["user"]["login"] for r in pkg_reviews}
if expected_reviewers.issubset(current_reviewers):
# Also ensure they are all approved (not just requested)
approved_reviewers = {r["user"]["login"] for r in pkg_reviews if r["state"] == "APPROVED"}
if expected_reviewers.issubset(approved_reviewers):
# And check project PR for bot approval
prj_approved = any(r["user"]["login"] == "autogits_obs_staging_bot" and r["state"] == "APPROVED" for r in prj_reviews)
if prj_approved:
all_requested = True
print(f"All expected reviewers {expected_reviewers} and staging bot have approved.")
break
pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number)
prj_details = gitea_env.get_pr_details("myproducts/mySLFO", project_pr_number)
assert not pkg_details.get("merged"), "Package PR merged prematurely (ManualMergeOnly ignored?)"
assert not prj_details.get("merged"), "Project PR merged prematurely (ManualMergeOnly ignored?)"
time.sleep(2)
assert all_requested, f"Timed out waiting for all expected reviewers {expected_reviewers} to approve. Current: {current_reviewers}"
print("Both PRs have all required approvals but are not merged (as expected with ManualMergeOnly).")
# 4. Comment "merge ok" from a requested reviewer (usera)
print("Commenting 'merge ok' on package PR...")
ownerB_client.create_issue_comment("mypool/pkgA", package_pr_number, "merge ok")
# 5. Verify both PRs are merged
print("Polling for PR merge status...")
package_merged = False
project_merged = False
for i in range(20): # Poll for up to 20 seconds
if not package_merged:
pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number)
if pkg_details.get("merged"):
package_merged = True
print(f"Package PR mypool/pkgA#{package_pr_number} merged.")
if not project_merged:
prj_details = gitea_env.get_pr_details("myproducts/mySLFO", project_pr_number)
if prj_details.get("merged"):
project_merged = True
print(f"Project PR myproducts/mySLFO#{project_pr_number} merged.")
if package_merged and project_merged:
break
time.sleep(1)
assert not package_merged, f"Package PR mypool/pkgA#{package_pr_number} was merged after 'merge ok'."
assert not project_merged, f"Project PR myproducts/mySLFO#{project_pr_number} was merged after 'merge ok'."
print("Both PRs merged not after 'merge ok'.")
@pytest.mark.t008
def test_008_merge_mode_ff_only_success(merge_ff_env, test_user_client):
"""

View File

@@ -19,6 +19,7 @@ package main
*/
import (
"bytes"
"flag"
"fmt"
"io/fs"
@@ -123,7 +124,9 @@ func processConfiguredRepositoryAction(action *common.RepositoryWebhookEvent, co
common.LogError(" - ", action.Repository.Name, "repo is not sha256. Ignoring.")
return
}
common.PanicOnError(git.GitExec(gitPrj, "submodule", "--quiet", "add", "--force", "--depth", "1", action.Repository.Clone_Url, action.Repository.Name))
relpath, err := common.RelativeRepositoryPath(gitOrg, action.Repository.Clone_Url)
common.PanicOnError(err)
common.PanicOnError(git.GitExec(gitPrj, "submodule", "--quiet", "add", "--force", "--depth", "1", relpath, action.Repository.Name))
defer git.GitExecQuietOrPanic(gitPrj, "submodule", "deinit", "--all", "-f")
branch := strings.TrimSpace(git.GitExecWithOutputOrPanic(path.Join(gitPrj, action.Repository.Name), "branch", "--show-current"))
@@ -268,6 +271,12 @@ func verifyProjectState(git common.Git, org string, config *common.AutogitConfig
sub, err := git.GitSubmoduleList(gitPrj, "HEAD")
common.PanicOnError(err)
submodulesData, err := git.GitCatFile(gitPrj, "HEAD", ".gitmodules")
var submoduleEntries []common.Submodule
if err == nil {
submoduleEntries, _ = common.ParseSubmodulesFile(bytes.NewReader(submodulesData))
}
common.LogDebug(" * Getting package links")
var pkgLinks []*PackageRebaseLink
if f, err := fs.Stat(os.DirFS(path.Join(git.GetPath(), gitPrj)), common.PrjLinksFile); err == nil && (f.Mode()&fs.ModeType == 0) && f.Size() < 1000000 {
@@ -395,8 +404,27 @@ next_repo:
}
// }
relpath, err := common.RelativeRepositoryPath(gitOrg, r.CloneURL)
common.PanicOnError(err)
for repo := range sub {
if repo == r.Name {
// verify we are using relative repository paths, and if not, adjust them
sidx := slices.IndexFunc(submoduleEntries, func(s common.Submodule) bool {
if path.Base(s.Path) == r.Name {
return true
}
return false
})
if sidx >= 0 && submoduleEntries[sidx].Url != relpath {
submoduleEntries[sidx].Url = relpath
f, err := os.OpenFile(path.Join(git.GetPath(), ".gitmodules"), os.O_CREATE|os.O_TRUNC, 0o6400)
common.PanicOnError(err)
defer f.Close()
common.PanicOnError(common.WriteSubmodules(submoduleEntries, f))
isGitUpdated = true
}
// not missing
continue next_repo
}
@@ -420,7 +448,7 @@ next_repo:
}
// add repository to git project
common.PanicOnError(git.GitExec(gitPrj, "submodule", "--quiet", "add", "--force", "--depth", "1", r.CloneURL, r.Name))
common.PanicOnError(git.GitExec(gitPrj, "submodule", "--quiet", "add", "--force", "--depth", "1", relpath, r.Name))
curBranch := strings.TrimSpace(git.GitExecWithOutputOrPanic(path.Join(gitPrj, r.Name), "branch", "--show-current"))
if branch != curBranch {

View File

@@ -144,6 +144,7 @@ func (pr *PRProcessor) SetSubmodulesToMatchPRSet(prset *common.PRSet) error {
return err
}
PrjGitOrg, _, _ := prset.Config.GetPrjGit()
for _, pr := range prset.PRs {
if prset.IsPrjGitPR(pr.PR) {
continue
@@ -198,7 +199,12 @@ func (pr *PRProcessor) SetSubmodulesToMatchPRSet(prset *common.PRSet) error {
ref := fmt.Sprintf(common.PrPattern, org, repo, idx)
commitMsg := fmt.Sprintln("Add package", repo, "\n\nThis commit was autocreated by", GitAuthor, "\n\nreferencing PRs:\n", ref)
git.GitExecOrPanic(common.DefaultGitPrj, "submodule", "add", "-b", pr.PR.Base.Name, pr.PR.Base.Repo.SSHURL, repo)
relpath, err := common.RelativeRepositoryPath(PrjGitOrg, pr.PR.Base.Repo.CloneURL)
if err != nil {
common.LogError("Cannot calculate relative path for repository", pr.PR.Base.Repo.CloneURL, err)
return err
}
git.GitExecOrPanic(common.DefaultGitPrj, "submodule", "add", "-b", pr.PR.Base.Name, relpath, repo)
updateSubmoduleInPR(repo, prHead, git)
common.PanicOnError(git.GitExec(common.DefaultGitPrj, "commit", "-a", "-m", commitMsg))
@@ -482,11 +488,11 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error {
if _, ok := err.(*repository.RepoMergePullRequestConflict); !ok {
common.PanicOnError(err)
}
// } else {
// Gitea.AddComment(pr.PR, "Closing here because the associated Project PR has been closed.")
// Gitea.UpdatePullRequest(org, repo, idx, &models.EditPullRequestOption{
// State: "closed",
// })
// } else {
// Gitea.AddComment(pr.PR, "Closing here because the associated Project PR has been closed.")
// Gitea.UpdatePullRequest(org, repo, idx, &models.EditPullRequestOption{
// State: "closed",
// })
}
}
}

View File

@@ -28,8 +28,10 @@ func TestPrjGitDescription(t *testing.T) {
Base: &models.PRBranchInfo{
Ref: "main",
Repo: &models.Repository{
Name: "pkg-a",
Owner: &models.User{UserName: "test-org"},
Name: "pkg-a",
Owner: &models.User{UserName: "test-org"},
CloneURL: "http://example.com/test-org/pkg-a.git",
SSHURL: "git@example.com:test-org/pkg-a.git",
},
},
},
@@ -202,8 +204,10 @@ func TestSetSubmodulesToMatchPRSet(t *testing.T) {
Base: &models.PRBranchInfo{
Ref: "main",
Repo: &models.Repository{
Name: "pkg-a",
Owner: &models.User{UserName: "test-org"},
Name: "pkg-a",
Owner: &models.User{UserName: "test-org"},
CloneURL: "http://example.com/test-org/pkg-a.git",
SSHURL: "git@example.com:test-org/pkg-a.git",
},
},
Head: &models.PRBranchInfo{
@@ -630,7 +634,12 @@ func TestCreatePRjGitPR_Integration(t *testing.T) {
PR: &models.PullRequest{
State: "open",
Base: &models.PRBranchInfo{
Repo: &models.Repository{Name: "pkg-a", Owner: &models.User{UserName: "test-org"}},
Repo: &models.Repository{
Name: "pkg-a",
Owner: &models.User{UserName: "test-org"},
CloneURL: "http://example.com/test-org/pkg-a.git",
SSHURL: "git@example.com:test-org/pkg-a.git",
},
},
Head: &models.PRBranchInfo{Sha: "pkg-sha"},
},
@@ -653,14 +662,23 @@ func TestCreatePRjGitPR_Integration(t *testing.T) {
Base: &models.PRBranchInfo{
Name: "main",
RepoID: 1,
Repo: &models.Repository{Name: "test-prj", Owner: &models.User{UserName: "test-org"}},
Repo: &models.Repository{
Name: "test-prj",
Owner: &models.User{UserName: "test-org"},
CloneURL: "http://example.com/test-org/test-prj.git",
SSHURL: "git@example.com:test-org/test-prj.git",
},
},
Head: &models.PRBranchInfo{
Sha: "prj-head-sha",
},
}
gitea.EXPECT().GetRepository(gomock.Any(), gomock.Any()).Return(&models.Repository{Owner: &models.User{UserName: "test-org"}}, nil).AnyTimes()
gitea.EXPECT().GetRepository(gomock.Any(), gomock.Any()).Return(&models.Repository{
Owner: &models.User{UserName: "test-org"},
CloneURL: "http://example.com/test-org/test-prj.git",
SSHURL: "git@example.com:test-org/test-prj.git",
}, nil).AnyTimes()
// CreatePullRequestIfNotExist returns isNew=true
gitea.EXPECT().CreatePullRequestIfNotExist(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(prjPR, nil, true).AnyTimes()
// Expect SetLabels to be called for new PR