nox-review: also offer option to rebuild changed tests

default.nix

@@ -3,4 +3,5 @@ with import <nixpkgs> { };
 nox.overrideAttrs (oldAttrs : {
   src = ./.;
   buildInputs = oldAttrs.buildInputs ++ [ git ];
+  propagatedBuildInputs = oldAttrs.propagatedBuildInputs ++ [ python3.pkgs.psutil ];
 })

nox/enumerate_tests.nix

@@ -0,0 +1,36 @@
+# small utility to gather the attrname of all nixos tests
+# this is cpu intensive so instead only process 1/numJobs of the list
+{ jobIndex ? 0, numJobs ? 1, disable_blacklist ? false }:
+let
+  tests = (import <nixpkgs/nixos/release.nix> {
+    supportedSystems = [ builtins.currentSystem ];
+  }).tests;
+  lib = (import <nixpkgs/lib>);
+  blacklist = if disable_blacklist then [] else
+    # list of patterns of tests to never rebuild
+    # they depend on ./. so are rebuilt on each commit
+    [ "installer" "containers-.*" "initrd-network-ssh" "boot" "ec2-.*" ];
+  enumerate = prefix: name: value:
+  # an attr in tests is either { x86_64 = derivation; } or an attrset of such values.
+  if lib.any (x: builtins.match x name != null) blacklist then [] else
+  if lib.hasAttr builtins.currentSystem value then
+    [ {attr="${prefix}${name}"; drv = value.${builtins.currentSystem};} ]
+  else
+    lib.flatten (lib.attrValues (lib.mapAttrs (enumerate (prefix + name + ".")) value));
+  # list of {attr="tests.foo"; drv=...}
+  data = enumerate "" "tests" tests;
+  # only keep a fraction of the list
+  filterFraction = list: (lib.foldl'
+    ({n, result}: element: {
+      result = if n==jobIndex then result ++ [ element ] else result;
+      n = if n+1==numJobs then 0 else n+1;
+    })
+    { n=0; result = []; }
+    list).result;
+  myData = filterFraction data;
+  evaluable = lib.filter ({attr, drv}: (builtins.tryEval drv).success) myData;
+in
+  map ({attr, drv}: {inherit attr; drv=drv.drvPath;}) evaluable
+
+
+

nox/nixpkgs_repo.py

@@ -1,10 +1,15 @@
 import os
 import subprocess
+import json
 from pathlib import Path
+from collections import defaultdict
+from concurrent.futures import ThreadPoolExecutor
+from fnmatch import fnmatch
 
 from .cache import region
 
 import click
+import psutil
 
 
 class Repo:
@@ -23,7 +28,6 @@ def __init__(self):
             self.git('config user.email nox@example.com')
             self.git('config user.name nox')
 
-
         if (Path.cwd() / '.git').exists():
             git_version = self.git('version', output=True).strip()
             if git_version >= 'git version 2':
@@ -48,9 +52,6 @@ def git(self, command, *args, cwd=None, output=False, **kwargs):
         f = subprocess.check_output if output else subprocess.check_call
         return f(command, *args, cwd=cwd, universal_newlines=output, **kwargs)
 
-
-
-
     def checkout(self, sha):
         self.git(['checkout', '-f', '--quiet', sha])
 
@@ -67,25 +68,129 @@ def merge_base(self, first, second):
         except subprocess.CalledProcessError:
             return None
 
+
 _repo = None
 
+
 def get_repo():
     global _repo
     if not _repo:
         _repo = Repo()
     return _repo
 
 
