Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor generic git functions into git module #5863

Merged
merged 4 commits into from
Aug 22, 2024
Merged

Refactor generic git functions into git module #5863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9196b67
WIP refactor generic git functions into git module
r-richardson Aug 15, 2024
bdd93d4
Refactor format_git_error
r-richardson Aug 16, 2024
677243a
Improve git error handling and refactor $path
r-richardson Aug 16, 2024
e7eb938
Avoid array recreation with _run_cmd helper method
r-richardson Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 67 additions & 25 deletions lib/OpenQA/Git.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
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);

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

sub _prepare_git_command ($self, $args = undef) {
my $dir = $args->{dir} // $self->dir;
sub _validate_attributes ($self) {
for my $mandatory_property (qw(app dir user)) {
die "no $mandatory_property specified" unless $self->$mandatory_property();
}
}

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

sub _ssh_git_cmd ($self, $args) {
run_cmd_with_log_return_error(['env', 'GIT_SSH_COMMAND=ssh -oBatchMode=yes', $self->_prepare_git_command, @$args]);
}

sub _prepare_git_command ($self) {
my $dir = $self->dir;
b10n1k marked this conversation as resolved.
Show resolved Hide resolved
die 'no valid directory was found during git preparation' unless $dir;
if ($dir !~ /^\//) {
my $absolute_path = abs_path($dir);
$dir = $absolute_path if ($absolute_path);
}
return ('git', '-C', $dir);
}

sub _format_git_error ($result, $error_message) {
sub _format_git_error ($self, $result, $error_message) {
my $dir = $self->dir;
if ($result->{stderr} or $result->{stdout}) {
$error_message .= ': ' . $result->{stdout} . $result->{stderr};
$error_message .= " ($dir): " . $result->{stdout} . $result->{stderr};
}
r-richardson marked this conversation as resolved.
Show resolved Hide resolved
return $error_message;
}

sub _validate_attributes ($self) {
for my $mandatory_property (qw(app dir user)) {
die "no $mandatory_property specified" unless $self->$mandatory_property();
}
}

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]);
return _format_git_error($res, 'Unable to fetch from origin master') unless $res->{status};
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']);
return _format_git_error($res, 'Unable to reset repository to HEAD') unless $res->{status};
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]);
return _format_git_error($res, 'Unable to reset repository to origin/master') unless $res->{status};
my $res = $self->_run_cmd(['rebase', $update_branch]);
return $self->_format_git_error($res, 'Unable to reset repository to origin/master') unless $res->{status};
}

