Skip to content

Conversation

@CyberShadow
Copy link
Contributor

"nix-store --verify-path" exits with status 1 in various situations, including when the path does not exist (due to having failed to build) or when the checksum doesn't match the Nix database.

Since nixpkgs-review tries to report build failures specifically, use "nix store verify" instead with its "--no-contents" flag. This avoids classifying checksum mismatches as build failures, which can be misleading to nixpkgs-review users.

Though still an abnormal situation that might be useful to report, we now ignore store path errors; these are generally an abnormal situation, and detecting them is out-of-scope for nixpkgs-review. Should it become clearer that it would be useful to report them, it could be done by removing --no-contents and checking "nix store verify"'s exit status, which indicates the nature of the problem.

Fixes #415

"nix-store --verify-path" exits with status 1 in various situations,
including when the path does not exist (due to having failed to build)
or when the checksum doesn't match the Nix database.

Since nixpkgs-review tries to report build failures specifically, use
"nix store verify" instead with its "--no-contents" flag. This avoids
classifying checksum mismatches as build failures, which can be
misleading to nixpkgs-review users.

Though still an abnormal situation that might be useful to report, we
now ignore store path errors; these are generally an abnormal
situation, and detecting them is out-of-scope for nixpkgs-review.  Should
it become clearer that it would be useful to report them, it could be
done by removing --no-contents and checking "nix store verify"'s exit
status, which indicates the nature of the problem.
@CyberShadow
Copy link
Contributor Author

CyberShadow commented Sep 9, 2024

I tested the two approaches with this script:

#!/usr/bin/env bash
set -eEuo pipefail

expr='
let
  name = builtins.getEnv "name";  # https://github.com/NixOS/nix/issues/2678
  pkgs = import <nixpkgs> {};
  pkg = pkgs.stdenv.mkDerivation {
    inherit name;
    dontUnpack = true;
    buildCommand = if name == "failed" then "false" else "mkdir $out";
  };
in {
  path = "${pkgs.lib.getOutput "out" pkg}";
  inherit pkg;
}
'

for f in good corrupted failed ; do
  name="$f" \
  nix-build \
    -A pkg \
    --expr "$expr" || true

  path=$(
    name="$f" \
    nix \
      --extra-experimental-features nix-command \
      eval \
      --expr "$expr" \
      path \
      --impure \
      --raw
  )
  if [[ "$f" == corrupted ]] ; then
    sudo touch     "$path"/oops.txt
    sudo chmod 666 "$path"/oops.txt
  fi

  status=0
  # old method:
  # nix-store --verify-path "$path" || status=$?
  # new method:
  nix --extra-experimental-features nix-command store verify --no-contents --no-trust "$path" || status=$?
  echo "$f: $status"
done

The "old method" fails on failed and corrupted. The "new method" fails only on failed, as desired.

@Mic92
Copy link
Owner

Mic92 commented Sep 9, 2024

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Sep 9, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 1097624

@mergify mergify bot merged commit 1097624 into Mic92:master Sep 9, 2024
4 checks passed
@Mic92
Copy link
Owner

Mic92 commented Sep 9, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nixpkgs-review doesn't differentiate between build and store errors.

2 participants