From 1665aed5e302f213db048a38af44021b5e815d80 Mon Sep 17 00:00:00 2001 From: Rick van Schijndel Date: Wed, 3 Apr 2024 22:45:53 +0200 Subject: [PATCH 1/3] t: content-addressed: add test for caDependingOnFailingCA This uncovers an issue with the front-end. --- t/content-addressed/basic.t | 4 ++-- t/jobs/content-addressed.nix | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/t/content-addressed/basic.t b/t/content-addressed/basic.t index 6597e727..4f92b1dc 100644 --- a/t/content-addressed/basic.t +++ b/t/content-addressed/basic.t @@ -27,13 +27,13 @@ my $project = $db->resultset('Projects')->create({name => "tests", displayname = my $jobset = createBaseJobset("content-addressed", "content-addressed.nix", $ctx{jobsdir}); ok(evalSucceeds($jobset), "Evaluating jobs/content-addressed.nix should exit with return code 0"); -is(nrQueuedBuildsForJobset($jobset), 5, "Evaluating jobs/content-addressed.nix should result in 4 builds"); +is(nrQueuedBuildsForJobset($jobset), 6, "Evaluating jobs/content-addressed.nix should result in 6 builds"); for my $build (queuedBuildsForJobset($jobset)) { ok(runBuild($build), "Build '".$build->job."' from jobs/content-addressed.nix should exit with code 0"); my $newbuild = $db->resultset('Builds')->find($build->id); is($newbuild->finished, 1, "Build '".$build->job."' from jobs/content-addressed.nix should be finished."); - my $expected = $build->job eq "fails" ? 1 : $build->job =~ /with_failed/ ? 6 : 0; + my $expected = $build->job eq "fails" ? 1 : $build->job =~ /with_failed/ ? 6 : $build->job =~ /FailingCA/ ? 2 : 0; is($newbuild->buildstatus, $expected, "Build '".$build->job."' from jobs/content-addressed.nix should have buildstatus $expected."); my $response = request("/build/".$build->id); diff --git a/t/jobs/content-addressed.nix b/t/jobs/content-addressed.nix index 65496df5..03bb56e7 100644 --- a/t/jobs/content-addressed.nix +++ b/t/jobs/content-addressed.nix @@ -25,6 +25,13 @@ rec { FOO = empty_dir; }; + caDependingOnFailingCA = + cfg.mkContentAddressedDerivation { + name = "ca-depending-on-failing-ca"; + builder = ./dir-with-file-builder.sh; + FOO = fails; + }; + nonCaDependingOnCA = cfg.mkDerivation { name = "non-ca-depending-on-ca"; From 71986632ced0dcaa0bf8262d2f41e52d3216c39d Mon Sep 17 00:00:00 2001 From: Rick van Schijndel Date: Wed, 3 Apr 2024 22:47:22 +0200 Subject: [PATCH 2/3] hydra-server: findLog: fix issue with ca-derivations enabled When content addressed derivations are built on the hydra server, one may run into an issue where some builds suddenly don't load anymore. This seems to be caused by outPaths that are NULL (which is allowed for ca-derivations). Filter them out to prevent querying the database for them, which is not supported by the database abstraction layer that's currently in use. On my instance this appears to resolve the issue. I feel like I might be doing this at the wrong abstraction layer, but on the other hand -- it seems to resolve it and it also doesn't really look like it will hurt anything. The test added in a previous commit uncovers this issue, and this commit resolves it. So I'm happy with this patch for now. The issue I was seeing on my server: hydra-server[2549]: [error] Couldn't render template "undef error - DBIx::Class::SQLMaker::ClassicExtensions::puke(): Fatal: NULL-within-IN not implemented: The upcoming SQL::Abstract::Classic 2.0 will emit the logically correct SQL instead of raising this exception. at /nix/store/-hydra-unstable-2024-03-08_nix_2_20/libexec/hydra/lib/Hydra/Helper/Nix.pm line 190 See also short discussion here: https://github.com/NixOS/nixpkgs/pull/297392#issuecomment-2035366263 --- src/lib/Hydra/Helper/Nix.pm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm index 71a8a7d7..7b16b1be 100644 --- a/src/lib/Hydra/Helper/Nix.pm +++ b/src/lib/Hydra/Helper/Nix.pm @@ -187,6 +187,10 @@ sub findLog { return undef if scalar @outPaths == 0; + # Filter out any NULLs. Content-addressed derivations + # that haven't built yet or failed to build may have a NULL outPath. + @outPaths = grep {defined} @outPaths; + my @steps = $c->model('DB::BuildSteps')->search( { path => { -in => [@outPaths] } }, { select => ["drvpath"] From 3f913a771d1cdfba004a3639fc109e57bf3dae8e Mon Sep 17 00:00:00 2001 From: Rick van Schijndel Date: Wed, 3 Apr 2024 22:55:00 +0200 Subject: [PATCH 3/3] t: content-addressed: add a comment about a misleading testcase --- t/content-addressed/basic.t | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/content-addressed/basic.t b/t/content-addressed/basic.t index 4f92b1dc..aefb457b 100644 --- a/t/content-addressed/basic.t +++ b/t/content-addressed/basic.t @@ -55,6 +55,8 @@ for my $build (queuedBuildsForJobset($jobset)) { } +# XXX: deststoredir is undefined: Use of uninitialized value $ctx{"deststoredir"} in concatenation (.) or string at t/content-addressed/basic.t line 58. +# XXX: This test seems to not do what it seems to be doing. See documentation: https://metacpan.org/pod/Test2::V0#isnt($got,-$do_not_want,-$name) isnt(<$ctx{deststoredir}/realisations/*>, "", "The destination store should have the realisations of the built derivations registered"); done_testing;