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.

Sign up for GitHub

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

nixos/testing-python: Avoid infinite recursion in node definitions #144110

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

nixos/testing-python: Avoid infinite recursion in node definitions #144110

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 19 additions & 31 deletions nixos/lib/testing-python.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ rec {

# Generate convenience wrappers for running the test driver
# has vlans, vms and test script defaulted through env variables
# also instantiates test script with nodes, if it's a function (contract)
setupDriverForTest = {
testScript
, testName
Expand Down Expand Up @@ -143,12 +142,6 @@ rec {
(node: builtins.match "^[A-z_]([A-z0-9_]+)?$" node == null)
nodeHostNames;

testScript' =
# Call the test script with the computed nodes.
if lib.isFunction testScript
then testScript { inherit nodes; }
else testScript;

in
if lib.length invalidNodeNames > 0 then
throw ''
Expand All @@ -161,9 +154,8 @@ rec {
''
else lib.warnIf skipLint "Linting is disabled" (runCommand testDriverName
{
inherit testName;
inherit testName testScript;
nativeBuildInputs = [ makeWrapper ];
testScript = testScript';
preferLocalBuild = true;
passthru = passthru // {
inherit nodes;
Expand Down Expand Up @@ -207,14 +199,16 @@ rec {
, ...
} @ t:
let
nodes = qemu_pkg:
let
testScript' =
# Call the test script with the computed nodes.
if lib.isFunction testScript
then testScript { nodes = nodes qemu_pkg; }
else testScript;
# Instantiates test script with nodes, if it's a function (contract)
mkTestScript = qemu_pkg:
# Call the test script with the computed nodes.
if lib.isFunction testScript
then testScript { nodes = nodes qemu_pkg false; }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
then testScript { nodes = nodes qemu_pkg false; }
# To avoid an infinite recursion (nodes -> testScript -> nodes), we force disable
# the effect of config.virtualisation.useNixStoreImage, so `testScript -> nodes`
# becomes `testScript -> nodes2` where `nodes2` does not reference the `testScript`.
then testScript { nodes = nodes qemu_pkg false; }

else testScript;

nodes = qemu_pkg: addAdditionalPaths:
let
testScript' = mkTestScript qemu_pkg;
build-vms = import ./build-vms.nix {
inherit system lib pkgs minimal specialArgs;
extraConfigurations = extraConfigurations ++ [(
Expand All @@ -231,17 +225,9 @@ rec {
# copied to the image.
virtualisation.additionalPaths =
lib.optional
# A testScript may evaluate nodes, which has caused
# infinite recursions. The demand cycle involves:
# testScript -->
# nodes -->
# toplevel -->
# additionalPaths -->
# hasContext testScript' -->
# testScript (ad infinitum)
# If we don't need to build an image, we can break this
# cycle by short-circuiting when useNixStoreImage is false.
(config.virtualisation.useNixStoreImage && builtins.hasContext testScript')
(config.virtualisation.useNixStoreImage
&& addAdditionalPaths
&& builtins.hasContext testScript')
(pkgs.writeStringReferencesToFile testScript');

# Ensure we do not use aliases. Ideally this is only set
Expand All @@ -256,16 +242,18 @@ rec {
);

driver = setupDriverForTest {
inherit testScript enableOCR skipLint passthru;
inherit enableOCR skipLint passthru;
testScript = mkTestScript pkgs.qemu_test;
testName = name;
qemu_pkg = pkgs.qemu_test;
nodes = nodes pkgs.qemu_test;
nodes = nodes pkgs.qemu_test true;
Comment on lines +245 to +249
Copy link
Member

Choose a reason for hiding this comment

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

It would be best if the testScript could reuse this nodes value instead; only invoking the configs an extra time, when lib.any (n: n.config.virtualisation.useNixStoreImage) nodes.

};
driverInteractive = setupDriverForTest {
inherit testScript enableOCR skipLint passthru;
inherit enableOCR skipLint passthru;
testScript = mkTestScript pkgs.qemu;
testName = name;
qemu_pkg = pkgs.qemu;
nodes = nodes pkgs.qemu;
nodes = nodes pkgs.qemu true;
};

test =
Expand Down