From 845e6d4760ae810b9a9286712a130554a95318a1 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 9 Feb 2022 13:40:51 -0500 Subject: [PATCH] captureStdoutStderr*: move to Hydra::Helper::Exec which helps avoid some environment variable fixation problems --- src/lib/Hydra/Helper/Exec.pm | 35 +++++++++++++++++++ src/lib/Hydra/Helper/Nix.pm | 25 ------------- src/lib/Hydra/Plugin/BazaarInput.pm | 1 + src/lib/Hydra/Plugin/DarcsInput.pm | 1 + src/lib/Hydra/Plugin/MercurialInput.pm | 1 + src/lib/Hydra/Plugin/SubversionInput.pm | 1 + src/script/hydra-eval-jobset | 1 + t/Hydra/Controller/Jobset/channel.t | 4 +-- t/Hydra/Plugin/RunCommand/basic.t | 2 ++ t/lib/CliRunners.pm | 22 +----------- t/lib/Setup.pm | 3 +- .../build-locally-with-substitutable-path.t | 4 +++ t/scripts/hydra-create-user.t | 1 + t/scripts/hydra-init.t | 1 + t/scripts/hydra-send-stats.t | 1 + .../hydra-update-gc-roots/update-gc-roots.t | 1 + 16 files changed, 54 insertions(+), 50 deletions(-) create mode 100644 src/lib/Hydra/Helper/Exec.pm diff --git a/src/lib/Hydra/Helper/Exec.pm b/src/lib/Hydra/Helper/Exec.pm new file mode 100644 index 00000000..3e24d60d --- /dev/null +++ b/src/lib/Hydra/Helper/Exec.pm @@ -0,0 +1,35 @@ +use warnings; +use strict; +use IPC::Run; + +package Hydra::Helper::Exec; +our @ISA = qw(Exporter); +our @EXPORT = qw( + captureStdoutStderr + captureStdoutStderrWithStdin +); + +sub captureStdoutStderr { + my ($timeout, @cmd) = @_; + + return captureStdoutStderrWithStdin($timeout, \@cmd, ""); +} + +sub captureStdoutStderrWithStdin { + my ($timeout, $cmd, $stdin) = @_; + my $stdout; + my $stderr; + + eval { + local $SIG{ALRM} = sub { die "timeout\n" }; # NB: \n required + alarm $timeout; + IPC::Run::run($cmd, \$stdin, \$stdout, \$stderr); + alarm 0; + 1; + } or do { + die unless $@ eq "timeout\n"; # propagate unexpected errors + return (-1, $stdout, ($stderr // "") . "timeout\n"); + }; + + return ($?, $stdout, $stderr); +} diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm index 6f7852af..796d9844 100644 --- a/src/lib/Hydra/Helper/Nix.pm +++ b/src/lib/Hydra/Helper/Nix.pm @@ -18,8 +18,6 @@ use UUID4::Tiny qw(is_uuid4_string); our @ISA = qw(Exporter); our @EXPORT = qw( cancelBuilds - captureStdoutStderr - captureStdoutStderrWithStdin constructRunCommandLogPath findLog gcRootFor @@ -429,30 +427,7 @@ sub pathIsInsidePrefix { } -sub captureStdoutStderr { - my ($timeout, @cmd) = @_; - return captureStdoutStderrWithStdin($timeout, \@cmd, ""); -} - -sub captureStdoutStderrWithStdin { - my ($timeout, $cmd, $stdin) = @_; - my $stdout; - my $stderr; - - eval { - local $SIG{ALRM} = sub { die "timeout\n" }; # NB: \n required - alarm $timeout; - IPC::Run::run($cmd, \$stdin, \$stdout, \$stderr); - alarm 0; - 1; - } or do { - die unless $@ eq "timeout\n"; # propagate unexpected errors - return (-1, $stdout, ($stderr // "") . "timeout\n"); - }; - - return ($?, $stdout, $stderr); -} sub run { diff --git a/src/lib/Hydra/Plugin/BazaarInput.pm b/src/lib/Hydra/Plugin/BazaarInput.pm index 458f5141..230d108b 100644 --- a/src/lib/Hydra/Plugin/BazaarInput.pm +++ b/src/lib/Hydra/Plugin/BazaarInput.pm @@ -5,6 +5,7 @@ use warnings; use parent 'Hydra::Plugin'; use Digest::SHA qw(sha256_hex); use File::Path; +use Hydra::Helper::Exec; use Hydra::Helper::Nix; use Nix::Store; diff --git a/src/lib/Hydra/Plugin/DarcsInput.pm b/src/lib/Hydra/Plugin/DarcsInput.pm index 42f6a9a4..b7f3db55 100644 --- a/src/lib/Hydra/Plugin/DarcsInput.pm +++ b/src/lib/Hydra/Plugin/DarcsInput.pm @@ -5,6 +5,7 @@ use warnings; use parent 'Hydra::Plugin'; use Digest::SHA qw(sha256_hex); use File::Path; +use Hydra::Helper::Exec; use Hydra::Helper::Nix; use Nix::Store; diff --git a/src/lib/Hydra/Plugin/MercurialInput.pm b/src/lib/Hydra/Plugin/MercurialInput.pm index 3aaa0ba3..921262ad 100644 --- a/src/lib/Hydra/Plugin/MercurialInput.pm +++ b/src/lib/Hydra/Plugin/MercurialInput.pm @@ -6,6 +6,7 @@ use parent 'Hydra::Plugin'; use Digest::SHA qw(sha256_hex); use File::Path; use Hydra::Helper::Nix; +use Hydra::Helper::Exec; use Nix::Store; use Fcntl qw(:flock); diff --git a/src/lib/Hydra/Plugin/SubversionInput.pm b/src/lib/Hydra/Plugin/SubversionInput.pm index 7879364e..456c6892 100644 --- a/src/lib/Hydra/Plugin/SubversionInput.pm +++ b/src/lib/Hydra/Plugin/SubversionInput.pm @@ -4,6 +4,7 @@ use strict; use warnings; use parent 'Hydra::Plugin'; use Digest::SHA qw(sha256_hex); +use Hydra::Helper::Exec; use Hydra::Helper::Nix; use IPC::Run; use Nix::Store; diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset index a68fe858..450c5f50 100755 --- a/src/script/hydra-eval-jobset +++ b/src/script/hydra-eval-jobset @@ -11,6 +11,7 @@ use File::Slurper qw(read_text); use Hydra::Helper::AddBuilds; use Hydra::Helper::CatalystUtils; use Hydra::Helper::Email; +use Hydra::Helper::Exec; use Hydra::Helper::Nix; use Hydra::Model::DB; use Hydra::Plugin; diff --git a/t/Hydra/Controller/Jobset/channel.t b/t/Hydra/Controller/Jobset/channel.t index 7fa7498e..1410260d 100644 --- a/t/Hydra/Controller/Jobset/channel.t +++ b/t/Hydra/Controller/Jobset/channel.t @@ -6,13 +6,13 @@ use IO::Uncompress::Bunzip2 qw(bunzip2); use Archive::Tar; use JSON::MaybeXS qw(decode_json); use Data::Dumper; +use Hydra::Helper::Exec; my %ctx = test_init( use_external_destination_store => 0 ); require Hydra::Schema; require Hydra::Model::DB; -require Hydra::Helper::Nix; use Test2::V0; require Catalyst::Test; @@ -47,7 +47,7 @@ $tar->extract_file("channel/default.nix", $defaultnix); print STDERR $tar->get_content("channel/default.nix"); -(my $status, my $stdout, my $stderr) = Hydra::Helper::Nix::captureStdoutStderr(5, "nix-env", "--json", "--query", "--available", "--attr-path", "--file", $defaultnix); +(my $status, my $stdout, my $stderr) = captureStdoutStderr(5, "nix-env", "--json", "--query", "--available", "--attr-path", "--file", $defaultnix); is($stderr, "", "Stderr should be empty"); is($status, 0, "Querying the packages should succeed"); diff --git a/t/Hydra/Plugin/RunCommand/basic.t b/t/Hydra/Plugin/RunCommand/basic.t index 6e31aed3..e9fc730b 100644 --- a/t/Hydra/Plugin/RunCommand/basic.t +++ b/t/Hydra/Plugin/RunCommand/basic.t @@ -13,6 +13,8 @@ my %ctx = test_init( require Hydra::Schema; require Hydra::Model::DB; +require Hydra::Helper::Nix; + use Test2::V0; diff --git a/t/lib/CliRunners.pm b/t/lib/CliRunners.pm index f803c2db..885f2ae4 100644 --- a/t/lib/CliRunners.pm +++ b/t/lib/CliRunners.pm @@ -2,35 +2,15 @@ use warnings; use strict; package CliRunners; +use Hydra::Helper::Exec; our @ISA = qw(Exporter); our @EXPORT = qw( - captureStdoutStderr - captureStdoutStderrWithStdin evalFails evalSucceeds runBuild sendNotifications ); - -sub captureStdoutStderr { - # "Lazy"-load Hydra::Helper::Nix to avoid the compile-time - # import of Hydra::Model::DB. Early loading of the DB class - # causes fixation of the DSN, and we need to fixate it after - # the temporary DB is setup. - require Hydra::Helper::Nix; - return Hydra::Helper::Nix::captureStdoutStderr(@_) -} - -sub captureStdoutStderrWithStdin { - # "Lazy"-load Hydra::Helper::Nix to avoid the compile-time - # import of Hydra::Model::DB. Early loading of the DB class - # causes fixation of the DSN, and we need to fixate it after - # the temporary DB is setup. - require Hydra::Helper::Nix; - return Hydra::Helper::Nix::captureStdoutStderrWithStdin(@_) -} - sub evalSucceeds { my ($jobset) = @_; my ($res, $stdout, $stderr) = captureStdoutStderr(60, ("hydra-eval-jobset", $jobset->project->name, $jobset->name)); diff --git a/t/lib/Setup.pm b/t/lib/Setup.pm index 56e01bdc..09e0def7 100644 --- a/t/lib/Setup.pm +++ b/t/lib/Setup.pm @@ -8,12 +8,11 @@ use File::Temp; use File::Path qw(make_path); use File::Basename; use Cwd qw(abs_path getcwd); +use Hydra::Helper::Exec; use CliRunners; our @ISA = qw(Exporter); our @EXPORT = qw( - captureStdoutStderr - captureStdoutStderrWithStdin createBaseJobset createJobsetWithOneInput evalFails diff --git a/t/queue-runner/build-locally-with-substitutable-path.t b/t/queue-runner/build-locally-with-substitutable-path.t index f335d284..4f2c3057 100644 --- a/t/queue-runner/build-locally-with-substitutable-path.t +++ b/t/queue-runner/build-locally-with-substitutable-path.t @@ -3,10 +3,14 @@ use warnings; use Setup; use Data::Dumper; use Test2::V0; +use Hydra::Helper::Exec; + my $ctx = test_context( use_external_destination_store => 1 ); +require Hydra::Helper::Nix; + # This test is regarding https://github.com/NixOS/hydra/pull/1126 # # A hydra instance was regularly failing to build derivations with: diff --git a/t/scripts/hydra-create-user.t b/t/scripts/hydra-create-user.t index 66d019ef..71a5eda3 100644 --- a/t/scripts/hydra-create-user.t +++ b/t/scripts/hydra-create-user.t @@ -2,6 +2,7 @@ use strict; use warnings; use Setup; use Test2::V0; +use Hydra::Helper::Exec; my $ctx = test_context(); my $db = $ctx->db(); diff --git a/t/scripts/hydra-init.t b/t/scripts/hydra-init.t index 3fbcb629..bd5bd4bf 100644 --- a/t/scripts/hydra-init.t +++ b/t/scripts/hydra-init.t @@ -2,6 +2,7 @@ use feature 'unicode_strings'; use strict; use warnings; use Setup; +use Hydra::Helper::Exec; my %ctx = test_init(); diff --git a/t/scripts/hydra-send-stats.t b/t/scripts/hydra-send-stats.t index 1c0e1eeb..dddeab1a 100644 --- a/t/scripts/hydra-send-stats.t +++ b/t/scripts/hydra-send-stats.t @@ -2,6 +2,7 @@ use feature 'unicode_strings'; use strict; use warnings; use Setup; +use Hydra::Helper::Exec; my %ctx = test_init(); diff --git a/t/scripts/hydra-update-gc-roots/update-gc-roots.t b/t/scripts/hydra-update-gc-roots/update-gc-roots.t index 9729e066..d47e36c1 100644 --- a/t/scripts/hydra-update-gc-roots/update-gc-roots.t +++ b/t/scripts/hydra-update-gc-roots/update-gc-roots.t @@ -3,6 +3,7 @@ use strict; use warnings; use Setup; use Test2::V0; +use Hydra::Helper::Exec; my $ctx = test_context(); my $builds = $ctx->makeAndEvaluateJobset(