From 1d5b3d568604e8e938542da07f44b5f2cfe67c8b Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Mon, 12 Feb 2024 17:15:51 +0100 Subject: [PATCH] Use subs instead of global variables for local & binary cache store Otherwise the `include`-test would fail: this test defines a `hydra.conf` with an `include` statement. However, the file to be included is created after the test base is set up. This means that - among others - the `Hydra::Helper::Nix` module is loaded before the file to include exists. Since the global variables are directly initialized and try to read from the config, the test fails, because loading the config at that time doesn't work. Delaying this into a subrouting solves the issue. I decided against messing around with the config mechanism here since I consider this behavior to be correct. The local store gets (i.e. `Hydra::Store->new()` w/o args) cached in the Perl bindings already[1], however this isn't the case for the binary cache store. Because of that, a `state` variable is used to cache the binary cache store. At runtime, the location of that store doesn't change anyways. [1] https://github.com/NixOS/nix/blob/e3ccb7e42a55529d8d50544e7e0cebb5cbc606cf/perl/lib/Nix/Store.xs#L88-L96 --- src/lib/Hydra/Base/Controller/NixChannel.pm | 2 +- src/lib/Hydra/Controller/Build.pm | 16 ++++++++-------- src/lib/Hydra/Controller/Root.pm | 2 +- src/lib/Hydra/Helper/Nix.pm | 18 +++++++++++++----- src/lib/Hydra/Plugin/BazaarInput.pm | 6 +++--- src/lib/Hydra/Plugin/DarcsInput.pm | 6 +++--- src/lib/Hydra/Plugin/GitInput.pm | 6 +++--- src/lib/Hydra/Plugin/MercurialInput.pm | 6 +++--- src/lib/Hydra/Plugin/PathInput.pm | 4 ++-- src/lib/Hydra/Plugin/SubversionInput.pm | 8 ++++---- src/lib/Hydra/View/NARInfo.pm | 6 +++--- src/script/hydra-eval-jobset | 8 ++++---- src/script/hydra-update-gc-roots | 4 ++-- 13 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/lib/Hydra/Base/Controller/NixChannel.pm b/src/lib/Hydra/Base/Controller/NixChannel.pm index 9a815034b..084e755fa 100644 --- a/src/lib/Hydra/Base/Controller/NixChannel.pm +++ b/src/lib/Hydra/Base/Controller/NixChannel.pm @@ -29,7 +29,7 @@ sub getChannelData { my $outputs = {}; foreach my $output (@outputs) { my $outPath = $output->get_column("outpath"); - next if $checkValidity && !$BINARY_CACHE_STORE->isValidPath($outPath); + next if $checkValidity && !binaryCacheStore()->isValidPath($outPath); $outputs->{$output->get_column("outname")} = $outPath; push @storePaths, $outPath; # Put the system type in the manifest (for top-level diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm index 6d93ef647..ae18d1553 100644 --- a/src/lib/Hydra/Controller/Build.pm +++ b/src/lib/Hydra/Controller/Build.pm @@ -79,8 +79,8 @@ sub build_GET { # XXX: If the derivation is content-addressed then this will always return # false because `$_->path` will be empty $c->stash->{available} = - all { $_->path && $BINARY_CACHE_STORE->isValidPath($_->path) } $build->buildoutputs->all; - $c->stash->{drvAvailable} = $MACHINE_LOCAL_STORE->isValidPath($build->drvpath); + all { $_->path && binaryCacheStore()->isValidPath($_->path) } $build->buildoutputs->all; + $c->stash->{drvAvailable} = machineLocalStore()->isValidPath($build->drvpath); if ($build->finished && $build->iscachedbuild) { my $path = ($build->buildoutputs)[0]->path or undef; @@ -304,7 +304,7 @@ sub output : Chained('buildChain') PathPart Args(1) { error($c, "This build is not finished yet.") unless $build->finished; my $output = $build->buildoutputs->find({name => $outputName}); notFound($c, "This build has no output named ‘$outputName’") unless defined $output; - gone($c, "Output is no longer available.") unless $BINARY_CACHE_STORE->isValidPath($output->path); + gone($c, "Output is no longer available.") unless binaryCacheStore()->isValidPath($output->path); $c->response->header('Content-Disposition', "attachment; filename=\"build-${\$build->id}-${\$outputName}.nar.bz2\""); $c->stash->{current_view} = 'NixNAR'; @@ -421,7 +421,7 @@ sub getDependencyGraph { }; $$done{$path} = $node; my @refs; - foreach my $ref ($BINARY_CACHE_STORE->queryReferences($path)) { + foreach my $ref (binaryCacheStore()->queryReferences($path)) { next if $ref eq $path; next unless $runtime || $ref =~ /\.drv$/; getDependencyGraph($self, $c, $runtime, $done, $ref); @@ -429,7 +429,7 @@ sub getDependencyGraph { } # Show in reverse topological order to flatten the graph. # Should probably do a proper BFS. - my @sorted = reverse $BINARY_CACHE_STORE->topoSortPaths(@refs); + my @sorted = reverse binaryCacheStore()->topoSortPaths(@refs); $node->{refs} = [map { $$done{$_} } @sorted]; } @@ -442,7 +442,7 @@ sub build_deps : Chained('buildChain') PathPart('build-deps') { my $build = $c->stash->{build}; my $drvPath = $build->drvpath; - error($c, "Derivation no longer available.") unless $MACHINE_LOCAL_STORE->isValidPath($drvPath); + error($c, "Derivation no longer available.") unless machineLocalStore()->isValidPath($drvPath); $c->stash->{buildTimeGraph} = getDependencyGraph($self, $c, 0, {}, $drvPath); @@ -457,7 +457,7 @@ sub runtime_deps : Chained('buildChain') PathPart('runtime-deps') { requireLocalStore($c); - error($c, "Build outputs no longer available.") unless all { $BINARY_CACHE_STORE->isValidPath($_) } @outPaths; + error($c, "Build outputs no longer available.") unless all { binaryCacheStore()->isValidPath($_) } @outPaths; my $done = {}; $c->stash->{runtimeGraph} = [ map { getDependencyGraph($self, $c, 1, $done, $_) } @outPaths ]; @@ -477,7 +477,7 @@ sub nix : Chained('buildChain') PathPart('nix') CaptureArgs(0) { if (isLocalStore) { foreach my $out ($build->buildoutputs) { notFound($c, "Path " . $out->path . " is no longer available.") - unless $BINARY_CACHE_STORE->isValidPath($out->path); + unless binaryCacheStore()->isValidPath($out->path); } } diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm index 9d394f2c9..0d8d8dff4 100644 --- a/src/lib/Hydra/Controller/Root.pm +++ b/src/lib/Hydra/Controller/Root.pm @@ -395,7 +395,7 @@ sub narinfo :Path :Args(StrMatch[NARINFO_REGEX]) { my ($hash) = $narinfo =~ NARINFO_REGEX; die("Hash length was not 32") if length($hash) != 32; - my $path = $BINARY_CACHE_STORE->queryPathFromHashPart($hash); + my $path = binaryCacheStore()->queryPathFromHashPart($hash); if (!$path) { $c->response->status(404); diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm index 3c3965dfa..84c851a84 100644 --- a/src/lib/Hydra/Helper/Nix.pm +++ b/src/lib/Hydra/Helper/Nix.pm @@ -1,5 +1,7 @@ package Hydra::Helper::Nix; +use feature 'state'; + use strict; use warnings; use Exporter; @@ -40,12 +42,18 @@ our @EXPORT = qw( registerRoot restartBuilds run - $MACHINE_LOCAL_STORE - $BINARY_CACHE_STORE + machineLocalStore + binaryCacheStore ); -our $MACHINE_LOCAL_STORE = Nix::Store->new(); -our $BINARY_CACHE_STORE = Nix::Store->new(getStoreUri()); +sub machineLocalStore { + return Nix::Store->new(); +} +sub binaryCacheStore { + state $binaryCache; # Stores with a custom URL aren't cached by the perl bindings. + $binaryCache = Nix::Store->new(getStoreUri()) if not $binaryCache; + return $binaryCache; +} sub getHydraHome { @@ -499,7 +507,7 @@ sub restartBuilds { $builds = $builds->search({ finished => 1 }); foreach my $build ($builds->search({}, { columns => ["drvpath"] })) { - next if !$MACHINE_LOCAL_STORE->isValidPath($build->drvpath); + next if !machineLocalStore()->isValidPath($build->drvpath); registerRoot $build->drvpath; } diff --git a/src/lib/Hydra/Plugin/BazaarInput.pm b/src/lib/Hydra/Plugin/BazaarInput.pm index b35ed7c8c..8c6fada64 100644 --- a/src/lib/Hydra/Plugin/BazaarInput.pm +++ b/src/lib/Hydra/Plugin/BazaarInput.pm @@ -37,9 +37,9 @@ sub fetchInput { (my $cachedInput) = $self->{db}->resultset('CachedBazaarInputs')->search( {uri => $uri, revision => $revision}); - $MACHINE_LOCAL_STORE->addTempRoot($cachedInput->storepath) if defined $cachedInput; + machineLocalStore()->addTempRoot($cachedInput->storepath) if defined $cachedInput; - if (defined $cachedInput && $MACHINE_LOCAL_STORE->isValidPath($cachedInput->storepath)) { + if (defined $cachedInput && machineLocalStore()->isValidPath($cachedInput->storepath)) { $storePath = $cachedInput->storepath; $sha256 = $cachedInput->sha256hash; } else { @@ -57,7 +57,7 @@ sub fetchInput { ($sha256, $storePath) = split ' ', $stdout; # FIXME: time window between nix-prefetch-bzr and addTempRoot. - $MACHINE_LOCAL_STORE->addTempRoot($storePath); + machineLocalStore()->addTempRoot($storePath); $self->{db}->txn_do(sub { $self->{db}->resultset('CachedBazaarInputs')->create( diff --git a/src/lib/Hydra/Plugin/DarcsInput.pm b/src/lib/Hydra/Plugin/DarcsInput.pm index a8df63964..d513f700e 100644 --- a/src/lib/Hydra/Plugin/DarcsInput.pm +++ b/src/lib/Hydra/Plugin/DarcsInput.pm @@ -57,7 +57,7 @@ sub fetchInput { {uri => $uri, revision => $revision}, {rows => 1}); - if (defined $cachedInput && $MACHINE_LOCAL_STORE->isValidPath($cachedInput->storepath)) { + if (defined $cachedInput && machineLocalStore()->isValidPath($cachedInput->storepath)) { $storePath = $cachedInput->storepath; $sha256 = $cachedInput->sha256hash; $revision = $cachedInput->revision; @@ -74,8 +74,8 @@ sub fetchInput { die "darcs changes --count failed" if $? != 0; system "rm", "-rf", "$tmpDir/export/_darcs"; - $storePath = $MACHINE_LOCAL_STORE->addToStore("$tmpDir/export", 1, "sha256"); - $sha256 = $MACHINE_LOCAL_STORE->queryPathHash($storePath); + $storePath = machineLocalStore()->addToStore("$tmpDir/export", 1, "sha256"); + $sha256 = machineLocalStore()->queryPathHash($storePath); $sha256 =~ s/sha256://; $self->{db}->txn_do(sub { diff --git a/src/lib/Hydra/Plugin/GitInput.pm b/src/lib/Hydra/Plugin/GitInput.pm index 0de021282..e1dac085f 100644 --- a/src/lib/Hydra/Plugin/GitInput.pm +++ b/src/lib/Hydra/Plugin/GitInput.pm @@ -186,9 +186,9 @@ sub fetchInput { {uri => $uri, branch => $branch, revision => $revision, isdeepclone => defined($deepClone) ? 1 : 0}, {rows => 1}); - $MACHINE_LOCAL_STORE->addTempRoot($cachedInput->storepath) if defined $cachedInput; + machineLocalStore()->addTempRoot($cachedInput->storepath) if defined $cachedInput; - if (defined $cachedInput && $MACHINE_LOCAL_STORE->isValidPath($cachedInput->storepath)) { + if (defined $cachedInput && machineLocalStore()->isValidPath($cachedInput->storepath)) { $storePath = $cachedInput->storepath; $sha256 = $cachedInput->sha256hash; $revision = $cachedInput->revision; @@ -217,7 +217,7 @@ sub fetchInput { ($sha256, $storePath) = split ' ', grab(cmd => ["nix-prefetch-git", $clonePath, $revision], chomp => 1); # FIXME: time window between nix-prefetch-git and addTempRoot. - $MACHINE_LOCAL_STORE->addTempRoot($storePath); + machineLocalStore()->addTempRoot($storePath); $self->{db}->txn_do(sub { $self->{db}->resultset('CachedGitInputs')->update_or_create( diff --git a/src/lib/Hydra/Plugin/MercurialInput.pm b/src/lib/Hydra/Plugin/MercurialInput.pm index 85bd2c708..076489d66 100644 --- a/src/lib/Hydra/Plugin/MercurialInput.pm +++ b/src/lib/Hydra/Plugin/MercurialInput.pm @@ -67,9 +67,9 @@ sub fetchInput { (my $cachedInput) = $self->{db}->resultset('CachedHgInputs')->search( {uri => $uri, branch => $branch, revision => $revision}); - $MACHINE_LOCAL_STORE->addTempRoot($cachedInput->storepath) if defined $cachedInput; + machineLocalStore()->addTempRoot($cachedInput->storepath) if defined $cachedInput; - if (defined $cachedInput && $MACHINE_LOCAL_STORE->isValidPath($cachedInput->storepath)) { + if (defined $cachedInput && machineLocalStore()->isValidPath($cachedInput->storepath)) { $storePath = $cachedInput->storepath; $sha256 = $cachedInput->sha256hash; } else { @@ -84,7 +84,7 @@ sub fetchInput { ($sha256, $storePath) = split ' ', $stdout; # FIXME: time window between nix-prefetch-hg and addTempRoot. - $MACHINE_LOCAL_STORE->addTempRoot($storePath); + machineLocalStore()->addTempRoot($storePath); $self->{db}->txn_do(sub { $self->{db}->resultset('CachedHgInputs')->update_or_create( diff --git a/src/lib/Hydra/Plugin/PathInput.pm b/src/lib/Hydra/Plugin/PathInput.pm index c923a03c3..a69de7fef 100644 --- a/src/lib/Hydra/Plugin/PathInput.pm +++ b/src/lib/Hydra/Plugin/PathInput.pm @@ -29,7 +29,7 @@ sub fetchInput { {srcpath => $uri, lastseen => {">", $timestamp - $timeout}}, {rows => 1, order_by => "lastseen DESC"}); - if (defined $cachedInput && $MACHINE_LOCAL_STORE->isValidPath($cachedInput->storepath)) { + if (defined $cachedInput && machineLocalStore()->isValidPath($cachedInput->storepath)) { $storePath = $cachedInput->storepath; $sha256 = $cachedInput->sha256hash; $timestamp = $cachedInput->timestamp; @@ -45,7 +45,7 @@ sub fetchInput { } chomp $storePath; - $sha256 = ($MACHINE_LOCAL_STORE->queryPathInfo($storePath, 0))[1] or die; + $sha256 = (machineLocalStore()->queryPathInfo($storePath, 0))[1] or die; ($cachedInput) = $self->{db}->resultset('CachedPathInputs')->search( {srcpath => $uri, sha256hash => $sha256}); diff --git a/src/lib/Hydra/Plugin/SubversionInput.pm b/src/lib/Hydra/Plugin/SubversionInput.pm index 83c1f39d2..484e86051 100644 --- a/src/lib/Hydra/Plugin/SubversionInput.pm +++ b/src/lib/Hydra/Plugin/SubversionInput.pm @@ -44,7 +44,7 @@ sub fetchInput { (my $cachedInput) = $self->{db}->resultset('CachedSubversionInputs')->search( {uri => $uri, revision => $revision}); - $MACHINE_LOCAL_STORE->addTempRoot($cachedInput->storepath) if defined $cachedInput; + machineLocalStore()->addTempRoot($cachedInput->storepath) if defined $cachedInput; if (defined $cachedInput && isValidPath($cachedInput->storepath)) { $storePath = $cachedInput->storepath; @@ -61,16 +61,16 @@ sub fetchInput { die "error checking out Subversion repo at `$uri':\n$stderr" if $res; if ($type eq "svn-checkout") { - $storePath = $MACHINE_LOCAL_STORE->addToStore($wcPath, 1, "sha256"); + $storePath = machineLocalStore()->addToStore($wcPath, 1, "sha256"); } else { # Hm, if the Nix Perl bindings supported filters in # addToStore(), then we wouldn't need to make a copy here. my $tmpDir = File::Temp->newdir("hydra-svn-export.XXXXXX", CLEANUP => 1, TMPDIR => 1) or die; (system "svn", "export", $wcPath, "$tmpDir/source", "--quiet") == 0 or die "svn export failed"; - $storePath = $MACHINE_LOCAL_STORE->addToStore("$tmpDir/source", 1, "sha256"); + $storePath = machineLocalStore()->addToStore("$tmpDir/source", 1, "sha256"); } - $sha256 = $MACHINE_LOCAL_STORE->queryPathHash($storePath); $sha256 =~ s/sha256://; + $sha256 = machineLocalStore()->queryPathHash($storePath); $sha256 =~ s/sha256://; $self->{db}->txn_do(sub { $self->{db}->resultset('CachedSubversionInputs')->update_or_create( diff --git a/src/lib/Hydra/View/NARInfo.pm b/src/lib/Hydra/View/NARInfo.pm index bf8711a4a..fa2642acf 100644 --- a/src/lib/Hydra/View/NARInfo.pm +++ b/src/lib/Hydra/View/NARInfo.pm @@ -16,7 +16,7 @@ sub process { $c->response->content_type('text/x-nix-narinfo'); # !!! check MIME type - my ($deriver, $narHash, $time, $narSize, $refs) = $MACHINE_LOCAL_STORE->queryPathInfo($storePath, 1); + my ($deriver, $narHash, $time, $narSize, $refs) = machineLocalStore()->queryPathInfo($storePath, 1); my $info; $info .= "StorePath: $storePath\n"; @@ -27,8 +27,8 @@ sub process { $info .= "References: " . join(" ", map { basename $_ } @{$refs}) . "\n"; if (defined $deriver) { $info .= "Deriver: " . basename $deriver . "\n"; - if ($MACHINE_LOCAL_STORE->isValidPath($deriver)) { - my $drv = $MACHINE_LOCAL_STORE->derivationFromPath($deriver); + if (machineLocalStore()->isValidPath($deriver)) { + my $drv = machineLocalStore()->derivationFromPath($deriver); $info .= "System: $drv->{platform}\n"; } } diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset index 72a386f57..58e025a22 100755 --- a/src/script/hydra-eval-jobset +++ b/src/script/hydra-eval-jobset @@ -85,14 +85,14 @@ sub attrsToSQL { # Fetch a store path from 'eval_substituter' if not already present. sub getPath { my ($path) = @_; - return 1 if $MACHINE_LOCAL_STORE->isValidPath($path); + return 1 if machineLocalStore()->isValidPath($path); my $substituter = $config->{eval_substituter}; system("nix", "--experimental-features", "nix-command", "copy", "--from", $substituter, "--", $path) if defined $substituter; - return $MACHINE_LOCAL_STORE->isValidPath($path); + return machineLocalStore()->isValidPath($path); } @@ -143,7 +143,7 @@ sub fetchInputBuild { , version => $version , outputName => $mainOutput->name }; - if ($MACHINE_LOCAL_STORE->isValidPath($prevBuild->drvpath)) { + if (machineLocalStore()->isValidPath($prevBuild->drvpath)) { $result->{drvPath} = $prevBuild->drvpath; } @@ -233,7 +233,7 @@ sub fetchInputEval { my $out = $build->buildoutputs->find({ name => "out" }); next unless defined $out; # FIXME: Should we fail if the path is not valid? - next unless $MACHINE_LOCAL_STORE->isValidPath($out->path); + next unless machineLocalStore()->isValidPath($out->path); $jobs->{$build->get_column('job')} = $out->path; } diff --git a/src/script/hydra-update-gc-roots b/src/script/hydra-update-gc-roots index 130cbb682..5c8050da5 100755 --- a/src/script/hydra-update-gc-roots +++ b/src/script/hydra-update-gc-roots @@ -46,7 +46,7 @@ sub keepBuild { $build->finished && ($build->buildstatus == 0 || $build->buildstatus == 6)) { foreach my $path (split / /, $build->get_column('outpaths')) { - if ($MACHINE_LOCAL_STORE->isValidPath($path)) { + if (machineLocalStore()->isValidPath($path)) { addRoot $path; } else { print STDERR " warning: output ", $path, " has disappeared\n" if $build->finished; @@ -54,7 +54,7 @@ sub keepBuild { } } if (!$build->finished || ($keepFailedDrvs && $build->buildstatus != 0)) { - if ($MACHINE_LOCAL_STORE->isValidPath($build->drvpath)) { + if (machineLocalStore()->isValidPath($build->drvpath)) { addRoot $build->drvpath; } else { print STDERR " warning: derivation ", $build->drvpath, " has disappeared\n";