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

infinisil · infinisil · commit 53b43ce0e322 · 2023-12-15T01:01:14.000+01:00
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.
diff --git a/pkgs/test/nixpkgs-check-by-name/README.md b/pkgs/test/nixpkgs-check-by-name/README.md
@@ -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
diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs
@@ -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))
-            })
+            check_nixpkgs(base, eval_accessible_paths, error_writer)?.result_map(
+                |base_nixpkgs_version| {
+                    Ok(Nixpkgs::compare(
+                        Some(base_nixpkgs_version),
+                        nixpkgs_version,
+                    ))
+                },
+            )
         } else {
-            Ok(Nixpkgs::compare(
-                version::Nixpkgs::default(),
-                nixpkgs_version,
-            ))
+            Ok(Nixpkgs::compare(None, nixpkgs_version))
         }
     })?;
 
diff --git a/pkgs/test/nixpkgs-check-by-name/src/version.rs b/pkgs/test/nixpkgs-check-by-name/src/version.rs
@@ -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)
             }),
         )
     }
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/default.nix b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/default.nix
@@ -0,0 +1 @@
+import ../../mock-nixpkgs.nix { root = ./.; }
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/pkgs/by-name/README.md b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/base/pkgs/by-name/README.md
@@ -0,0 +1 @@
+(this is just here so the directory can get tracked by git)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+import ../../mock-nixpkgs.nix { root = ./.; }`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+(this is just here so the directory can get tracked by git)`