Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ filelock==3.13.1
# via copr
fnc==0.5.3
# via -r requirements.txt.in
gitdb==4.0.12
# via gitpython
gitpython==3.1.45
# via -r requirements.txt.in
gssapi==1.9.0
# via requests-gssapi
humanize==4.9.0
Expand Down Expand Up @@ -117,6 +121,8 @@ six==1.16.0
# via
# koji
# python-dateutil
smmap==5.0.2
# via gitdb
tft-cli==0.0.16
# via -r requirements.txt.in
typer[all]==0.7.0
Expand Down
1 change: 1 addition & 0 deletions requirements.txt.in
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ fnc
coverage==7.10.5
koji
pytest==8.4.1
GitPython
260 changes: 260 additions & 0 deletions scripts/bisect-with-snapshots.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
import argparse
import re
import subprocess
import tempfile
from typing import Self

import copr.v3
import dnf
import dnf.cli
import git


class CoprProject:
UNTESTED = 0
GOOD = 1
BAD = 2

def __init__(self, name: str):
self.name = name
self.index = -1
self._status = CoprProject.UNTESTED

def __lt__(self, other: Self) -> bool:
return self.name < other.name

@property
def commit(self) -> str:
return self._commit

@commit.setter
def commit(self, commit: str) -> None:
self._commit = commit

@property
def status(self) -> int:
return self._status

@status.setter
def status(self, status: int) -> None:
self._status = status
Comment on lines +13 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this usage of an IntEnum that is a subclass of int can help here to make the code more "readable". Also, if you make your CoprProject a dataclass you can define defaults and access the members. Usually a name: str = "default" is enough but since you explicitly want to only order by name we have to say which field participates in the comparison operators like le. I hope this makes sense.

Suggested change
class CoprProject:
UNTESTED = 0
GOOD = 1
BAD = 2
def __init__(self, name: str):
self.name = name
self.index = -1
self._status = CoprProject.UNTESTED
def __lt__(self, other: Self) -> bool:
return self.name < other.name
@property
def commit(self) -> str:
return self._commit
@commit.setter
def commit(self, commit: str) -> None:
self._commit = commit
@property
def status(self) -> int:
return self._status
@status.setter
def status(self, status: int) -> None:
self._status = status
import dataclasses
import enum
@enum.unique
class CoprProjectStatus(enum.IntEnum):
UNTESTED = 0
GOOD = 1
BAD = 2
@dataclasses.dataclass(kw_only=True, order=True)
class CoprProject:
name: str = field(compare=True)
index: int = field(compare=False, default=-1)
status: CoprProjectStatus = field(compare=False, default=CoprProjectStatus.UNTESTED)
commit: str | None = field(compare=False, default=None)



def get_snapshot_projects(chroot: str | None = None) -> list[CoprProject]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please put a comment here? Does the function get all snapshot projects for a given chroot ordered by name.

copr_client = copr.v3.Client.create_from_config_file()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please, know that we have a function for this here as well.

projects = []
for p in copr_client.project_proxy.get_list(ownername="@fedora-llvm-team"):
if not re.match(r"llvm-snapshots-big-merge-[0-9]+", p.name):
continue
if chroot and chroot not in list(p.chroot_repos.keys()):
continue
projects.append(CoprProject(p.name))
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you accept my suggestion on the dataclass above you're gonna have to make use of kw arguments here:

Suggested change
projects.append(CoprProject(p.name))
projects.append(CoprProject(name=p.name))

projects.sort()
for idx, p in enumerate(projects):
p.index = idx
return projects
Comment on lines +53 to +55
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need an explicit index here?

Copy link
Collaborator

Choose a reason for hiding this comment

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



def get_clang_commit_for_snapshot_project(project_name: str, chroot: str) -> str:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def get_clang_commit_for_snapshot_project(project_name: str, chroot: str) -> str:
def get_clang_commit_for_snapshot_project(project: CoprProject, chroot: str) -> str:

copr_client = copr.v3.Client.create_from_config_file()