-def packages(path):
-    """List all nix packages in the repo, as a set"""
-    output = subprocess.check_output(['nix-env', '-f', path, '-qaP', '--out-path', '--show-trace'],
-                                     universal_newlines=True)
-    return set(output.split('\n'))
-
+class Buildable:
+    """
+    attr (str): attribute name under which the buildable can be built
+    path (str or tuple of them): for example <nixpkgs>, a list can be used to
+        pass other arguments like --argstr foo bar
+    hash: anything which contains the drvPath for example
+    __slots__ = "path", "attr", "extra_args", "hash"
+    """
+    def __init__(self, attr, hash, path="<nixpkgs>"):
+        self.attr = attr
+        self.hash = hash
+        self.path = path
+
+    def __eq__(self, other):
+        return hash(self) == hash(other)
+
+    def __hash__(self):
+        return hash(self.hash)
+
+    @property
+    def path_args(self):
+        if isinstance(self.path, str):
+            return (self.path, )
+        return self.path
+
+    def __repr__(self):
+        return "Buildable(attr={!r}, hash={!r}, path={!r})".format(self.attr, self.hash, self.path_args)
+
+
+def get_build_commands(buildables, program="nix-build", extra_args=[]):
+    """ Get the appropriate commands to use to build the given buildables """
+    prefix = [program]
+    prefix += extra_args
+    path_to_cmd = defaultdict(lambda *x: prefix[:])
+    for b in buildables:
+        command = path_to_cmd[b.path_args]
+        command.append('-A')
+        command.append(b.attr)
+    return [command + list(path) for path, command in path_to_cmd.items()]
+
+
+def at_given_sha(f):
+    """decorator which calls the wrappee with the path of nixpkgs at the given sha
+
+    Turns a function path -> 'a into a function sha -> 'a.
+    If the sha passed is None, passes the current directory as argument.
+    """
+    def _wrapped(sha, *args, **kwargs):
+        if sha is not None:
+            repo = get_repo()
+            repo.checkout(sha)
+            path = repo.path
+        else:
+            path = os.getcwd()
+        return f(path, *args, **kwargs)
+    _wrapped.__name__ = f.__name__
+    return _wrapped
+
+
+def cache_on_not_None(f):
+    """like region.cache_on_argument() but does not cache if the key starts None"""
+    wf = region.cache_on_arguments()(f)
+    def _wrapped(arg, *args):
+        if arg is None:
+            return f(arg, *args)
+        return wf(arg, *args)
+    _wrapped.__name__ = f.__name__
+    return _wrapped
+
+
+@cache_on_not_None
+@at_given_sha
+def packages_for_sha(path):
+    """List all nix packages in the repo, as a set of buildables"""
+    output = subprocess.check_output(['nix-env', '-f', path, '-qaP',
+        '--out-path', '--show-trace'], universal_newlines=True)
+    return {Buildable(attr, hash) for attr, hash in
+            map(lambda line: line.split(" ", 1), output.splitlines())}
+
+
+enumerate_tests = str(Path(__file__).parent / "enumerate_tests.nix")
+
+
+@cache_on_not_None
+@at_given_sha
+def tests_for_sha(path, disable_blacklist=False):
+    """List all tests wich evaluate in the repo, as a set of (attr, drvPath)"""
+    num_jobs = 32
+    # at this size, each job takes 1~1.7 GB mem
+    max_workers = max(1, psutil.virtual_memory().available//(1700*1024*1024))
+    # a job is also cpu hungry
+    try:
+        max_workers = min(max_workers, os.cpu_count())
+    except: pass
+
+    def eval(i):
+        output = subprocess.check_output(['nix-instantiate', '--eval',
+            '--json', '--strict', '-I', "nixpkgs="+str(path), enumerate_tests,
+            '--arg', "jobIndex", str(i), '--arg', 'numJobs', str(num_jobs),
+            '--arg', 'disableBlacklist', str(disable_blacklist).lower(),
+            '--show-trace'], universal_newlines=True)
+        return json.loads(output)
+
+    with ThreadPoolExecutor(max_workers=max_workers) as executor:
+        evals = executor.map(eval, range(num_jobs))
+
+    path = ("<nixpkgs/nixos/release.nix>", "--arg", "supportedSystems", "[builtins.currentSystem]")
+    attrs = set()
+    for partial in evals:
+        for test in partial:
+            b = Buildable(test["attr"], test["drv"], path=path)
+            attrs.add(b)
+
+    return attrs
 
-@region.cache_on_arguments()
-def packages_for_sha(sha):
-    """List all nix packages for the given sha"""
-    repo = get_repo()
-    repo.checkout(sha)
-    return packages(repo.path)

nox/review.py

@@ -8,60 +8,48 @@
 import click
 import requests
 
-from .nixpkgs_repo import get_repo, packages, packages_for_sha
+from .nixpkgs_repo import get_repo, at_given_sha, get_build_commands, packages_for_sha, tests_for_sha
 
 
-def get_build_command(args, attrs, path):
-    """ Get the appropriate command to use to build the given attributes """
-    command = ['nix-build']
-    command += args
-    for a in attrs:
-        command.append('-A')
-        command.append(a)
-    command.append(path)
-    return command
-
-
-def build_in_path(args, attrs, path, dry_run=False):
+@at_given_sha
+def build_sha(path, buildables, extra_args=[], dry_run=False):
     """Build the given package attributes in the given nixpkgs path"""
-    if not attrs:
+    if not buildables:
         click.echo('Nothing changed')
         return
 
     canonical_path = str(Path(path).resolve())
     result_dir = tempfile.mkdtemp(prefix='nox-review-')
     click.echo('Building in {}: {}'.format(click.style(result_dir, bold=True),
-                                           click.style(' '.join(attrs), bold=True)))
-
-    command = get_build_command(args, attrs, canonical_path)
-
-    click.echo('Invoking {}'.format(' '.join(command)))
-
-    if dry_run:
-        return
-
-    try:
-        subprocess.check_call(command, cwd=result_dir)
-    except subprocess.CalledProcessError:
-        click.secho('The invocation of "{}" failed'.format(' '.join(command)), fg='red')
-        sys.exit(1)
-    click.echo('Result in {}'.format(click.style(result_dir, bold=True)))
-    subprocess.check_call(['ls', '-l', result_dir])
+                                           click.style(' '.join(s.attr for s in buildables), bold=True)))
+
+    for command in get_build_commands(buildables, extra_args=extra_args+["-I", "nixpkgs="+canonical_path]):
+        click.echo('Invoking {}'.format(' '.join(command)))
+
+        if not dry_run:
+            try:
+                subprocess.check_call(command, cwd=result_dir)
+            except subprocess.CalledProcessError:
+                click.secho('The invocation of "{}" failed'.format(' '.join(command)), fg='red')
+                sys.exit(1)
+            click.echo('Result in {}'.format(click.style(result_dir, bold=True)))
+            subprocess.check_call(['ls', '-l', result_dir])
+
+
+def build_difference(old_sha, new_sha, extra_args=[], with_tests=False, disable_test_blacklist=False, dry_run=False):
+    click.echo("Listing old packages...")
+    before = packages_for_sha(old_sha)
+    if with_tests:
+        click.echo("Listing old tests...")
+        before |= tests_for_sha(old_sha, disable_test_blacklist)
+    click.echo("Listing new packages...")
+    after = packages_for_sha(new_sha)
+    if with_tests:
+        click.echo("Listing new tests...")
+        after |= tests_for_sha(new_sha, disable_test_blacklist)
+    build_sha(new_sha, after-before, extra_args, dry_run)
 
 
-def build_sha(args, attrs, sha, dry_run=False):
-    """Build the given package attributs for a given sha"""
-    repo = get_repo()
-    repo.checkout(sha)
-    build_in_path(args, attrs, repo.path, dry_run=dry_run)
-
-
-def differences(old, new):
-    """Return set of attributes that changed between two packages list"""
-    raw = new - old
-    # Only keep the attribute name
-    return {l.split()[0] for l in raw}
-
 def setup_nixpkgs_config(f):
     def _(*args, **kwargs):
         with tempfile.NamedTemporaryFile() as cfg:
