From 6a54ab24e26b4e387c41e6d8958e7a9ec36c8398 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Thu, 7 Dec 2023 02:00:22 -0500
Subject: [PATCH 1/2] Use factored-out `BuildResult` serializer
For the record, here is the Nix 2.19 version:
https://github.com/NixOS/nix/blob/2.19-maintenance/src/libstore/serve-protocol.cc,
which is what we would initially use.
It is a more complete version of what Hydra has today except for one
thing: it always unconditionally sets the start/stop times.
I think that is correct at the other end seems to unconditionally
measure them, but just to be extra careful, I reproduced the old
behavior of falling back on Hydra's own measurements if `startTime` is
0.
The only difference is that the fallback `stopTime` is now measured from
after the entire `BuildResult` is transferred over the wire, but I think
that should be negligible if it is measurable at all. (And remember,
this is fallback case I already suspect is dead code.)
---
src/hydra-queue-runner/build-remote.cc | 36 ++++++++++----------------
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc
index 73f46a53..5206ae5a 100644
--- a/src/hydra-queue-runner/build-remote.cc
+++ b/src/hydra-queue-runner/build-remote.cc
@@ -286,9 +286,6 @@ static BuildResult performBuild(
counter & nrStepsBuilding
)
{
-
- BuildResult result;
-
conn.to << ServeProto::Command::BuildDerivation << localStore.printStorePath(drvPath);
writeDerivation(conn.to, localStore, drv);
conn.to << options.maxSilentTime << options.buildTimeout;
@@ -301,30 +298,25 @@ static BuildResult performBuild(
}
conn.to.flush();
- result.startTime = time(0);
+ BuildResult result;
+ time_t startTime, stopTime;
+
+ startTime = time(0);
{
MaintainCount<counter> mc(nrStepsBuilding);
- result.status = (BuildResult::Status)readInt(conn.from);
+ result = ServeProto::Serialise<BuildResult>::read(localStore, conn);
}
- result.stopTime = time(0);
+ stopTime = time(0);
-
- result.errorMsg = readString(conn.from);
- if (GET_PROTOCOL_MINOR(conn.remoteVersion) >= 3) {
- result.timesBuilt = readInt(conn.from);
- result.isNonDeterministic = readInt(conn.from);
- auto start = readInt(conn.from);
- auto stop = readInt(conn.from);
- if (start && start) {
- /* Note: this represents the duration of a single
- round, rather than all rounds. */
- result.startTime = start;
- result.stopTime = stop;
- }
- }
- if (GET_PROTOCOL_MINOR(conn.remoteVersion) >= 6) {
- ServeProto::Serialise<DrvOutputs>::read(localStore, conn);
+ if (!result.startTime) {
+ // If the builder gave `startTime = 0`, use our measurements
+ // instead of the builder's.
+ //
+ // Note: this represents the duration of a single round, rather
+ // than all rounds.
+ result.startTime = startTime;
+ result.stopTime = stopTime;
}
return result;
From 411e4d0c2458ec5319b8ea88129dff807793f7d7 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Fri, 8 Dec 2023 11:30:31 -0500
Subject: [PATCH 2/2] Let tests themselves intentionally leak temp dir (#1320)
* Let tests themselves intentionally leak temp dir
By default Yath will clean up temporary files, so the result is the
same. But `--keep-dirs` can be passed to `yath test` telling Yath to
*not* clean them up instead. This is very useful for debugging.
* Update t/lib/HydraTestContext.pm
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
---
t/lib/HydraTestContext.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/t/lib/HydraTestContext.pm b/t/lib/HydraTestContext.pm
index 53eaa0f7..a22c3df1 100644
--- a/t/lib/HydraTestContext.pm
+++ b/t/lib/HydraTestContext.pm
@@ -39,7 +39,9 @@ use Hydra::Helper::Exec;
sub new {
my ($class, %opts) = @_;
- my $dir = File::Temp->newdir();
+ # Cleanup will be managed by yath. By the default it will be cleaned
+ # up, but can be kept to aid in debugging test failures.
+ my $dir = File::Temp->newdir(CLEANUP => 0);
$ENV{'HYDRA_DATA'} = "$dir/hydra-data";
mkdir $ENV{'HYDRA_DATA'};