builds = copr_client.build_proxy.get_list(
"@fedora-llvm-team", project_name, packagename="llvm", status="succeeded"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"@fedora-llvm-team", project_name, packagename="llvm", status="succeeded"
"@fedora-llvm-team", project.name, packagename="llvm", status="succeeded"

)
regex = re.compile("llvm-[0-9.]+~pre[0-9]+.g([0-9a-f]+)")
for b in builds:
if chroot in b["chroots"]:
print(b)
m = regex.search(b["source_package"]["url"])
if m:
return m.group(1)
raise Exception(f"Could not find commit for {project_name}, {chroot}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
raise Exception(f"Could not find commit for {project_name}, {chroot}")
raise Exception(f"Could not find commit for {project.name}, {chroot}")



def test_with_copr_builds(copr_project: str, test_command: str) -> bool:
rpms = {"llvm", "clang"}

print(f"Testing {copr_project}\n")
copr_fullname = f"@fedora-llvm-team/{copr_project}"
# Remove existing versions of clang and llvm
with dnf.Base() as base:
base.read_all_repos()
base.fill_sack()
for r in rpms:
try:
base.remove(r)
except dnf.exceptions.PackagesNotInstalledError:
pass
base.resolve(allow_erasing=True)
base.do_transaction()

# Enable the copr repo that we want to test.
# FIXME: There is probably some way to do this via the python API, but I
# can't figure it out.
subprocess.run(["dnf", "copr", "enable", "-y", copr_fullname])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try this:

base = dnf.Base()
base.read_all_repos()
base.fill_sack()

repos = base.repos.get_matching('copr:' + copr_project)
repos.enable()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This works, but you have to first download the .repo file from COPR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see the problem now. Calling dnf copr is a simpler API.

# Install clang and llvm builds to test
with dnf.Base() as base:
base.read_all_repos()
base.fill_sack()
for r in rpms:
base.install(r)
base.resolve(allow_erasing=True)
base.download_packages(base.transaction.install_set)
base.do_transaction()

# Disable project so future installs don't use it.
# FIXME: There is probably some way to do this via the python API, but I
# can't figure it out.
subprocess.run(["dnf", "copr", "disable", "-y", copr_fullname])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Likewise, but use repos.disable() instead.


print(test_command)
p = subprocess.run(test_command.split())
success = True if p.returncode == 0 else False
print("{} project".format("Good" if success else "Bad"))
return success


def git_bisect(
repo: git.Repo,
good_commit: str,
bad_commit: str,
configure_command: str,
build_command: str,
test_command: str,
) -> bool:
print(f"Running git bisect with {good_commit} and {bad_commit}")
print(configure_command)
print(build_command)
print(test_command)

# Configure llvm
subprocess.run(configure_command.split(), cwd=repo.working_tree_dir)

# Use subprocess.run here instead of builtin commands so we can stream output.
subprocess.run(
["git", "-C", repo.working_tree_dir, "bisect", "start", bad_commit, good_commit]
)
with tempfile.NamedTemporaryFile(mode="w+", delete=False) as bisect_script:
cmd = f"""
set -x
pwd
if ! {build_command}; then
exit 125
fi
{test_command}
"""
print(cmd)
bisect_script.write(cmd)
# Use the cwd argument instead of passing -C to git, so that the bisect script is
# run in the llvm-project directory.
subprocess.run(
["git", "bisect", "run", "/usr/bin/bash", bisect_script.name],
cwd=repo.working_tree_dir,
shell=True,
)
print(repo.git.bisect("log"))
return True


def main() -> bool:

parser = argparse.ArgumentParser()
parser.add_argument("--good-commit")
parser.add_argument("--bad-commit")
parser.add_argument("--llvm-project-dir")
parser.add_argument(
"--configure-command",
default="cmake -S llvm -G Ninja -B build -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=Native -DLLVM_ENABLE_PROJECTS=clang -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER_LAUNCHER=ccache",
)
parser.add_argument(
"--build-command",
default="ninja -C build install-clang install-clang-resource-headers install-LLVMgold install-llvm-ar install-llvm-ranlib",
)
parser.add_argument("--test-command")
parser.add_argument("--srpm")
parser.add_argument("--chroot")
args = parser.parse_args()

repo = git.Repo(args.llvm_project_dir)

chroot = args.chroot
projects = get_snapshot_projects()
good_project = None
bad_project = None

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest you create this helper function to eliminate the need to repeat all non-changing arguments. You only change the good/bad commit parts, right?

Suggested change
def git_bisect_helper(good_commit:str, bad_commit=str):
return git_bisect(
repo,
good_commit,
bad_commit,
args.configure_command,
args.build_command,
args.test_command,
)

# Find for the oldest COPR project that is newer than the good commit.
for p in projects:
try:
p.commit = get_clang_commit_for_snapshot_project(p.name, chroot)
repo.git.merge_base("--is-ancestor", args.good_commit, p.commit)
except Exception:
continue
print(p.commit, p.name, p.index, "/", len(projects))

if not test_with_copr_builds(p.name, args.test_command):
# The oldest commit was a 'bad' commit so we can use that as our
# 'bad' commit for bisecting.
return git_bisect(
repo,
args.good_commit,
p.commit,
args.configure_command,
args.build_command,
args.test_command,
)
Comment on lines +197 to +204
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return git_bisect(
repo,
args.good_commit,
p.commit,
args.configure_command,
args.build_command,
args.test_command,
)
return git_bisect_helper(good_commit=args.good_commit, bad_commit=p.commit)

good_project = p
break

# Find the newest COPR project that is older than the bad commit.
for p in reversed(projects):
try:
p.commit = get_clang_commit_for_snapshot_project(p.name, chroot)
repo.git.merge_base("--is-ancestor", p.commit, args.bad_commit)
except Exception:
continue
print(p.commit, p.name, p.index, "/", len(projects))

# We found a project, so test it.
if test_with_copr_builds(p.name, args.test_command):
# The newest commit was a 'good' commit, so we can use that as our
# good commit for testing.
return git_bisect(
repo,
p.commit,
args.bad_commit,
args.configure_command,
args.build_command,
args.test_command,
)
Comment on lines +221 to +228
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return git_bisect(
repo,
p.commit,
args.bad_commit,
args.configure_command,
args.build_command,
args.test_command,
)
return git_bisect_helper(good_commit=p.commit, bad_commit=args.bad_commit)

bad_project = p
break

# Bisect using copr builds
if good_project and bad_project:
while good_project.index + 1 < bad_project.index:
test_project = projects[(good_project.index + bad_project.index) // 2]
print(f"Testing: {test_project.name} - {test_project.commit}")
if test_with_copr_builds(test_project.name, args.test_command):
print("Good")
good_project = test_project
else:
print("Bad")
bad_project = test_project
if good_project:
args.good_commit = good_project.commit
if bad_project:
args.bad_commit = bad_project.commit

# Bisect the rest of the way using git.
return git_bisect(
repo,
args.good_commit,
args.bad_commit,
args.configure_command,
args.build_command,
args.test_command,
)
Comment on lines +249 to +256
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return git_bisect(
repo,
args.good_commit,
args.bad_commit,
args.configure_command,
args.build_command,
args.test_command,
)
return git_bisect_helper(good_commit=args.good_commit, bad_commit=args.bad_commit)



if __name__ == "__main__":
main()