Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support nixpkgs shallow clone and make use of github pull/<pr>/merge ref #426

Merged
merged 2 commits into from
Nov 5, 2024
Merged

Support nixpkgs shallow clone and make use of github pull/<pr>/merge ref #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b59557d
Support nixpkgs shallow clone and make use of github pull/<pr>/merge ref
azuwis Oct 27, 2024
a218ae9
tests: Fix tests after d2c0b50e25592f8abca8cc47314154934fde9ce7
azuwis Oct 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 29 additions & 14 deletions nixpkgs_review/review.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,13 @@ def git_merge(self, commit: str) -> None:
f"Failed to merge {commit} into {self.worktree_dir()}. git merge failed with exit code {res.returncode}"
)

def git_checkout(self, commit: str) -> None:
res = sh(["git", "checkout", commit], cwd=self.worktree_dir())
if res.returncode != 0:
raise NixpkgsReviewError(
f"Failed to checkout {commit} in {self.worktree_dir()}. git checkout failed with exit code {res.returncode}"
)

def apply_unstaged(self, staged: bool = False) -> None:
args = ["git", "--no-pager", "diff", "--no-ext-diff"]
args.extend(["--staged"] if staged else [])
Expand Down Expand Up @@ -191,6 +198,8 @@ def build_commit(

if reviewed_commit is None:
self.apply_unstaged(staged)
elif self.checkout == CheckoutOption.MERGE:
self.git_checkout(reviewed_commit)
else:
self.git_merge(reviewed_commit)

Expand Down Expand Up @@ -228,13 +237,6 @@ def git_worktree(self, commit: str) -> None:
f"Failed to add worktree for {commit} in {self.worktree_dir()}. git worktree failed with exit code {res.returncode}"
)

def checkout_pr(self, base_rev: str, pr_rev: str) -> None:
if self.checkout == CheckoutOption.MERGE:
self.git_worktree(base_rev)
self.git_merge(pr_rev)
else:
self.git_worktree(pr_rev)

def build(
self, packages_per_system: dict[System, set[str]], args: str
) -> dict[System, list[Attr]]:
Expand Down Expand Up @@ -272,15 +274,19 @@ def build_pr(self, pr_number: int) -> dict[System, list[Attr]]:
packages_per_system = self.github_client.get_borg_eval_gist(pr)
else:
packages_per_system = None
merge_rev, pr_rev = fetch_refs(
self.remote,
pr["base"]["ref"],
f"pull/{pr['number']}/head",
)

if self.checkout == CheckoutOption.MERGE:
base_rev = merge_rev
base_rev, pr_rev = fetch_refs(
self.remote,
pr["base"]["ref"],
f"pull/{pr['number']}/merge",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact there is a race condition with this ref sometimes as github needs some time to compute it. But this is probably only relevant for CIs. So we should be unlikely to when humans run nixpkgs-review (except for @SuperSandro2000 maybe)

)
else:
merge_rev, pr_rev = fetch_refs(
self.remote,
pr["base"]["ref"],
f"pull/{pr['number']}/head",
)
run = subprocess.run(
["git", "merge-base", merge_rev, pr_rev],
stdout=subprocess.PIPE,
Expand All @@ -295,7 +301,7 @@ def build_pr(self, pr_number: int) -> dict[System, list[Attr]]:
if packages_per_system is None:
return self.build_commit(base_rev, pr_rev)

self.checkout_pr(base_rev, pr_rev)
self.git_worktree(pr_rev)

for system in list(packages_per_system.keys()):
if system not in self.systems:
Expand Down Expand Up @@ -589,6 +595,15 @@ def filter_packages(

def fetch_refs(repo: str, *refs: str) -> list[str]:
cmd = ["git", "-c", "fetch.prune=false", "fetch", "--no-tags", "--force", repo]
shallow = subprocess.run(
["git", "rev-parse", "--is-shallow-repository"],
text=True,
stdout=subprocess.PIPE,
)
if shallow.returncode != 0:
raise NixpkgsReviewError(f"Failed to detect if {repo} is shallow repository")
if shallow.stdout.strip() == "true":
cmd.append("--depth=1")
for i, ref in enumerate(refs):
cmd.append(f"{ref}:refs/nixpkgs-review/{i}")
res = sh(cmd)
Expand Down
20 changes: 10 additions & 10 deletions tests/test_pr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ def test_pr_local_eval(helpers: Helpers, capfd: pytest.CaptureFixture) -> None:
f.write("foo")
subprocess.run(["git", "add", "."])
subprocess.run(["git", "commit", "-m", "example-change"])
subprocess.run(["git", "checkout", "-b", "pull/1/head"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/1/head"])
subprocess.run(["git", "checkout", "-b", "pull/1/merge"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/1/merge"])

path = main(
"nixpkgs-review",
Expand All @@ -54,8 +54,8 @@ def test_pr_local_eval_missing_nom(
f.write("foo")
subprocess.run(["git", "add", "."])
subprocess.run(["git", "commit", "-m", "example-change"])
subprocess.run(["git", "checkout", "-b", "pull/1/head"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/1/head"])
subprocess.run(["git", "checkout", "-b", "pull/1/merge"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/1/merge"])

path = main(
"nixpkgs-review",
Expand All @@ -82,8 +82,8 @@ def test_pr_local_eval_without_nom(
f.write("foo")
subprocess.run(["git", "add", "."])
subprocess.run(["git", "commit", "-m", "example-change"])
subprocess.run(["git", "checkout", "-b", "pull/1/head"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/1/head"])
subprocess.run(["git", "checkout", "-b", "pull/1/merge"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/1/merge"])

path = main(
"nixpkgs-review",
Expand All @@ -110,8 +110,8 @@ def test_pr_local_eval_with_sandbox(helpers: Helpers) -> None:
f.write("foo")
subprocess.run(["git", "add", "."])
subprocess.run(["git", "commit", "-m", "example-change"])
subprocess.run(["git", "checkout", "-b", "pull/1/head"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/1/head"])
subprocess.run(["git", "checkout", "-b", "pull/1/merge"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/1/merge"])

path = main(
"nixpkgs-review",
Expand All @@ -135,8 +135,8 @@ def test_pr_ofborg_eval(mock_urlopen: MagicMock, helpers: Helpers) -> None:
f.write("foo")
subprocess.run(["git", "add", "."])
subprocess.run(["git", "commit", "-m", "example-change"])
subprocess.run(["git", "checkout", "-b", "pull/37200/head"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/37200/head"])
subprocess.run(["git", "checkout", "-b", "pull/37200/merge"])
subprocess.run(["git", "push", str(nixpkgs.remote), "pull/37200/merge"])

mock_urlopen.side_effect = [
mock_open(read_data=helpers.read_asset("github-pull-37200.json"))(),
Expand Down