return undef;
Expand All @@ -68,30 +77,63 @@ 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}}]);
return _format_git_error($res, "Unable to $cmd via Git") unless $res->{status};
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]);
return _format_git_error($res, 'Unable to commit via Git') unless $res->{status};
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']);
return _format_git_error($res, 'Unable to push Git commit') unless $res->{status};
$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 $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 get_remote_default_branch ($self, $url) {
my $r = $self->_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 ($self, $url) {
my $r = $self->_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 $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 $r = $self->_ssh_git_cmd(['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 $r = $self->_run_cmd(['reset', '--hard', "origin/$branch"]);
die $self->_format_git_error($r, "Failed to reset to 'origin/$branch'") unless $r->{status};
}

1;
62 changes: 12 additions & 50 deletions lib/OpenQA/Task/Git/Clone.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

r-richardson marked this conversation as resolved.
Show resolved Hide resolved
package OpenQA::Task::Git::Clone;
use Mojo::Base 'Mojolicious::Plugin', -signatures;
use Mojo::Util 'trim';
use OpenQA::Git;

use OpenQA::Utils qw(run_cmd_with_log_return_error);
use Mojo::File;
Expand All @@ -13,7 +13,6 @@ 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)
Expand Down Expand Up @@ -42,84 +41,47 @@ sub _git_clone_all ($job, $clones) {
for my $path (sort { length($a) <=> length($b) } keys %$clones) {
my $url = $clones->{$path};
die "Don't even think about putting '..' into '$path'." if $path =~ /\.\./;
eval { _git_clone($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 _get_current_branch ($path) {
my $r = run_cmd_with_log_return_error(['git', '-C', $path, 'branch', '--show-current']);
die "Error detecting current branch for '$path': $r->{stderr}" unless $r->{status};
return trim($r->{stdout});
}

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

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

sub _git_clone_url_to_path ($url, $path) {
my $r = run_cmd_with_log_return_error(_ssh_git_cmd(['clone', $url, $path]));
die "Failed to clone $url into '$path': $r->{stderr}" unless $r->{status};
}

sub _git_get_origin_url ($path) {
my $r = run_cmd_with_log_return_error(['git', '-C', $path, 'remote', 'get-url', 'origin']);
die "Failed to get origin url for '$path': $r->{stderr}" unless $r->{status};
return trim($r->{stdout});
}

sub _git_fetch ($path, $branch_arg) {
my $r = run_cmd_with_log_return_error(_ssh_git_cmd(['-C', $path, 'fetch', 'origin', $branch_arg]));
die "Failed to fetch from '$branch_arg': $r->{stderr}" unless $r->{status};
}

sub _git_reset_hard ($path, $branch) {
my $r = run_cmd_with_log_return_error(['git', '-C', $path, 'reset', '--hard', "origin/$branch"]);
die "Failed to reset to 'origin/$branch': $r->{stderr}" unless $r->{status};
}

sub _git_clone ($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;
$url->fragment(undef);
my $remote_default_branch = _get_remote_default_branch($url);
my $remote_default_branch = $git->get_remote_default_branch($url);
$requested_branch ||= $remote_default_branch;
$ctx->debug(qq{Remote default branch $remote_default_branch});
die "Unable to detect remote default branch for '$url'" unless $remote_default_branch;

if (!-d $path) {
_git_clone_url_to_path($url, $path);
$git->git_clone_url($url);
# update local branch to latest remote branch version
_git_fetch($path, "$requested_branch:$requested_branch")
$git->git_fetch("$requested_branch:$requested_branch")
if ($requested_branch ne $remote_default_branch);
}

my $origin_url = _git_get_origin_url($path);
my $origin_url = $git->git_get_origin_url;
if ($url ne $origin_url) {
$ctx->warn("Local checkout at $path has origin $origin_url but requesting to clone from $url");
return;
}

my $current_branch = _get_current_branch($path);
my $current_branch = $git->get_current_branch;
if ($requested_branch eq $current_branch) {
# updating default branch (including checkout)
_git_fetch($path, $requested_branch);
_git_reset_hard($path, $requested_branch);
$git->git_fetch($requested_branch);
$git->git_reset_hard($requested_branch);
}
else {
# updating local branch to latest remote branch version
_git_fetch($path, "$requested_branch:$requested_branch");
$git->git_fetch("$requested_branch:$requested_branch");
}
}

Expand Down
40 changes: 32 additions & 8 deletions t/14-grutasks.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use OpenQA::Jobs::Constants;
use OpenQA::JobDependencies::Constants;
use OpenQA::JobGroupDefaults;
use OpenQA::Schema::Result::Jobs;
use OpenQA::Git;
use OpenQA::Task::Git::Clone;
use File::Copy;
require OpenQA::Test::Database;
Expand Down Expand Up @@ -645,12 +646,13 @@ subtest 'handling dying GRU task' => sub {
};

subtest 'git clone' => sub {
my $openqa_utils = Test::MockModule->new('OpenQA::Task::Git::Clone');
my $openqa_git = Test::MockModule->new('OpenQA::Git');
my @mocked_git_calls;
$openqa_utils->redefine(
$openqa_git->redefine(
run_cmd_with_log_return_error => sub ($cmd) {
#warn '!!!run_cmd_with_log_return_error' ;
my $stdout = '';
$stdout = 'ref: refs/heads/master HEAD' if $cmd->[3] eq 'ls-remote';
$stdout = 'ref: refs/heads/master HEAD' if $cmd->[5] // '' eq 'ls-remote';
$stdout = 'http://localhost/foo.git' if $cmd->[4] eq 'get-url';
$stdout = 'master' if $cmd->[3] eq 'branch';
my $git_call = join(' ', @$cmd);
Expand All @@ -669,20 +671,42 @@ subtest 'git clone' => sub {
};
my $res = run_gru_job($t->app, 'git_clone', $clone_dirs, {priority => 10});
is $res->{result}, 'Job successfully executed', 'minion job result indicates success';
like $mocked_git_calls[3], qr'git -C /etc/ fetch origin master', 'fetch origin master for /etc/';
like $mocked_git_calls[4], qr'reset --hard origin/master', 'reset origin/master for /etc/';
like $mocked_git_calls[8], qr'git -C /root/ fetch origin foobranch:foobranch', 'fetch non-default branch';
like $mocked_git_calls[10], qr'git clone http://localhost/bar.git /this_directory_does_not_exist/',

is $mocked_git_calls[0],
'env GIT_SSH_COMMAND=ssh -oBatchMode=yes git -C /etc/ ls-remote --symref http://localhost/foo.git HEAD',
'git ssh ls-remote call';
is $mocked_git_calls[1], 'git -C /etc/ remote get-url origin', 'get remote URL for /etc/';
is $mocked_git_calls[2], 'git -C /etc/ branch --show-current', 'show current branch for /etc/';
is $mocked_git_calls[3], 'env GIT_SSH_COMMAND=ssh -oBatchMode=yes git -C /etc/ fetch origin master',
'reset origin/master for /etc/';
is $mocked_git_calls[4], 'git -C /etc/ reset --hard origin/master', 'git hard reset to origin/master for /etc/';
is $mocked_git_calls[5],
'env GIT_SSH_COMMAND=ssh -oBatchMode=yes git -C /root/ ls-remote --symref http://localhost/foo.git HEAD',
'ls-remote for /root/';
is $mocked_git_calls[6], 'git -C /root/ remote get-url origin', 'get remote URL for /root/';
is $mocked_git_calls[7], 'git -C /root/ branch --show-current', 'show current branch for /root/';
is $mocked_git_calls[8], 'env GIT_SSH_COMMAND=ssh -oBatchMode=yes git -C /root/ fetch origin foobranch:foobranch',
'fetch non-default branch';
is $mocked_git_calls[9],
'env GIT_SSH_COMMAND=ssh -oBatchMode=yes git -C /this_directory_does_not_exist/ ls-remote --symref http://localhost/bar.git HEAD',
'ssh git ls-remote for ';
is $mocked_git_calls[10],
'env GIT_SSH_COMMAND=ssh -oBatchMode=yes git -C /this_directory_does_not_exist/ clone http://localhost/bar.git /this_directory_does_not_exist/',
'clone to /this_directory_does_not_exist/';
is $mocked_git_calls[11], 'git -C /this_directory_does_not_exist/ remote get-url origin',
'get remote URL for /this_directory_does_not_exist/';

subtest 'git clone retried on failure' => sub {
$ENV{OPENQA_GIT_CLONE_RETRIES} = 1;
$openqa_utils->redefine(_git_clone => sub (@) { die "fake error\n" });
my $openqa_clone = Test::MockModule->new('OpenQA::Task::Git::Clone');
$openqa_clone->redefine(_git_clone => sub (@) { die "fake error\n" });
$res = run_gru_job($t->app, 'git_clone', $clone_dirs, {priority => 10});
is $res->{retries}, 1, 'job retries incremented';
is $res->{state}, 'inactive', 'job set back to inactive';
};
subtest 'git clone fails when all retry attempts exhausted' => sub {
my $openqa_clone = Test::MockModule->new('OpenQA::Task::Git::Clone');
$openqa_clone->redefine(_git_clone => sub (@) { die "fake error\n" });
$ENV{OPENQA_GIT_CLONE_RETRIES} = 0;
$res = run_gru_job($t->app, 'git_clone', $clone_dirs, {priority => 10});
is $res->{retries}, 0, 'job retries not incremented';
Expand Down
5 changes: 3 additions & 2 deletions t/16-utils-runcmd.t
View file
Open in desktop
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
Loading