Skip to content

Commit

Permalink
Avoid array recreation with _run_cmd helper method
Browse files Browse the repository at this point in the history 
- Minor fixes
  • Loading branch information
@r-richardson
r-richardson committed Aug 20, 2024
1 parent 677243a commit 3f93a7f
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 43 deletions.
60 changes: 26 additions & 34 deletions lib/OpenQA/Git.pm
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# Copyright 2019 SUSE LLC
# SPDX-License-Identifier: GPL-2.0-or-later

package OpenQA::Git;

use Mojo::Base -base, -signatures;
use Mojo::Util 'trim';
use Cwd 'abs_path';
use OpenQA::Utils qw(run_cmd_with_log_return_error);
use Mojo::File;

has 'app';
has 'dir';
Expand All @@ -22,8 +22,8 @@ sub config ($self, $args = undef) {
return $app->config->{'scm git'};
}

sub _prepare_git_command ($self, $args = undef) {
my $dir = $args->{dir} // $self->dir;
sub _prepare_git_command ($self) {
my $dir = $self->dir;
if ($dir !~ /^\//) {
my $absolute_path = abs_path($dir);
$dir = $absolute_path if ($absolute_path);
Expand All @@ -32,10 +32,9 @@ sub _prepare_git_command ($self, $args = undef) {
}

sub _format_git_error ($self, $result, $error_message) {

my $path = $self->dir;
my $dir = $self->dir;
if ($result->{stderr} or $result->{stdout}) {
$error_message .= "($path): " . $result->{stdout} . $result->{stderr};
$error_message .= " ($dir): " . $result->{stdout} . $result->{stderr};
}
return $error_message;
}
Expand All @@ -46,22 +45,22 @@ sub _validate_attributes ($self) {
}
}

sub _run_cmd ($self, $args) { run_cmd_with_log_return_error([$self->_prepare_git_command, @$args]) }

sub set_to_latest_master ($self, $args = undef) {
$self->_validate_attributes;

my @git = $self->_prepare_git_command($args);

if (my $update_remote = $self->config->{update_remote}) {
my $res = run_cmd_with_log_return_error([@git, 'remote', 'update', $update_remote]);
my $res = $self->_run_cmd(['remote', 'update', $update_remote]);
return $self->_format_git_error($res, 'Unable to fetch from origin master') unless $res->{status};
}

if (my $update_branch = $self->config->{update_branch}) {
if ($self->config->{do_cleanup} eq 'yes') {
my $res = run_cmd_with_log_return_error([@git, 'reset', '--hard', 'HEAD']);
my $res = $self->_run_cmd(['reset', '--hard', 'HEAD']);
return $self->_format_git_error($res, 'Unable to reset repository to HEAD') unless $res->{status};
}
my $res = run_cmd_with_log_return_error([@git, 'rebase', $update_branch]);
my $res = $self->_run_cmd(['rebase', $update_branch]);
return $self->_format_git_error($res, 'Unable to reset repository to origin/master') unless $res->{status};
}

Expand All @@ -71,74 +70,67 @@ sub set_to_latest_master ($self, $args = undef) {
sub commit ($self, $args = undef) {
$self->_validate_attributes;

my @git = $self->_prepare_git_command($args);
my @files;

# stage changes
for my $cmd (qw(add rm)) {
next unless $args->{$cmd};
push(@files, @{$args->{$cmd}});
my $res = run_cmd_with_log_return_error([@git, $cmd, @{$args->{$cmd}}]);
my $res = $self->_run_cmd([$cmd, @{$args->{$cmd}}]);
return $self->_format_git_error($res, "Unable to $cmd via Git") unless $res->{status};
}

# commit changes
my $message = $args->{message};
my $author = sprintf('--author=%s <%s>', $self->user->fullname, $self->user->email);
my $res = run_cmd_with_log_return_error([@git, 'commit', '-q', '-m', $message, $author, @files]);
my $res = $self->_run_cmd(['commit', '-q', '-m', $message, $author, @files]);
return $self->_format_git_error($res, 'Unable to commit via Git') unless $res->{status};

# push changes
if (($self->config->{do_push} || '') eq 'yes') {
$res = run_cmd_with_log_return_error([@git, 'push']);
$res = $self->_run_cmd(['push']);
return $self->_format_git_error($res, 'Unable to push Git commit') unless $res->{status};
}

return undef;
}

sub get_current_branch ($self) { #
my @git = $self->_prepare_git_command();
my $r = run_cmd_with_log_return_error([@git, 'branch', '--show-current']);
die $self->_format_git_error($r, "Error detecting current branch") unless $r->{status};
sub get_current_branch ($self) {
my $r = $self->_run_cmd(['branch', '--show-current']);
die $self->_format_git_error($r, 'Error detecting current branch') unless $r->{status};
return Mojo::Util::trim($r->{stdout});
}

sub _ssh_git_cmd ($self, $git_args) {
my @git = $self->_prepare_git_command();
return ['env', 'GIT_SSH_COMMAND="ssh -oBatchMode=yes"', 'git', @$git_args];
sub _ssh_git_cmd ($git_args) {
return ['env', 'GIT_SSH_COMMAND=ssh -oBatchMode=yes', 'git', @$git_args];
}

sub get_remote_default_branch ($self, $url) {
my @git = $self->_prepare_git_command();
my $r = run_cmd_with_log_return_error($self->_ssh_git_cmd(['ls-remote', '--symref', $url, 'HEAD']));
my $r = run_cmd_with_log_return_error(_ssh_git_cmd(['ls-remote', '--symref', $url, 'HEAD']));
die $self->_format_git_error($r, "Error detecting remote default branch name for '$url'")
unless $r->{status} && $r->{stdout} =~ /refs\/heads\/(\S+)\s+HEAD/;
return $1;
}

sub git_clone_url_to_path ($self, $url, $path) {
my @git = $self->_prepare_git_command();
my $r = run_cmd_with_log_return_error($self->_ssh_git_cmd(['clone', $url, $path]));
die $self->_format_git_error($r, "Failed to clone $url into '$path'") unless $r->{status};
sub git_clone_url ($self, $url) {
my $r = $self->_run_cmd(_ssh_git_cmd(['clone', $url, $self->dir]));
die $self->_format_git_error($r, "Failed to clone $url") unless $r->{status};
}

sub git_get_origin_url ($self) {
my @git = $self->_prepare_git_command();
my $r = run_cmd_with_log_return_error([@git, 'remote', 'get-url', 'origin']);
die $self->_format_git_error($r, "Failed to get origin url") unless $r->{status};
my $r = $self->_run_cmd(['remote', 'get-url', 'origin']);
die $self->_format_git_error($r, 'Failed to get origin url') unless $r->{status};
return Mojo::Util::trim($r->{stdout});
}

sub git_fetch ($self, $branch_arg) {
my @git = $self->_prepare_git_command();
my $r = run_cmd_with_log_return_error($self->_ssh_git_cmd([@git, 'fetch', 'origin', $branch_arg]));
my $r = $self->_run_cmd(_ssh_git_cmd([@git, 'fetch', 'origin', $branch_arg]));
die $self->_format_git_error($r, "Failed to fetch from '$branch_arg'") unless $r->{status};
}

sub git_reset_hard ($self, $branch) {
my @git = $self->_prepare_git_command();
my $r = run_cmd_with_log_return_error([@git, 'reset', '--hard', "origin/$branch"]);
my $r = $self->_run_cmd(['reset', '--hard', "origin/$branch"]);
die $self->_format_git_error($r, "Failed to reset to 'origin/$branch'") unless $r->{status};
}

Expand Down
17 changes: 12 additions & 5 deletions lib/OpenQA/Task/Git/Clone.pm
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright SUSE LLC
# SPDX-License-Identifier: GPL-2.0-or-later

package OpenQA::Task::Git::Clone;
use Mojo::Base 'Mojolicious::Plugin', -signatures;
use OpenQA::Git;
Expand All @@ -12,11 +13,17 @@ sub register ($self, $app, @) {
$app->minion->add_task(git_clone => \&_git_clone_all);
}

# $clones is a hashref with paths as keys and urls to git repos as values.
# The urls may also refer to a branch via the url fragment.
# If no branch is set, the default branch of the remote (if target path doesn't exist yet)
# or local checkout is used.
# If the target path already exists, an update of the specified branch is performed,
# if the url matches the origin remote.

sub _git_clone_all ($job, $clones) {
my $app = $job->app;
my $job_id = $job->id;


# Prevent multiple git clone tasks for the same path to run in parallel
my @guards;
my $retry_delay = {delay => 30 + int(rand(10))};
Expand All @@ -32,18 +39,18 @@ sub _git_clone_all ($job, $clones) {

# iterate clones sorted by path length to ensure that a needle dir is always cloned after the corresponding casedir
for my $path (sort { length($a) <=> length($b) } keys %$clones) {
my $git = OpenQA::Git->new(app => $app, dir => $path);
my $url = $clones->{$path};
die "Don't even think about putting '..' into '$path'." if $path =~ /\.\./;
eval { _git_clone($git, $job, $ctx, $path, $url) };
eval { _git_clone($app, $job, $ctx, $path, $url) };
next unless my $error = $@;
my $max_retries = $ENV{OPENQA_GIT_CLONE_RETRIES} // 10;
return $job->retry($retry_delay) if $job->retries < $max_retries;
return $job->fail($error);
}
}

sub _git_clone ($git, $job, $ctx, $path, $url) {
sub _git_clone ($app, $job, $ctx, $path, $url) {
my $git = OpenQA::Git->new(app => $app, dir => $path);
$ctx->debug(qq{Updating $path to $url});
$url = Mojo::URL->new($url);
my $requested_branch = $url->fragment;
Expand All @@ -54,7 +61,7 @@ sub _git_clone ($git, $job, $ctx, $path, $url) {
die "Unable to detect remote default branch for '$url'" unless $remote_default_branch;

if (!-d $path) {
$git->git_clone_url_to_path($url, $path);
$git->git_clone_url($url);
# update local branch to latest remote branch version
$git->git_fetch("$requested_branch:$requested_branch")
if ($requested_branch ne $remote_default_branch);
Expand Down
2 changes: 0 additions & 2 deletions t/14-grutasks.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,8 +645,6 @@ subtest 'handling dying GRU task' => sub {
like $associated_job->reason, qr/^preparation failed: Thrown fail at/, 'reason of associated job set';
};



subtest 'git clone' => sub {
my $openqa_git = Test::MockModule->new('OpenQA::Git');
my @mocked_git_calls;
Expand Down
5 changes: 3 additions & 2 deletions t/16-utils-runcmd.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ subtest 'make git commit (error handling)' => sub {
my $res;
stdout_like { $res = $git->commit({cmd => 'status', message => 'test'}) }
qr/.*\[warn\].*fatal: Not a git repository/i, 'git message found';
like $res, qr'^Unable to commit via Git: fatal: (N|n)ot a git repository \(or any', 'Git error message returned';
like $res, qr"^Unable to commit via Git \($empty_tmp_dir\): fatal: (N|n)ot a git repository \(or any",
'Git error message returned';
};

# setup mocking
Expand Down Expand Up @@ -117,7 +118,7 @@ subtest 'git commands with mocked run_cmd_with_log_return_error' => sub {
$mock_return_value{stdout} = '';
is(
$git->set_to_latest_master,
'Unable to fetch from origin master: mocked error',
'Unable to fetch from origin master (foo/bar): mocked error',
'an error occurred on remote update'
);
is_deeply(\@executed_commands, [[qw(git -C foo/bar remote update origin)],], 'git reset not attempted',)
Expand Down

0 comments on commit 3f93a7f

Please sign in to comment.