Commit 53b43ce0 authored by Silvan Mosberger's avatar Silvan Mosberger
Browse files

tests.nixpkgs-check-by-name: Fix and document behavior without --base

Previously, not passing `--base` would enforce the most strict checks.
While there's currently no actual violation of these stricter checks,
this does not match the previous behavior.

This won't matter once CI passes `--base`, the code handling the
optionality can be removed then.
parent bb08bfc2
Loading
Loading
Loading
Loading
+7 −2
Original line number Diff line number Diff line
@@ -10,8 +10,13 @@ This API may be changed over time if the CI workflow making use of it is adjuste

- Command line: `nixpkgs-check-by-name [--base <BASE_NIXPKGS>] <NIXPKGS>`
- Arguments:
  - `<BASE_NIXPKGS>`: The path to the Nixpkgs to check against
  - `<NIXPKGS>`: The path to the Nixpkgs to check
  - `<NIXPKGS>`: The path to the Nixpkgs to check.
  - `<BASE_NIXPKGS>`: The path to the Nixpkgs to use as the base to compare `<NIXPKGS>` against.
    This allows the strictness of checks to increase over time by only preventing _new_ violations from being introduced,
    while allowing violations that already existed.

    If not specified, all violations of stricter checks are allowed.
    However, this flag will become required once CI passes it.
- Exit code:
  - `0`: If the [validation](#validity-checks) is successful
  - `1`: If the [validation](#validity-checks) is not successful
+9 −7
Original line number Diff line number Diff line
@@ -75,14 +75,16 @@ pub fn process<W: io::Write>(
    let main_result = check_nixpkgs(main_nixpkgs, eval_accessible_paths)?;
    let check_result = main_result.result_map(|nixpkgs_version| {
        if let Some(base) = base_nixpkgs {
            check_nixpkgs(base, eval_accessible_paths)?.result_map(|base_nixpkgs_version| {
                Ok(Nixpkgs::compare(base_nixpkgs_version, nixpkgs_version))
            })
        } else {
            check_nixpkgs(base, eval_accessible_paths, error_writer)?.result_map(
                |base_nixpkgs_version| {
                    Ok(Nixpkgs::compare(
                version::Nixpkgs::default(),
                        Some(base_nixpkgs_version),
                        nixpkgs_version,
                    ))
                },
            )
        } else {
            Ok(Nixpkgs::compare(None, nixpkgs_version))
        }
    })?;

+15 −2
Original line number Diff line number Diff line
@@ -16,12 +16,25 @@ impl Nixpkgs {
    /// Compares two Nixpkgs versions against each other, returning validation errors only if the
    /// `from` version satisfied the stricter checks, while the `to` version doesn't satisfy them
    /// anymore.
    pub fn compare(from: Self, to: Self) -> Validation<()> {
    pub fn compare(optional_from: Option<Self>, to: Self) -> Validation<()> {
        validation::sequence_(
            // We only loop over the current attributes,
            // we don't need to check ones that were removed
            to.attributes.into_iter().map(|(name, attr_to)| {
                Attribute::compare(&name, from.attributes.get(&name), &attr_to)
                let attr_from = if let Some(from) = &optional_from {
                    from.attributes.get(&name)
                } else {
                    // This pretends that if there's no base version to compare against, all
                    // attributes existed without conforming to the new strictness check for
                    // backwards compatibility.
                    // TODO: Remove this case. This is only needed because the `--base`
                    // argument is still optional, which doesn't need to be once CI is updated
                    // to pass it.
                    Some(&Attribute {
                        empty_non_auto_called: EmptyNonAutoCalled::Invalid,
                    })
                };
                Attribute::compare(&name, attr_from, &attr_to)
            }),
        )
    }
+1 −0
Original line number Diff line number Diff line
import ../../mock-nixpkgs.nix { root = ./.; }
+1 −0
Original line number Diff line number Diff line
(this is just here so the directory can get tracked by git)