From a851cb8a18f05c070433d80bfbe8754f9edf2e97 Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <maximilian@mbosch.me>
Date: Sun, 13 Oct 2024 19:23:12 +0200
Subject: [PATCH] Bring back constituents

This partially reverts commit 370a4bf138a830c4de8a05e248ace043adbc8f4f.
---
 src/script/hydra-eval-jobset                  | 40 +++++++++++++++++++
 t/Hydra/Controller/Build/api.t                | 24 +++++++++++
 t/evaluator/evaluate-constituents-broken.t    | 32 +++++++++++++++
 t/evaluator/evaluate-constituents-gc.t        | 20 ++++++++++
 t/queue-runner/constituents.t                 | 32 +++++++++++++++
 t/queue-runner/direct-indirect-constituents.t | 35 ++++++++++++++++
 6 files changed, 183 insertions(+)
 create mode 100644 t/evaluator/evaluate-constituents-broken.t
 create mode 100644 t/evaluator/evaluate-constituents-gc.t
 create mode 100644 t/queue-runner/constituents.t
 create mode 100644 t/queue-runner/direct-indirect-constituents.t

diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset
index a03ed6dd4..bc2e72a98 100755
--- a/src/script/hydra-eval-jobset
+++ b/src/script/hydra-eval-jobset
@@ -371,6 +371,7 @@ sub evalJobs {
     push @cmd, ("--gc-roots-dir", getGCRootsDir);
     push @cmd, ("--max-jobs", 1);
     push @cmd, "--meta";
+    push @cmd, "--constituents";
     push @cmd, "--force-recurse";
     push @cmd, ("--option", "allow-import-from-derivation", "false") if $config->{allow_import_from_derivation} // "true" ne "true";
     push @cmd, ("--workers", $config->{evaluator_workers} // 1);
@@ -778,6 +779,8 @@ sub checkJobsetWrapped {
             , nixexprpath => $jobset->nixexprpath
             });
 
+        my @jobsWithConstituents;
+
         while (defined(my $job = $jobsIter->())) {
             if ($jobsetsJobset) {
                 die "The .jobsets jobset must only have a single job named 'jobsets'"
@@ -790,6 +793,10 @@ sub checkJobsetWrapped {
 
             checkBuild($db, $jobset, $ev, $inputInfo, $job, \%buildMap, $prevEval, $jobOutPathMap, $plugins)
                 unless defined $job->{error};
+
+            if (defined $job->{constituents}) {
+                push @jobsWithConstituents, $job;
+            }
         }
 
         # Have any builds been added or removed since last time?
@@ -812,6 +819,39 @@ sub checkJobsetWrapped {
                 $ev->jobsetevalmembers->create({ build => $id, isnew => $x->{new} });
             }
 
+            # Create AggregateConstituents mappings.  Since there can
+            # be jobs that alias each other, if there are multiple
+            # builds for the same derivation, pick the one with the
+            # shortest name.
+            my %drvPathToId;
+            foreach my $id (keys %buildMap) {
+                my $x = $buildMap{$id};
+                my $y = $drvPathToId{$x->{drvPath}};
+                if (defined $y) {
+                    next if length $x->{attr} > length $y->{attr};
+                    next if length $x->{attr} == length $y->{attr} && $x->{attr} ge $y->{attr};
+                }
+                $drvPathToId{$x->{drvPath}} = $x;
+            }
+
+            foreach my $job (values @jobsWithConstituents) {
+                next unless defined $job->{constituents};
+                if (defined $job->{error}) {
+                    die "aggregate job ‘$job->{attr}’ failed with the error: $job->{error}\n";
+                }
+
+                my $x = $drvPathToId{$job->{drvPath}} or
+                    die "aggregate job ‘$job->{attr}’ has no corresponding build record.\n";
+                foreach my $drvPath (@{$job->{constituents}}) {
+                    my $constituent = $drvPathToId{$drvPath};
+                    if (defined $constituent) {
+                        $db->resultset('AggregateConstituents')->update_or_create({aggregate => $x->{id}, constituent => $constituent->{id}});
+                    } else {
+                        warn "aggregate job ‘$job->{attr}’ has a constituent ‘$drvPath’ that doesn't correspond to a Hydra build\n";
+                    }
+                }
+            }
+
             foreach my $name (keys %{$inputInfo}) {
                 for (my $n = 0; $n < scalar(@{$inputInfo->{$name}}); $n++) {
                     my $input = $inputInfo->{$name}->[$n];
diff --git a/t/Hydra/Controller/Build/api.t b/t/Hydra/Controller/Build/api.t
index bd2d82929..91a553df9 100644
--- a/t/Hydra/Controller/Build/api.t
+++ b/t/Hydra/Controller/Build/api.t
@@ -78,4 +78,28 @@ subtest "validating the JSON representation of a build" => sub {
     }, "The build's JSON matches our API.");
 };
 
+subtest "accessing the constituents API" => sub {
+    my $url = $build_url . "/constituents";
+
+    my $constituents = request(GET $url,
+        Accept => 'application/json',
+    );
+
+    ok($constituents->is_success, "Getting the constituent builds");
+
+    my $data;
+    my $valid_json = lives { $data = decode_json($constituents->content); };
+    ok($valid_json, "We get back valid JSON.");
+    if (!$valid_json) {
+        use Data::Dumper;
+        print STDERR Dumper $constituents->content;
+    }
+
+    my ($buildA) = grep { $_->{nixname} eq "empty-dir-a" } @$data;
+    my ($buildB) = grep { $_->{nixname} eq "empty-dir-b" } @$data;
+
+    is($buildA->{job}, "a");
+    is($buildB->{job}, "b");
+};
+
 done_testing;
diff --git a/t/evaluator/evaluate-constituents-broken.t b/t/evaluator/evaluate-constituents-broken.t
new file mode 100644
index 000000000..4014f09fb
--- /dev/null
+++ b/t/evaluator/evaluate-constituents-broken.t
@@ -0,0 +1,32 @@
+use strict;
+use warnings;
+use Setup;
+use Test2::V0;
+use Hydra::Helper::Exec;
+
+my $ctx = test_context();
+
+my $jobsetCtx = $ctx->makeJobset(
+    expression => 'constituents-broken.nix',
+);
+my $jobset = $jobsetCtx->{"jobset"};
+
+my ($res, $stdout, $stderr) = captureStdoutStderr(60,
+    ("hydra-eval-jobset", $jobsetCtx->{"project"}->name, $jobset->name)
+);
+isnt($res, 0, "hydra-eval-jobset exits non-zero");
+ok(utf8::decode($stderr), "Stderr output is UTF8-clean");
+like(
+    $stderr,
+    qr/aggregate job ‘mixed_aggregate’ failed with the error: constituentA: does not exist/,
+    "The stderr record includes a relevant error message"
+);
+
+$jobset->discard_changes({ '+columns' => {'errormsg' => 'errormsg'} });  # refresh from DB
+like(
+    $jobset->errormsg,
+    qr/aggregate job ‘mixed_aggregate’ failed with the error: constituentA: does not exist/,
+    "The jobset records a relevant error message"
+);
+
+done_testing;
diff --git a/t/evaluator/evaluate-constituents-gc.t b/t/evaluator/evaluate-constituents-gc.t
new file mode 100644
index 000000000..a9b23e6c3
--- /dev/null
+++ b/t/evaluator/evaluate-constituents-gc.t
@@ -0,0 +1,20 @@
+use strict;
+use warnings;
+use Setup;
+use Test2::V0;
+
+my $ctx = test_context();
+
+my $builds = $ctx->makeAndEvaluateJobset(
+    expression => 'constituents.nix',
+);
+
+my $constituentA = $builds->{"constituentA"};
+my $directAggregate = $builds->{"direct_aggregate"};
+my $indirectAggregate = $builds->{"indirect_aggregate"};
+
+is(system('nix-store', '--delete', $constituentA->drvpath), 256, "Deleting a constituent derivation fails");
+is(system('nix-store', '--delete', $directAggregate->drvpath), 256, "Deleting the direct aggregate derivation fails");
+is(system('nix-store', '--delete', $indirectAggregate->drvpath), 256, "Deleting the indirect aggregate derivation fails");
+
+done_testing;
diff --git a/t/queue-runner/constituents.t b/t/queue-runner/constituents.t
new file mode 100644
index 000000000..80534e056
--- /dev/null
+++ b/t/queue-runner/constituents.t
@@ -0,0 +1,32 @@
+use feature 'unicode_strings';
+use strict;
+use warnings;
+use Setup;
+
+my %ctx = test_init();
+
+require Hydra::Schema;
+require Hydra::Model::DB;
+
+use Test2::V0;
+
+my $db = Hydra::Model::DB->new;
+hydra_setup($db);
+
+my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"});
+
+my $jobset = createBaseJobset("broken-constituent", "broken-constituent.nix", $ctx{jobsdir});
+
+ok(evalSucceeds($jobset),               "Evaluating jobs/broken-constituent.nix should exit with return code 0");
+is(nrQueuedBuildsForJobset($jobset), 0, "Evaluating jobs/broken-constituent.nix should not queue any builds");
+
+like(
+    $jobset->errormsg,
+    qr/^does-not-exist: does not exist$/m,
+    "Evaluating jobs/broken-constituent.nix should log an error for does-not-exist");
+like(
+    $jobset->errormsg,
+    qr/^does-not-evaluate: error: assertion failed/m,
+    "Evaluating jobs/broken-constituent.nix should log an error for does-not-evaluate");
+
+done_testing;
diff --git a/t/queue-runner/direct-indirect-constituents.t b/t/queue-runner/direct-indirect-constituents.t
new file mode 100644
index 000000000..a017c76f6
--- /dev/null
+++ b/t/queue-runner/direct-indirect-constituents.t
@@ -0,0 +1,35 @@
+use strict;
+use warnings;
+use Setup;
+use Test2::V0;
+
+my $ctx = test_context();
+
+my $builds = $ctx->makeAndEvaluateJobset(
+    expression => 'constituents.nix',
+);
+
+my $constituentBuildA = $builds->{"constituentA"};
+my $constituentBuildB = $builds->{"constituentB"};
+
+my $eval = $constituentBuildA->jobsetevals->first();
+is($eval->evaluationerror->has_error, 0);
+
+subtest "Verifying the direct aggregate" => sub {
+    my $aggBuild = $builds->{"direct_aggregate"};
+    is($aggBuild->constituents->first()->id, $constituentBuildA->id, "The ID of the constituent is correct");
+};
+
+subtest "Verifying the indirect aggregate" => sub {
+    my $indirectBuild = $builds->{"indirect_aggregate"};
+    is($indirectBuild->constituents->first()->id, $constituentBuildA->id, "The ID of the constituent is correct");
+};
+
+subtest "Verifying a mix of direct and indirect aggregate references" => sub {
+    my $mixedBuild = $builds->{"mixed_aggregate"};
+    my ($constituentA, $constituentB) = $mixedBuild->constituents()->search({}, {order_by => { -asc => "job"} });
+    is($constituentA->id, $constituentBuildA->id, "The ID of the constituent is correct");
+    is($constituentB->id, $constituentBuildB->id, "The ID of the constituent is correct");
+};
+
+done_testing;