diff --git a/scripts/update-dashboard.sh b/scripts/update-dashboard.sh index 10cbe5b..a0070f5 100755 --- a/scripts/update-dashboard.sh +++ b/scripts/update-dashboard.sh @@ -1,8 +1,8 @@ source $HOME/.ci-auth skare3-dashboard -o /proj/sot/ska/jgonzalez/index.html skare3-dashboard -o /proj/sot/ska/jgonzalez/packages.json -skare3-test-dashboard -o /proj/sot/ska/jgonzalez/test_results.html --static-dir https://cxc.cfa.harvard.edu/mta/ASPECT/skare3/dashboard/static --log-dir https://icxc.cfa.harvard.edu/aspect/skare3/dashboard/tests -skare3-test-dashboard -o /proj/sot/ska/jgonzalez/test_results.json +skare3-test-dashboard -b -o /proj/sot/ska/jgonzalez/test_results.html --static-dir https://cxc.cfa.harvard.edu/mta/ASPECT/skare3/dashboard/static --log-dir https://icxc.cfa.harvard.edu/aspect/skare3/dashboard/tests +skare3-test-dashboard -b -o /proj/sot/ska/jgonzalez/test_results.json # horrible hack: rm -f /proj/sot/ska/www/ASPECT_ICXC/skare3/dashboard/tests diff --git a/skare3_tools/github/github.py b/skare3_tools/github/github.py index 710bfb5..ecaafe6 100644 --- a/skare3_tools/github/github.py +++ b/skare3_tools/github/github.py @@ -393,7 +393,7 @@ def _get_list_generator(self, url, limit=None, **kwargs): yield item count += 1 if limit and count >= limit: - raise StopIteration() + return def _get_list(self, *args, **kwargs): """ @@ -436,6 +436,7 @@ def __init__(self, repo=None, owner=None, api=None): self.branches = Branches(self) self.checks = Checks(self) self.pull_requests = PullRequests(self) + self.compare = Compare(self) self.merge = Merge(self) self.dispatch_event = DispatchEvent(self) self.contents = Contents(self) @@ -615,7 +616,7 @@ def __call__(self, ref=None, **kwargs): json.update({k: kwargs[k] for k in optional if k in kwargs}) kwargs = {k: v for k, v in kwargs.items() if k not in json} if ref is not None: - return self._get( + return self._get_list( "repos/:owner/:repo/commits/:ref", ref=ref, params=json, **kwargs ) return self._get_list("repos/:owner/:repo/commits", params=json, **kwargs) @@ -778,6 +779,24 @@ def edit(self, issue_number, **kwargs): ) +class Compare(_EndpointGroup): + """Compare two commits + + (`compare API docs `) + """ + + def __call__(self, base, head, **kwargs): + """ """ + required = [] + json = {k: kwargs[k] for k in required} + kwargs = {k: v for k, v in kwargs.items() if k not in json} + return self._get( + "/repos/:owner/:repo/compare/:basehead", + basehead=f"{base}...{head}", + **kwargs, + ) + + class PullRequests(_EndpointGroup): """ Endpoints that have to do with pull requests @@ -833,7 +852,7 @@ def __call__(self, pull_number=None, **kwargs): json = {k: kwargs[k] for k in required} json.update({k: kwargs[k] for k in optional if k in kwargs}) kwargs = {k: v for k, v in kwargs.items() if k not in json} - return self._get("/repos/:owner/:repo/pulls", params=json, **kwargs) + return self._get_list("/repos/:owner/:repo/pulls", params=json, **kwargs) def create(self, **kwargs): """ diff --git a/skare3_tools/github/scripts/add_secrets.py b/skare3_tools/github/scripts/add_secrets.py index 3820822..ecb6d05 100755 --- a/skare3_tools/github/scripts/add_secrets.py +++ b/skare3_tools/github/scripts/add_secrets.py @@ -153,7 +153,7 @@ def add_secrets(repository, secrets): _driver_.find_element_by_id("secret_name").send_keys(secret) value = secrets[secret] - if type(value) == dict: + if type(value) is dict: value = json.dumps(value) _driver_.find_element_by_id("secret_value").send_keys(value) diff --git a/skare3_tools/github/scripts/merge_pr.py b/skare3_tools/github/scripts/merge_pr.py index 2d7f2be..1d7f1c5 100755 --- a/skare3_tools/github/scripts/merge_pr.py +++ b/skare3_tools/github/scripts/merge_pr.py @@ -46,7 +46,7 @@ def main(): kwargs["state"] = "open" prs = repository.pull_requests(**kwargs) - if type(prs) == dict and not prs["response"]["ok"]: + if type(prs) is dict and not prs["response"]["ok"]: print(f'Failed getting requested PR: {prs["response"]["reason"]}') sys.exit(1) diff --git a/skare3_tools/github/scripts/release_merge_info.py b/skare3_tools/github/scripts/release_merge_info.py index 2320a73..4b07193 100755 --- a/skare3_tools/github/scripts/release_merge_info.py +++ b/skare3_tools/github/scripts/release_merge_info.py @@ -11,88 +11,128 @@ import argparse import re import sys +import numpy as np +from packaging.version import Version from skare3_tools import github, packages -def parser(): +def get_parser(): parse = argparse.ArgumentParser(description=__doc__) parse.add_argument( "--repository", required=True, help="repository name. Example: sot/chandra_aca" ) parse.add_argument("--sha", help="sha of the release") + parse.add_argument("--tag", help="tag of the release") parse.add_argument( "--token", "-t", help="Github token, or name of file that contains token" ) + parse.add_argument( + "--stdout", + action="store_true", + help="print to stdout instead of editing release description", + ) return parse -def main(): - args = parser().parse_args() - github.init(token=args.token) - repository = github.Repository(args.repository) - - # get all releases and find the one we are working on - releases = repository.releases() - releases = [r for r in releases if not r["draft"] and not r["prerelease"]] - releases = sorted(releases, key=lambda r: r["tag_name"], reverse=True) - release_commits = [ - packages._get_release_commit(repository, r["tag_name"]) for r in releases - ] - release_shas = [c["sha"] for c in release_commits] - i_1 = None - for i, sha in enumerate(release_shas): - if sha == args.sha: - i_1 = i - if i_1 is None: - raise Exception(f"Release with sha {args.sha} was not found") - - # get all commits between this release and the previous one, if any. - kwargs = { - "until": repository.commits(ref=release_shas[i_1])["commit"]["author"]["date"] - } - if i_1 + 1 < len(releases): - kwargs["since"] = repository.commits(ref=release_shas[i_1 + 1])["commit"][ - "author" - ]["date"] - commits = repository.commits(sha="master", **kwargs)[ - :-1 - ] # remove the last one, the release +def merges_in_range(repo, sha_1, sha_2): + commits_1 = repo.commits(sha=sha_1) + commits_2 = repo.commits(sha=sha_2) + sha_1 = [c["sha"] for c in commits_1] + sha_2 = [c["sha"] for c in commits_2] + i = np.argwhere(np.in1d(sha_2, sha_1)).flatten()[0] + commits = commits_2[:i] + assert len(commits) # TODO: shouldn't it be possible with no commits? # get commit messages matching the standard merge commit merges = [] for commit in commits: msg = commit["commit"]["message"] match = re.match( - r"Merge pull request (?P.+) from (?P\S+)\n\n(?P.+)", + r"Merge pull request (?P.+) from (?P\S+)(\n\n(?P.+))?", msg, ) if match: msg = match.groupdict() - if msg["pr"][0] == "#": - msg["pr"] = ( - f'[{msg["pr"]}]' - f'(https://github.com/{args.repository}/pull/{msg["pr"][1:]})' - ) - merges.append(f'PR {msg["pr"]}: {msg["description"]}') - if merges: + if msg["description"] is None: + msg["description"] = repo.pull_requests(msg["pr"][1:])[0]["title"] + merges.append(msg) + return merges + + +def main(): + parser = get_parser() + args = parser.parse_args() + + if not args.tag and not args.sha: + parser.exit("Need to specify tag or sha") + + github.init(token=args.token) + + repo = github.Repository(args.repository) + + # get all releases and their commit sha + releases = repo.releases() + releases = [r for r in releases if not r["draft"] and not r["prerelease"]] + releases = sorted(releases, key=lambda r: Version(r["tag_name"]), reverse=True) + releases = {r["tag_name"]: r for r in releases} + for tag, rel in releases.items(): + releases[tag]["sha"] = packages._get_release_commit(repo, rel["tag_name"])[ + "sha" + ] + + release_shas = [rel["sha"] for rel in releases.values()] + releases_by_sha = {rel["sha"]: rel for rel in releases.values()} + + # normalize and check arguments + if args.tag and not args.sha: + args.sha = releases[args.tag]["sha"] + elif args.sha and not args.tag: + args.tag = releases_by_sha[args.sha]["tag_name"] + + assert args.sha in release_shas, f"Release with sha {args.sha} was not found" + assert args.tag in releases, f"Release with tag {args.tag} was not found" + assert releases[args.tag]["sha"] == args.sha, f"Inconsistent release sha and tag" + + # now find all merges between the previous release and the requested one + # checking for commit messages matching the standard merge commit + merges = merges_in_range( + repo, + release_shas[release_shas.index(args.sha) + 1], # TODO: limit check here? + args.sha, + ) + + msgs = [] + for merge in merges: + if merge["pr"][0] == "#": + merge["pr"] = ( + f'[{merge["pr"]}]' + f'(https://github.com/{args.repository}/pull/{merge["pr"][1:]})' + ) + msgs.append(f'PR {merge["pr"]}: {merge["description"]}') + + if msgs: # edit the release to include the merge information - release_id = releases[i_1]["id"] - body = releases[i_1]["body"] + release = releases[args.tag] + release_id = release["id"] + body = release["body"] if body: body += "\n\n" body += "Includes the following merges:\n" - for merge in merges: - body += f"- {merge}\n" - - r = repository.releases.edit(release_id, body=body) - if not r["response"]["ok"]: - sys.exit( - ( - f"Failed to edit release '{releases[i_1]['name']}'" - f" ({release_id}): {r['response']['reason']}" + for msg in msgs: + body += f"- {msg}\n" + + if args.stdout: + print(body) + else: + r = repo.releases.edit(release_id, body=body) + if not r["response"]["ok"]: + sys.exit( + ( + f"Failed to edit release '{release['name']}'" + f" ({release_id}): {r['response']['reason']}" + ) ) - ) if __name__ == "__main__": diff --git a/skare3_tools/packages.py b/skare3_tools/packages.py index 4a2354f..7ba661e 100755 --- a/skare3_tools/packages.py +++ b/skare3_tools/packages.py @@ -84,6 +84,7 @@ import requests import yaml +from packaging.version import Version, InvalidVersion from skare3_tools import github from skare3_tools.config import CONFIG @@ -339,6 +340,38 @@ def _get_tag_target(tag): """ +_COMPARE_COMMITS_QUERY = """ +{ + repository(name: "{{ name }}", owner: "{{ owner }}") { + ref(qualifiedName: "{{ base }}") { + compare(headRef: "{{ head }}") { + aheadBy + behindBy + commits(first: 100, after: "{{ after }}") { + nodes { + oid + message + pushedDate + author { + user { + login + } + } + } + pageInfo { + hasPreviousPage + hasNextPage + startCursor + endCursor + } + } + } + } + } +} +""" + + _COMMIT_QUERY = """ { repository(name: "{{ name }}", owner: "{{ owner }}") { @@ -373,6 +406,88 @@ def _get_tag_target(tag): """ +class Dict(dict): + def __getitem__(self, i): + if i in self.keys(): + return super().__getitem__(i) + return self.node(self, i) + + @staticmethod + def _node(root, path): + if path: + return Dict._node(root[path[0]], path[1:]) + return root + + @staticmethod + def node(root, path): + path = path.split("/") + return Dict._node(root, path) + + +def get_all_nodes( + owner, name, path, query, query_2=None, at="", reverse=False, **kwargs +): + if reverse: + cursor = "startCursor" + has_more = "hasPreviousPage" + else: + cursor = "endCursor" + has_more = "hasNextPage" + data = Dict( + github.GITHUB_API_V4( + jinja2.Template(query).render(name=name, owner=owner, cursor=at, **kwargs) + ) + ) + check_api_errors(data) + commits = data[path]["nodes"] + if query_2 is None: + query_2 = query + while data[path]["pageInfo"][has_more]: + at = data[path]["pageInfo"][cursor] + data = Dict( + github.GITHUB_API_V4( + jinja2.Template(query_2).render( + name=name, owner=owner, cursor=at, **kwargs + ) + ) + ) + check_api_errors(data) + commits += data[path]["nodes"] + return commits + + +def check_api_errors(data): + if "errors" in data: + try: + msg = "\n".join([e["message"] for e in data["errors"]]) + except Exception: + raise Exception(str(data["errors"])) + raise Exception(msg) + + +def _pr_commits(commits, all_pull_requests, use_pr_titles=True): + merges = [] + for commit in commits: + match = re.match( + r"Merge pull request #(?P.+) from (?P\S+)(\n\n(?P.+))?", + commit["message"], + ) + if match: + merge = match.groupdict() + merge["pr_number"] = int(merge["pr_number"]) + merges.append(merge) + + for merge in merges: + merge["author"] = "Unknown" + if merge["pr_number"] in all_pull_requests: + merge["author"] = all_pull_requests[merge["pr_number"]]["author"]["name"] + if use_pr_titles or merge["title"] is None: + # some times PR titles are changed after merging. Use that instead of the commit + merge["title"] = all_pull_requests[merge["pr_number"]]["title"] + + return merges + + def _get_repository_info_v4( owner_repo, since=7, @@ -382,8 +497,8 @@ def _get_repository_info_v4( ): owner, name = owner_repo.split("/") api = github.GITHUB_API_V4 - data_v4 = api( - jinja2.Template(github.graphql.REPO_QUERY).render(name=name, owner=owner) + data_v4 = Dict( + api(jinja2.Template(github.graphql.REPO_QUERY).render(name=name, owner=owner)) ) if "errors" in data_v4: try: @@ -394,55 +509,36 @@ def _get_repository_info_v4( branches = [ n - for n in data_v4["data"]["repository"]["refs"]["nodes"] + for n in data_v4["data/repository/refs/nodes"] if re.match("heads/", n["name"]) ] - releases = data_v4["data"]["repository"]["releases"]["nodes"] - commits = data_v4["data"]["repository"]["defaultBranchRef"]["target"]["history"][ - "nodes" - ] - issues = data_v4["data"]["repository"]["issues"]["nodes"] - default_branch = data_v4["data"]["repository"]["defaultBranchRef"]["name"] - - commit_data = data_v4 - while commit_data["data"]["repository"]["defaultBranchRef"]["target"]["history"][ - "pageInfo" - ]["hasNextPage"]: - cursor = commit_data["data"]["repository"]["defaultBranchRef"]["target"][ - "history" - ]["pageInfo"]["endCursor"] - commit_data = api( - jinja2.Template(_COMMIT_QUERY).render(name=name, owner=owner, cursor=cursor) - ) - commits += commit_data["data"]["repository"]["defaultBranchRef"]["target"][ - "history" - ]["nodes"] - - pr_data = data_v4 - pull_requests = pr_data["data"]["repository"]["pullRequests"]["nodes"] - while pr_data["data"]["repository"]["pullRequests"]["pageInfo"]["hasPreviousPage"]: - cursor = pr_data["data"]["repository"]["pullRequests"]["pageInfo"][ - "startCursor" - ] - pr_data = api( - jinja2.Template(_PR_QUERY).render(name=name, owner=owner, cursor=cursor) - ) - pull_requests += pr_data["data"]["repository"]["pullRequests"]["nodes"] - - releases = [r for r in releases if not r["isPrerelease"] and not r["isDraft"]] - for r in releases: - r["tag_oid"], r["committed_date"] = _get_tag_target(r["tag"]) + releases = data_v4["data/repository/releases/nodes"] + issues = data_v4["data/repository/issues/nodes"] + default_branch = data_v4["data/repository/defaultBranchRef/name"] + + commits_path = "data/repository/defaultBranchRef/target/history" + commits = data_v4[commits_path]["nodes"] + commits += get_all_nodes( + owner, + name, + commits_path, + _COMMIT_QUERY, + reverse=False, + at=data_v4[commits_path]["pageInfo"]["endCursor"], + ) - release_info = [ - { - "release_tag": "", - "release_tag_date": "", - "release_commit_date": datetime.datetime.now().isoformat(), - "commits": [], - "merges": [], - } - ] + pull_requests_path = "data/repository/pullRequests" + pull_requests = data_v4[pull_requests_path]["nodes"] + pull_requests += get_all_nodes( + owner, + name, + pull_requests_path, + _PR_QUERY, + reverse=True, + at=data_v4[pull_requests_path]["pageInfo"]["startCursor"], + ) + # from now, keep a list of the open pull requests on the main branch all_pull_requests = {pr["number"]: pr for pr in pull_requests} pull_requests = [ pr @@ -463,71 +559,29 @@ def _get_repository_info_v4( ] pull_requests = sorted(pull_requests, key=lambda pr: pr["number"], reverse=True) - for commit in commits: - sha = commit["oid"] - releases_at_commit = [ - { - "release_sha": release["tag_oid"], - "release_commit_date": release["committed_date"], - "release_tag": release["tagName"], - "release_tag_date": release["publishedAt"], - "commits": [], - "merges": [], - } - for release in releases - if release["tag_oid"] == sha - ] - release_info += releases_at_commit - - release_info[-1]["commits"].append(commit) - match = re.match( - r"Merge pull request #(?P<pr_number>.+) from (?P<branch>\S+)\n\n(?P<title>.+)", - commit["message"], - ) - if match: - merge = match.groupdict() - merge["pr_number"] = int(merge["pr_number"]) - # It is possible that a commit says "Merge pull request #..." without an actual PR. - # One such case is commits before a fork, in which case one has to do more digging to - # get the PR author or title. We do not care and set the author as Unknown. - merge["author"] = "Unknown" - if merge["pr_number"] in all_pull_requests: - merge["author"] = all_pull_requests[merge["pr_number"]]["author"][ - "name" - ] - if use_pr_titles: - # some times PR titles are changed after merging. Use that instead of the commit - merge["title"] = all_pull_requests[merge["pr_number"]][ - "title" - ] # .strip() - release_info[-1]["merges"].append(merge) - - # up to now, we followed the default branch commits, collecting all releases along the branch. - # Now we will add the remaining releases, which presumably happened in another branch. - - release_shas = [r["release_sha"] for r in release_info[1:]] - for release in releases: - if release["tag_oid"] not in release_shas: - release_info.append( - { - "release_sha": release["tag_oid"], - "release_tag": release["tagName"], - "release_tag_date": release["publishedAt"], - "release_commit_date": release["committed_date"], - "commits": [], - "merges": [], - } + # get release info since "since", excluding drafts, pre-releases, invalid versions + releases = [r for r in releases if not r["isPrerelease"] and not r["isDraft"]] + exclude = [] + for rel in releases: + rel["tag_oid"], rel["committed_date"] = _get_tag_target(rel["tag"]) + try: + Version(rel["tagName"]) + except InvalidVersion: + logging.debug( + f"{owner_repo} release {rel['tagName']} does not conform to PEP 440. " + "It will be ignored" ) + exclude += [rel["tagName"]] + releases = [r for r in releases if r["tagName"] not in exclude] + releases = sorted(releases, key=lambda r: Version(r["tagName"]), reverse=True) - release_info = sorted( - release_info, key=lambda r: r["release_commit_date"], reverse=True - ) - - release_tags = [r["release_tag"] for r in release_info] + release_tags = [r["tagName"] for r in releases] if type(since) is int: - release_info = release_info[: since + 1] + # keeping the last "since" releases, plus the current main branch + releases = releases[: since + 1] elif since in release_tags: - release_info = release_info[: release_tags.index(since)] + # keeping up to the "since" tag (inclusive), plus the current main branch + releases = releases[: release_tags.index(since) + 2] elif since is not None: raise Exception( "Requested repository info with since={since},".format(since=since) @@ -535,6 +589,54 @@ def _get_repository_info_v4( + "({release_tags})".format(release_tags=release_tags) ) + rel_commits = get_all_nodes( + owner, + name, + "data/repository/ref/compare/commits", + _COMPARE_COMMITS_QUERY, + reverse=False, + base=releases[0]["tagName"], + head=default_branch, + ) + rel_prs = _pr_commits(rel_commits, all_pull_requests, use_pr_titles=use_pr_titles) + release_info = [ + { + "release_tag": "", + "release_tag_date": "", + "release_commit_date": datetime.datetime.now().isoformat(), + "commits": [], + "merges": rel_prs, + } + ] + + for base, head in zip(releases[1:], releases[:-1]): + rel_commits = get_all_nodes( + owner, + name, + "data/repository/ref/compare/commits", + _COMPARE_COMMITS_QUERY, + reverse=False, + base=base["tagName"], + head=head["tagName"], + ) + rel_prs = _pr_commits( + rel_commits, all_pull_requests, use_pr_titles=use_pr_titles + ) + release = { + "release_sha": head["tag_oid"], + "release_commit_date": head["committed_date"], + "release_tag": head["tagName"], + "release_tag_date": head["publishedAt"], + "commits": [], + "merges": rel_prs, + } + release_info.append(release) + + # the first entry in the list is not a release, but the current main branch + release_info = release_info[:1] + sorted( + release_info[1:], key=lambda r: Version(r["release_tag"]), reverse=True + ) + if len(release_info) > 1: last_tag = release_info[1]["release_tag"] last_tag_date = release_info[1]["release_tag_date"] @@ -542,7 +644,7 @@ def _get_repository_info_v4( last_tag = "" last_tag_date = "" - # workflows are only in v4 + # workflows are only in v3 headers = {"Accept": "application/vnd.github.antiope-preview+json"} workflows = github.GITHUB_API_V3.get( "/repos/{owner}/{name}/actions/workflows".format(owner=owner, name=name), @@ -716,7 +818,9 @@ def _get_repository_info_v3( include_commits=False, ): """ - Get information about a Github repository + Get information about a Github repository. + + This uses Github API v3. This function is DEPRECATED, use v4 instead. :param owner_repo: str the name of the repository, including owner, something like 'sot/skare3'. @@ -911,11 +1015,6 @@ def repository_info_is_outdated(_, pkg_info): return outdated -@json_cache( - "pkg_repository_info", - directory="pkg_info", - update_policy=repository_info_is_outdated, -) def get_repository_info(owner_repo, version="v4", **kwargs): """ Get information about a Github repository @@ -940,6 +1039,18 @@ def get_repository_info(owner_repo, version="v4", **kwargs): Force update of the cached info. By default updates only if pushed_at or updated_at change. :return: """ + # the indirect call is to make sure the version argument is set at this point + # otherwise, there are two caches if the version is explicitly set to the default value + # (one where it is set and one where it is not) + return _get_repository_info(owner_repo, version, **kwargs) + + +@json_cache( + "pkg_repository_info", + directory="pkg_info", + update_policy=repository_info_is_outdated, +) +def _get_repository_info(owner_repo, version, **kwargs): owner, name = owner_repo.split("/") if version == "v4": @@ -955,6 +1066,10 @@ def get_repository_info(owner_repo, version="v4", **kwargs): return info +get_repository_info.clear_cache = _get_repository_info.clear_cache +get_repository_info.rm_cache_entry = _get_repository_info.rm_cache_entry + + def get_repositories_info(repositories=None, version="v4", update=False): if repositories is None: repositories = [