Merge pull request #448 from Mic92/improve-github-support

Improve GitHub support

.envrc

@@ -1,6 +1,6 @@
 # shellcheck shell=bash
-if has lorri; then
-  eval "$(lorri direnv)"
-else
-  use nix
+if ! has nix_direnv_version || ! nix_direnv_version 3.0.6; then
+  source_url "https://raw.githubusercontent.com/nix-community/nix-direnv/3.0.6/direnvrc" "sha256-RYcUJaRMf8oF5LznDrlCXbkOQrywm0HDv1VjYGaJGdM="
 fi
+
+use flake

README.md

@@ -238,21 +238,24 @@ As an alternative, one can also specify remote builder as usual in
 
 ## GitHub API token
 
-Some commands (i.e., `post-result` or `merge`) require a GitHub API token, and
-even for read-only calls, GitHub returns 403 error messages if your IP hits the
-rate limit for unauthenticated calls.
-
-Nixpkgs-review will automatically read the oauth_token stored by
-[hub](https://hub.github.com/) or [gh](https://cli.github.com/) if they are
-installed.
-
-Otherwise, you'll have to create a "personal access token (classic)" through
-GitHub's website. See [the GitHub documentation][3] for instructions. If you
-plan to post the generated reports, make sure to give it the `public_repo`
-scope.
-
-Then use either the `GITHUB_TOKEN` environment variable or the `--token`
-parameter of the `pr` subcommand to supply your token to nixpkgs-review.
+**Nixpkgs-review** requires a GitHub token to use cached evaluation results from
+GitHub and for certain commands (e.g., `post-result` or `merge`). Even for
+read-only operations, GitHub returns 403 error messages if your IP exceeds the
+rate limit for unauthenticated requests.
+
+**Automatic Token Usage** Nixpkgs-review will automatically use a GitHub token
+generated by [gh](https://cli.github.com/) (if installed). To set this up, run
+`gh auth login` once to log in.
+
+**Manual Token Creation** If you prefer to create a token manually, generate a
+"Personal Access Token (Classic)" through GitHub's website. Refer to
+[GitHub's documentation](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens)
+for detailed instructions. For posting generated reports, ensure the token is
+granted the `public_repo` scope.
+
+**Supplying the Token** You can provide your token to Nixpkgs-review using
+either the `GITHUB_TOKEN` environment variable or the `--token` parameter of the
+`pr` subcommand. Examples:
 
 ```console
 $ GITHUB_TOKEN=ghp_WAI7vpi9wVHbxPOA185NwWvaMawDuCnMGc3E nixpkgs-review pr 37244 --post-result

nixpkgs_review/cli/__init__.py

@@ -39,9 +39,9 @@ def pr_flags(
     pr_parser = subparsers.add_parser("pr", help="review a pull request on nixpkgs")
     pr_parser.add_argument(
         "--eval",
-        default="ofborg",
-        choices=["ofborg", "local"],
-        help="Whether to use ofborg's evaluation result",
+        default="auto",
+        choices=["auto", "github", "local", "ofborg"],  # ofborg is legacy
+        help="Whether to use github's evaluation result. Defaults to auto. Auto will use github if a github token is provided",
     )
     checkout_help = (
         "What to source checkout when building: "
@@ -139,18 +139,6 @@ def read_github_token() -> str | None:
     token = os.environ.get("GITHUB_OAUTH_TOKEN", os.environ.get("GITHUB_TOKEN"))
     if token:
         return token
-    try:
-        with hub_config_path().open() as f:
-            for line in f:
-                # Allow substring match as hub uses yaml. Example string we match:
-                # " - oauth_token: ghp_abcdefghijklmnopqrstuvwxyzABCDEF1234\n"
-                token_match = re.search(
-                    r"\s*oauth_token:\s+((?:gh[po]_)?[A-Za-z0-9]+)", line
-                )
-                if token_match:
-                    return token_match.group(1)
-    except OSError:
-        pass
     if which("gh"):
         r = subprocess.run(
             ["gh", "auth", "token"], stdout=subprocess.PIPE, text=True, check=False

nixpkgs_review/cli/pr.py

@@ -40,7 +40,24 @@ def parse_pr_numbers(number_args: list[str]) -> list[int]:
 
 def pr_command(args: argparse.Namespace) -> str:
     prs: list[int] = parse_pr_numbers(args.number)
-    use_ofborg_eval = args.eval == "ofborg"
+    match args.eval:
+        case "ofborg":
+            warn("Warning: `--eval=ofborg` is deprecated. Use `--eval=github` instead.")
+            args.eval = "github"
+        case "auto":
+            if args.token:
+                args.eval = "github"
+            else:
+                warn(
+                    "No GitHub token provided via GITHUB_TOKEN variable. Falling back to local evaluation.\n"
+                    "Tip: Install the `gh` command line tool and run `gh auth login` to authenticate."
+                )
+                args.eval = "local"
+        case "github":
+            if not args.token:
+                warn("No GitHub token provided")
+                sys.exit(1)
+    use_github_eval = args.eval == "github"
     checkout_option = (
         CheckoutOption.MERGE if args.checkout == "merge" else CheckoutOption.COMMIT
     )
@@ -80,7 +97,7 @@ def pr_command(args: argparse.Namespace) -> str:
                     run=args.run,
                     remote=args.remote,
                     api_token=args.token,
-                    use_ofborg_eval=use_ofborg_eval,
+                    use_github_eval=use_github_eval,
                     only_packages=set(args.package),
                     package_regexes=args.package_regex,
                     skip_packages=set(args.skip_package),

nixpkgs_review/github.py

@@ -5,12 +5,11 @@
 import urllib.parse
 import urllib.request
 import zipfile
-from collections import defaultdict
 from http.client import HTTPMessage
 from pathlib import Path
 from typing import IO, Any, override
 
-from .utils import System
+from .utils import System, warn
 
 
 def pr_url(pr: int) -> str:
@@ -176,46 +175,32 @@ def get_github_action_eval_result(
                 workflow_run["artifacts_url"],
             )["artifacts"]
 
+            found_comparison = False
             for artifact in artifacts:
                 if artifact["name"] != "comparison":
                     continue
+                found_comparison = True
                 changed_paths: Any = self.get_json_from_artifact(
                     workflow_id=artifact["id"],
                     json_filename="changed-paths.json",
                 )
                 if changed_paths is None:
+                    warn(
+                        f"Found comparison artifact, but no changed-paths.json in workflow {workflow_run['html_url']}"
+                    )
                     continue
                 if (path := changed_paths.get("rebuildsByPlatform")) is not None:
                     assert isinstance(path, dict)
                     return path
-        return None
-
-    def get_borg_eval_gist(self, pr: dict[str, Any]) -> dict[System, set[str]] | None:
-        packages_per_system: defaultdict[System, set[str]] = defaultdict(set)
-        statuses = self.get(pr["statuses_url"])
-        for status in statuses:
-            if (
-                status["description"] == "^.^!"
-                and status["state"] == "success"
-                and status["context"] == "ofborg-eval"
-                and status["creator"]["login"] == "ofborg[bot]"
-            ):
-                url = status.get("target_url", "")
-                if url == "":
-                    return packages_per_system
-
-                url = urllib.parse.urlparse(url)
-                gist_hash = url.path.split("/")[-1]
-                raw_gist_url = (
-                    f"https://gist.githubusercontent.com/GrahamcOfBorg/{gist_hash}/raw/"
-                )
 
-                with urllib.request.urlopen(raw_gist_url) as resp:  # noqa: S310
-                    for line in resp:
-                        if line == b"":
-                            break
-                        system, attribute = line.decode("utf-8").split()
-                        packages_per_system[system].add(attribute)
+            if not found_comparison:
+                if workflow_run["status"] == "queued":
+                    warn(
+                        f"Found eval workflow run, but evaluation is still work in progress: {workflow_run['html_url']}"
+                    )
+                else:
+                    warn(
+                        f"Found eval workflow run, but no comparison artifact in {workflow_run['html_url']}."
+                    )
 
-                return packages_per_system
         return None

nixpkgs_review/review.py

@@ -98,7 +98,7 @@ def __init__(
         nixpkgs_config: Path,
         extra_nixpkgs_config: str,
         api_token: str | None = None,
-        use_ofborg_eval: bool | None = True,
+        use_github_eval: bool | None = True,
         only_packages: set[str] | None = None,
         package_regexes: list[Pattern[str]] | None = None,
         skip_packages: set[str] | None = None,
@@ -122,7 +122,7 @@ def __init__(
         self.run = run
         self.remote = remote
         self.github_client = GithubClient(api_token)
-        self.use_ofborg_eval = use_ofborg_eval
+        self.use_github_eval = use_github_eval
         self.checkout = checkout
         self.only_packages = only_packages
         self.package_regex = package_regexes
@@ -296,16 +296,11 @@ def build_pr(self, pr_number: int) -> dict[System, list[Attr]]:
         pr = self.github_client.pull_request(pr_number)
 
         packages_per_system: dict[System, set[str]] | None = None
-        if self.use_ofborg_eval and all(system in PLATFORMS for system in self.systems):
+        if self.use_github_eval and all(system in PLATFORMS for system in self.systems):
             # Attempt to fetch the GitHub actions evaluation result
             print("-> Attempting to fetch eval results from GitHub actions")
             packages_per_system = self.github_client.get_github_action_eval_result(pr)
 
-            # If unsuccessfull, fallback to ofborg
-            if packages_per_system is None:
-                print("-> Unsuccessfull: Trying out legacy ofborg")
-                packages_per_system = self.github_client.get_borg_eval_gist(pr)
-
             if packages_per_system is not None:
                 print("-> Successfully fetched rebuilds: no local evaluation needed")