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.

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

Improve GitHub support #448

Merged
merged 3 commits into from
Jan 2, 2025
Merged
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
8 changes: 4 additions & 4 deletions .envrc
Original file line number Diff line number Diff line change
@@ -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
33 changes: 18 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 3 additions & 15 deletions nixpkgs_review/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: "
Expand Down Expand Up @@ -134,18 +134,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
Copy link
Contributor

Choose a reason for hiding this comment

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

This removes reading of the token from the hub tool. It looks unrelated to ofborg removal. Is the removal intentional?

Copy link
Owner Author

@Mic92 Mic92 Jan 5, 2025

Choose a reason for hiding this comment

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

Yes we had several instances where this token was out of date. Also hub now often stores git hub tokens in platform specific keychains. Using GH is better because the token than should always work and not leave the user with a 403 error, that they have to debug.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aha, thank you! I'll try to migrate from hub to gh.

)
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
Expand Down
21 changes: 19 additions & 2 deletions nixpkgs_review/cli/pr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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),
Expand Down
45 changes: 15 additions & 30 deletions nixpkgs_review/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
11 changes: 3 additions & 8 deletions nixpkgs_review/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -121,7 +121,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
Expand Down Expand Up @@ -294,16 +294,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")

Expand Down
Loading