From c631e72e90a0da5d1927cef12ac34439aaddbea0 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Mar 21 2020 21:50:39 +0000 Subject: Backport #2856 to fix extracting compressed download assets Signed-off-by: Adam Williamson --- diff --git a/0001-Fix-extracting-compressed-downloaded-files.patch b/0001-Fix-extracting-compressed-downloaded-files.patch new file mode 100644 index 0000000..ef2a24e --- /dev/null +++ b/0001-Fix-extracting-compressed-downloaded-files.patch @@ -0,0 +1,110 @@ +From 01f61780af32eaf71d51574e784a2002833189c3 Mon Sep 17 00:00:00 2001 +From: Adam Williamson +Date: Fri, 20 Mar 2020 12:15:29 -0700 +Subject: [PATCH] Fix extracting compressed downloaded files + +This regressed in #2736. When we want to download a compressed +file and extract it to produce the 'final' asset, we'll be doing +something like downloading 'http://example.com/file.img.gz' to +an asset called 'file.img'. After we download the asset, we write +it to a temporary file and feed that temporary file into +Archive::Extract, then extract it to the final asset location. +Archive::Extract uses the filename of the input file to determine +its type, so we should base the name of the temporary file on the +path component of the URL (where the compression is indicated) - +'file.img.gz' in this case - and not on the final target filename +(where the compression is not indicated) - 'file.img' in this +case. Before #2736, that's what the code did, but in #2736 it was +changed so the temporary filename is based on the target file. +I guess it was hard for @kraih to work this out without a real- +world example to understand how it's actually used in practice +(I think only Fedora uses this feature). + +This returns the code to something similar to how it was before, +but using `path()->to_string` as @kraih's version did. It also +tweaks the test cases, which were clearly the wrong way round +once you know how this is supposed to work (it doesn't make +sense to expect extracting 'test.gz' to 'test.gz' to succeed, +but extracting 'test.gz' to 'test' to fail). + +Signed-off-by: Adam Williamson +--- + lib/OpenQA/Downloader.pm | 2 +- + t/25-downloader.t | 16 ++++++++-------- + t/lib/OpenQA/Test/Utils.pm | 5 +++++ + 3 files changed, 14 insertions(+), 9 deletions(-) + +diff --git a/lib/OpenQA/Downloader.pm b/lib/OpenQA/Downloader.pm +index 52253c5fe..563b4d77b 100644 +--- a/lib/OpenQA/Downloader.pm ++++ b/lib/OpenQA/Downloader.pm +@@ -97,7 +97,7 @@ sub _get { + if ($size == $headers->content_length) { + + if ($options->{extract}) { +- my $tempfile = path($ENV{MOJO_TMPDIR}, $file)->to_string; ++ my $tempfile = path($ENV{MOJO_TMPDIR}, Mojo::URL->new($url)->path->parts->[-1])->to_string; + $log->info(qq{Extracting "$tempfile" to "$target"}); + $asset->move_to($tempfile); + +diff --git a/t/25-downloader.t b/t/25-downloader.t +index f5feb5d85..e6b4f530b 100644 +--- a/t/25-downloader.t ++++ b/t/25-downloader.t +@@ -160,29 +160,29 @@ subtest 'Size differs' => sub { + }; + + subtest 'Decompressing archive failed' => sub { +- $to = $tempdir->child('test'); +- my $from = "http://$host/test.gz"; ++ $to = $tempdir->child('test.gz'); ++ my $from = "http://$host/test"; + ok !$downloader->download($from, $to, {extract => 1}), 'Failed'; + + ok !-e $to, 'File not downloaded'; + +- like $cache_log, qr/Downloading "test" from "$from"/, 'Download attempt'; +- like $cache_log, qr/Extracting ".*test" to ".*test"/, 'Extracting download'; ++ like $cache_log, qr/Downloading "test.gz" from "$from"/, 'Download attempt'; ++ like $cache_log, qr/Extracting ".*test" to ".*test.gz"/, 'Extracting download'; + like $cache_log, qr/Extracting ".*test" failed: Could not determine archive type/, 'Extracting failed'; + $cache_log = ''; + }; + + subtest 'Decompressing archive' => sub { +- $to = $tempdir->child('test.gz'); ++ $to = $tempdir->child('test'); + my $from = "http://$host/test.gz"; + ok $downloader->download($from, $to, {extract => 1}), 'Success'; + + ok -e $to, 'File downloaded'; + is $to->slurp, 'This file was compressed!', 'File was decompressed'; + +- like $cache_log, qr/Downloading "test.gz" from "$from"/, 'Download attempt'; +- like $cache_log, qr/Extracting ".*test.gz" to ".*test.gz"/, 'Extracting download'; +- unlike $cache_log, qr/Extracting ".*test" failed:/, 'Extracting did not fail'; ++ like $cache_log, qr/Downloading "test" from "$from"/, 'Download attempt'; ++ like $cache_log, qr/Extracting ".*test.gz" to ".*test"/, 'Extracting download'; ++ unlike $cache_log, qr/Extracting ".*test.gz" failed:/, 'Extracting did not fail'; + $cache_log = ''; + }; + +diff --git a/t/lib/OpenQA/Test/Utils.pm b/t/lib/OpenQA/Test/Utils.pm +index 476d43a92..625a55a75 100644 +--- a/t/lib/OpenQA/Test/Utils.pm ++++ b/t/lib/OpenQA/Test/Utils.pm +@@ -89,6 +89,11 @@ sub fake_asset_server { + my $archive = gzip 'This file was compressed!'; + $c->render(data => $archive); + }); ++ $r->get( ++ '/test' => sub { ++ my $c = shift; ++ $c->render(text => 'This file was not compressed!', format => 'txt'); ++ }); + $r->get( + '/tests/:job/asset/:type/:filename' => sub { + my $c = shift; +-- +2.25.1 + diff --git a/openqa.spec b/openqa.spec index 836d638..e3b360b 100644 --- a/openqa.spec +++ b/openqa.spec @@ -66,7 +66,7 @@ Name: openqa Version: %{github_version} -Release: 45%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} +Release: 46%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} Summary: OS-level automated testing framework License: GPLv2+ Url: http://os-autoinst.github.io/openQA/ @@ -88,6 +88,10 @@ Source3: FedoraMessaging.pm # tests for the fedora-messaging publishing plugin Source4: 23-fedora-messaging.t +# Fix extracting compressed downloaded assets +# https://github.com/os-autoinst/openQA/pull/2856 +Patch0: 0001-Fix-extracting-compressed-downloaded-files.patch + BuildRequires: %{python_scripts_requires} BuildRequires: perl-generators # Standard for packages that have systemd services @@ -549,6 +553,9 @@ fi %{_datadir}/openqa/lib/OpenQA/WebAPI/Plugin/FedoraUpdateRestart.pm %changelog +* Fri Mar 20 2020 Adam Williamson - 4.6-46.20200319git12cea51 +- Backport #2856 to fix extracting compressed download assets + * Thu Mar 19 2020 Adam Williamson - 4.6-45.20200319git12cea51 - Bump to latest git (inc. fix for POO #62417) - Drop workaround for POO #62417