@@ -75,13 +63,17 @@ def _(*args, **kwargs):
 @click.group()
 @click.option('--keep-going', '-k', is_flag=True, help='Keep going in case of failed builds')
 @click.option('--dry-run', is_flag=True, help="Don't actually build packages, just print the commands that would have been run")
+@click.option('--with-tests', is_flag=True, help="Also rebuild affected NixOS tests")
+@click.option('--all-tests', is_flag=True, help="Do not blacklist tests known to be false positives")
 @click.pass_context
-def cli(ctx, keep_going, dry_run):
+def cli(ctx, keep_going, dry_run, with_tests, all_tests):
     """Review a change by building the touched commits"""
     ctx.obj = {'extra-args': []}
     if keep_going:
-        ctx.obj['extra-args'].append(['--keep-going'])
+        ctx.obj['extra-args'].append('--keep-going')
     ctx.obj['dry_run'] = dry_run
+    ctx.obj['tests'] = with_tests
+    ctx.obj['no-blacklist'] = all_tests
 
 
 @cli.command(short_help='difference between working tree and a commit')
@@ -103,10 +95,7 @@ def wip(ctx, against):
 
     sha = subprocess.check_output(['git', 'rev-parse', '--verify', against]).decode().strip()
 
-    attrs = differences(packages_for_sha(sha),
-                        packages('.'))
-
-    build_in_path(ctx.obj['extra-args'], attrs, '.', dry_run=ctx.obj['dry_run'])
+    build_difference(sha, None, extra_args=ctx.obj['extra-args'], with_tests=ctx.obj["tests"], disable_test_blacklist=ctx.obj["no-blacklist"], dry_run=ctx.obj['dry_run'])
 
 
 @cli.command('pr', short_help='changes in a pull request')
@@ -170,7 +159,7 @@ def review_pr(ctx, slug, token, merge, pr):
         while not repo.merge_base(head, base):
             repo.fetch(base_refspec, depth=depth)
             repo.fetch(head_refspec, depth=depth)
-            depth *=2
+            depth *= 2
 
         # It looks like this isn't enough for a merge, so we fetch more
         repo.fetch(base_refspec, depth=depth)
@@ -181,16 +170,12 @@ def review_pr(ctx, slug, token, merge, pr):
         repo.git(['merge', head, '--no-ff', '-qm', 'Nox automatic merge'])
         merged = repo.sha('HEAD')
 
-        attrs = differences(packages_for_sha(base),
-                            packages_for_sha(merged))
-
-        build_sha(ctx.obj['extra-args'], attrs, merged, dry_run=ctx.obj['dry_run'])
+        old = base
+        new = merged
 
     else:
         commits = requests.get(payload['commits_url'], headers=headers).json()
-        base_sha = commits[-1]['parents'][0]['sha']
-
-        attrs = differences(packages_for_sha(base_sha),
-                            packages_for_sha(payload['head']['sha']))
+        old = commits[-1]['parents'][0]['sha']
+        new = payload['head']['sha']
 
-        build_sha(ctx.obj['extra-args'], attrs, payload['head']['sha'], dry_run=ctx.obj['dry_run'])
+    build_difference(old, new, extra_args=ctx.obj['extra-args'], with_tests=ctx.obj["tests"], disable_test_blacklist=ctx.obj["no-blacklist"], dry_run=ctx.obj['dry_run'])