From efd5f0d0fdcaa30876e210d22876fedf8240cc3c Mon Sep 17 00:00:00 2001 From: Michael Schroeder Date: Tue, 27 Nov 2018 14:05:43 +0100 Subject: [PATCH] [backend] always run services on expanded links Services are run on checkin, which usually works with expanded links (keeplink mode), so this should not make a difference. What's changed is if a remoterun is triggered, where the current way of running the service on the unexpanded link is a source of lots of confusion. The new way to run on expanded links is way more useful. Lets see how it works in practice. Also, the link code was changed to always ignore service results of the link target if the link itself contains a service result. This was only done for branches before, it is now more consistent. --- src/backend/BSSrcServer/Link.pm | 19 +++++++-- src/backend/BSSrcServer/Service.pm | 66 +++++++++++------------------- src/backend/bs_servicedispatch | 34 ++++++--------- 3 files changed, 53 insertions(+), 66 deletions(-) diff --git a/src/backend/BSSrcServer/Link.pm b/src/backend/BSSrcServer/Link.pm index 33d8932a5ff..7af28da0c55 100644 --- a/src/backend/BSSrcServer/Link.pm +++ b/src/backend/BSSrcServer/Link.pm @@ -215,6 +215,12 @@ sub applylink { } my $flnk = BSRevision::lsrev($llnk); my $fsrc = BSRevision::lsrev($lsrc); + my $lnkhasservice; + $lnkhasservice = 1 if grep {/^_service/} keys %$flnk; # do we have service files? + if ($lnkhasservice) { + # drop all service files if the link also has service files + delete $fsrc->{$_} for grep {/^_service[:_]/} keys %$fsrc; + } my $l = $llnk->{'link'}; my $patches = $l->{'patches'} || {}; my @patches = (); @@ -311,10 +317,9 @@ sub applylink { return "baserev $baserev does not exist" unless $fbas; return "baserev is link" if $fbas->{'_link'}; - # ignore linked generated service files if our link contains service files - if (grep {/^_service/} keys %$flnk) { + # also delete service file from the base + if ($lnkhasservice) { delete $fbas->{$_} for grep {/^_service[:_]/} keys %$fbas; - delete $fsrc->{$_} for grep {/^_service[:_]/} keys %$fsrc; } # do 3-way merge my %destnames = (%$fsrc, %$flnk); @@ -535,6 +540,7 @@ sub handlelinks { my $projid = $rev->{'project'}; my $packid = $rev->{'package'}; my $linkrev = $rev->{'linkrev'}; + my $ignoreserviceerrors = $rev->{'ignoreserviceerrors'}; push @linkinfo, {'project' => $projid, 'package' => $packid, 'srcmd5' => $rev->{'srcmd5'}, 'rev' => $rev->{'rev'}}; delete $rev->{'srcmd5'}; delete $rev->{'linkrev'}; @@ -594,10 +600,17 @@ sub handlelinks { return "linked package '$packid' is strange" unless $lrev->{'srcmd5'} =~ /^[0-9a-f]{32}$/; $lrev->{'vrev'} = $l->{'vrev'} if defined $l->{'vrev'}; undef $files; + my $lrev_copy; + $lrev_copy = { %$lrev } if $ignoreserviceerrors; eval { # links point to expanded services $files = $lsrev_linktarget->($lrev); }; + if ($@ && $ignoreserviceerrors) { + # try expanding without service + %$lrev = %$lrev_copy; # restore lrev + eval { $files = BSRevision::lsrev($lrev) }; + } if ($@) { my $error = $@; chomp $error; diff --git a/src/backend/BSSrcServer/Service.pm b/src/backend/BSSrcServer/Service.pm index 8d4ad824a19..2322c3fa627 100644 --- a/src/backend/BSSrcServer/Service.pm +++ b/src/backend/BSSrcServer/Service.pm @@ -216,6 +216,7 @@ sub runservice { }; if ($@) { warn($@); + undef $oldfiles; undef $oldfilesrev; next if $@ =~ /service in progress/; } @@ -263,37 +264,28 @@ sub runservice { return; } - my $linkfiles; - my $linksrcmd5; + my $sendfiles = $files; # files we send to the service daemon + + # expand links + my $sendsrcmd5; if ($files->{'_link'}) { - # make sure it's a branch - my $l = BSRevision::revreadxml($rev, '_link', $files->{'_link'}, $BSXML::link, 1); - if (!$l || !$l->{'patches'} || @{$l->{'patches'}->{''} || []} != 1 || (keys %{$l->{'patches'}->{''}->[0]})[0] ne 'branch') { - #addrev_service($cgi, $rev, $files, "services only work on branches\n"); - #return; - # uh oh, not a branch! - $linkfiles = { %$files }; - delete $files->{'/SERVICE'}; - eval { - my $lrev = {%$rev, 'linkrev' => 'base'}; - $files = BSSrcServer::Link::handlelinks($lrev, $files); - die("bad link: $files\n") unless ref $files; - $linksrcmd5 = $lrev->{'srcmd5'}; - }; - if ($@) { - if (($@ =~ /service in progress/) && $linkfiles->{'/SERVICE'}) { - # delay, hope for an event. remove lock for now to re-trigger the service run. - BSSrcrep::addmeta_serviceerror($rev->{'project'}, $rev->{'package'}, $linkfiles->{'/SERVICE'}, undef); - return; - } - $files = $linkfiles; - addrev_service($cgi, $rev, $files, $@); - return; - } - $files->{'/SERVICE'} = $linkfiles->{'/SERVICE'} if $linkfiles->{'/SERVICE'} + $sendfiles = { %$files }; + delete $sendfiles->{'/SERVICE'}; + eval { + my $lrev = {%$rev, 'ignoreserviceerrors' => 1}; + $sendfiles = BSSrcServer::Link::handlelinks($lrev, $sendfiles); + die("bad link: $sendfiles\n") unless ref $sendfiles; + $sendsrcmd5 = $lrev->{'srcmd5'}; + }; + if ($@) { + addrev_service($cgi, $rev, $files, $@); + return; } + # drop all sevice files + delete $sendfiles->{$_} for grep {/^_service:/} keys %$sendfiles; } + # handoff to dispatcher if configured if ($files->{'/SERVICE'} && $BSConfig::servicedispatch) { my $projectservicesmd5; if ($projectservices) { @@ -309,7 +301,7 @@ sub runservice { 'srcmd5' => $rev->{'srcmd5'}, 'rev' => $rev->{'rev'}, }; - $ev->{'linksrcmd5'} = $linksrcmd5 if $linksrcmd5; + $ev->{'linksrcmd5'} = $sendsrcmd5 if $sendsrcmd5; $ev->{'projectservicesmd5'} = $projectservicesmd5 if $projectservicesmd5; $ev->{'oldsrcmd5'} = $oldfilesrev->{'srcmd5'} if %$oldfiles && $oldfilesrev; mkdir_p("$eventdir/servicedispatch"); @@ -327,9 +319,9 @@ sub runservice { BSUtil::touch($lockfile); } - my @send = map {BSRevision::revcpiofile($rev, $_, $files->{$_})} grep {$_ ne '/SERVICE'} sort(keys %$files); + my @send = map {BSRevision::revcpiofile($rev, $_, $sendfiles->{$_})} grep {$_ ne '/SERVICE'} sort(keys %$sendfiles); push @send, {'name' => '_serviceproject', 'data' => BSUtil::toxml($projectservices, $BSXML::services)} if $projectservices; - push @send, map {BSRevision::revcpiofile($rev, $_, $oldfiles->{$_})} grep {!$files->{$_}} sort(keys %$oldfiles); + push @send, map {BSRevision::revcpiofile($rev, $_, $oldfiles->{$_})} grep {!$sendfiles->{$_}} sort(keys %$oldfiles); # run the source update in own process (do not wait for it) my $pid = xfork(); @@ -377,12 +369,9 @@ sub runservice { for my $pfile (ls($odir)) { if ($pfile eq '.errors') { my $e = readstr("$odir/.errors"); - $e ||= 'empty .errors file'; - die($e); + die($e || "empty .errors file\n"); } - unless ($pfile =~ /^_service[_:]/) { - die("service returned a non-_service file: $pfile\n"); - } + die("service returned a non-_service file: $pfile\n") unless $pfile =~ /^_service[_:]/; BSVerify::verify_filename($pfile); $files->{$pfile} = BSSrcrep::addfile($projid, $packid, "$odir/$pfile", $pfile); } @@ -394,13 +383,6 @@ sub runservice { } BSUtil::cleandir($odir); rmdir($odir); - if ($linkfiles) { - # argh, a link! put service run result in old filelist - if (!$error) { - $linkfiles->{$_} = $files->{$_} for grep {/^_service[_:]/} keys %$files; - } - $files = $linkfiles; - } addrev_service($cgi, $rev, $files, $error); exit(0); } diff --git a/src/backend/bs_servicedispatch b/src/backend/bs_servicedispatch index fac91465c82..d8dedad2ccc 100755 --- a/src/backend/bs_servicedispatch +++ b/src/backend/bs_servicedispatch @@ -92,7 +92,7 @@ sub getrev { } sub runservice { - my ($projid, $packid, $servicemark, $srcmd5, $revid, $linksrcmd5, $projectservicesmd5, $oldsrcmd5) = @_; + my ($projid, $packid, $servicemark, $srcmd5, $revid, $sendsrcmd5, $projectservicesmd5, $oldsrcmd5) = @_; print "dispatching service $projid/$packid $servicemark $srcmd5\n"; # get revision and file list @@ -113,12 +113,13 @@ sub runservice { my $serviceerror = BSSrcrep::getserviceerror($projid, $packid, $servicemark); return if $serviceerror ne 'service in progress'; - # handle link case - my $linkfiles; - if ($linksrcmd5) { - $linkfiles = $files; - my $lrev = getrev($projid, $packid, $linksrcmd5); - $files = BSRevision::lsrev($lrev); + # get files to send to service daemon + my $sendfiles = $files; + if ($sendsrcmd5) { + my $lrev = getrev($projid, $packid, $sendsrcmd5); + $sendfiles = BSRevision::lsrev($lrev); + # drop all service files + delete $sendfiles->{$_} for grep {/^_service:/} keys %$sendfiles; } # get old files @@ -126,12 +127,13 @@ sub runservice { if ($oldsrcmd5) { my $oldrev = getrev($projid, $packid, $oldsrcmd5); $oldfiles = BSRevision::lsrev($oldrev); + delete $oldfiles->{$_} for grep {!/^_service:/} keys %$oldfiles; } $oldfiles ||= {}; - my @send = map {BSRevision::revcpiofile($rev, $_, $files->{$_})} sort(keys %$files); + my @send = map {BSRevision::revcpiofile($rev, $_, $sendfiles->{$_})} sort(keys %$sendfiles); push @send, BSRevision::revcpiofile({'project' => $projid, 'package' => '_project'}, '_serviceproject', $projectservicesmd5) if $projectservicesmd5; - push @send, map {BSRevision::revcpiofile($rev, $_, $oldfiles->{$_})} grep {!$files->{$_}} sort(keys %$oldfiles); + push @send, map {BSRevision::revcpiofile($rev, $_, $oldfiles->{$_})} grep {!$sendfiles->{$_}} sort(keys %$oldfiles); my $odir = "$uploaddir/runservice$$"; BSUtil::cleandir($odir) if -d $odir; @@ -164,12 +166,9 @@ sub runservice { for my $pfile (ls($odir)) { if ($pfile eq '.errors') { my $e = readstr("$odir/.errors"); - $e ||= 'empty .errors file'; - die($e); - } - unless ($pfile =~ /^_service[_:]/) { - die("service returned a non-_service file: $pfile\n"); + die($e || "empty .errors file\n"); } + die("service returned a non-_service file: $pfile\n") unless $pfile =~ /^_service[_:]/; BSVerify::verify_filename($pfile); $files->{$pfile} = BSSrcrep::addfile($projid, $packid, "$odir/$pfile", $pfile); } @@ -183,13 +182,6 @@ sub runservice { } BSUtil::cleandir($odir); rmdir($odir); - if ($linkfiles) { - # argh, a link! put service run result in old filelist - if (!$error) { - $linkfiles->{$_} = $files->{$_} for grep {/^_service[_:]/} keys %$files; - } - $files = $linkfiles; - } addrev_service($rev, $servicemark, $files, $error); }