From 2707465f59206e65773c3ff7b412d09f217e80a9 Mon Sep 17 00:00:00 2001 From: Ondrej Nosek Date: Mar 13 2020 11:20:37 +0000 Subject: Lookaside cache upload is not based on an extension Files recognized as binaries are uploaded to the lookaside cache. Its size or file extension doesn't matter. This functionality involves "import" command. "new-sources" command still allows any file to be uploaded manually. JIRA: COMPOSE-4155 Fixes: #484 Signed-off-by: Ondrej Nosek --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index ac08653..d7297f1 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -46,7 +46,9 @@ from pyrpkg.errors import (HashtypeMixingError, UnknownTargetError, rpkgAuthError, rpkgError) from pyrpkg.lookaside import CGILookasideCache from pyrpkg.sources import SourcesFile -from pyrpkg.utils import cached_property, find_me, is_file_tracked, log_result +from pyrpkg.utils import (cached_property, extract_srpm, find_me, + is_file_tracked, is_lookaside_eligible_file, + log_result) from .gitignore import GitIgnore @@ -85,12 +87,6 @@ class Commands(object): by clients """ - # This shouldn't change... often - UPLOADEXTS = ['tar', 'gz', 'bz2', 'lzma', 'xz', 'Z', 'zip', 'tff', - 'bin', 'tbz', 'tbz2', 'tgz', 'tlz', 'txz', 'pdf', 'rpm', - 'jar', 'war', 'db', 'cpio', 'jisp', 'egg', 'gem', 'spkg', - 'oxt', 'xpi', 'crate'] - def __init__(self, path, lookaside, lookasidehash, lookaside_cgi, gitbaseurl, anongiturl, branchre, kojiconfig, build_client, @@ -1362,12 +1358,28 @@ class Commands(object): files = [] uploadfiles = [] - # Cycle through the stuff and sort correctly by its extension - for file in contents: - if file.rsplit('.')[-1] in self.UPLOADEXTS: - uploadfiles.append(file) - else: - files.append(file) + # prepare temp directory to extract srpm there + target_dir = tempfile.mkdtemp(suffix="extract-srpm", prefix="rpkg") + try: + try: + self.log.debug("Extracting srpm '{}', destination '{}'".format( + srpm, target_dir + )) + # method 'is_lookaside_eligible_file' will access extracted + # files to detect its encoding (binary or not) + _, _ = extract_srpm(srpm, target_dir) + except Exception as e: + self.log.error("Extraction of srpm has failed {}".format(e)) + raise + + # Cycle through the srpm content and decide where to upload files + for file in contents: + if is_lookaside_eligible_file(file, target_dir): + uploadfiles.append(file) + else: + files.append(file) + finally: + shutil.rmtree(target_dir) return (name, files, uploadfiles) @@ -1866,15 +1878,12 @@ class Commands(object): self.repo.index.remove([file]) os.remove(file) - # Extract new files - cmd = ['rpm2cpio', srpm] - # We have to force cpio to copy out (u) because git messes with - # timestamps - cmd2 = ['cpio', '-iud', '--quiet'] - - rpmcall = subprocess.Popen(cmd, stdout=subprocess.PIPE) - cpiocall = subprocess.Popen(cmd2, stdin=rpmcall.stdout) - output, err = cpiocall.communicate() + try: + self.log.debug("Extracting srpm '{}'".format(srpm)) + output, err = extract_srpm(srpm) + except Exception as e: + self.log.error("Extraction of srpm has failed {}".format(e)) + raise if output: self.log.debug(output) if err: diff --git a/pyrpkg/utils.py b/pyrpkg/utils.py index ada1328..e6339db 100644 --- a/pyrpkg/utils.py +++ b/pyrpkg/utils.py @@ -16,6 +16,7 @@ from __future__ import print_function import argparse import os +import subprocess import sys import git @@ -239,3 +240,51 @@ def is_file_in_directory(file_path, dir_path): # (length of filename is safely longer than length of directory) return real_file_path[len(real_dir_path):].strip("/") return + + +def extract_srpm(srpm_path, target_dir=None): + """ + Extract srpm file into target directory. Target directory is a current + directory if not specified + """ + if not os.path.isfile(srpm_path): + raise IOError("Input file doesn't exist: {}".format(srpm_path)) + if target_dir and not os.path.isdir(target_dir): + raise IOError("Target directory doesn't exist: {}".format(target_dir)) + + # rpm2cpio | cpio -iud --quiet + cmd = ['rpm2cpio', srpm_path] + # We have to force cpio to copy out (u) because git messes with timestamps + cmd2 = ['cpio', '-iud', '--quiet'] + rpmcall = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True) + cpiocall = subprocess.Popen(cmd2, stdin=rpmcall.stdout, universal_newlines=True, cwd=target_dir) + output, err = cpiocall.communicate() + return output, err + + +def is_lookaside_eligible_file(file_name, dir_path=None): + """ + Binary files are eligible to be uploaded to the lookaside cache. + File size and file extension doesn't matter. + """ + file_path = os.path.join(dir_path or "", file_name) + if not os.path.isfile(file_path): + raise IOError("Input file doesn't exist: {}".format(file_path)) + + p = subprocess.Popen( + # parameter '-b' causes brief output - without filename in the output + ['file', '-b', '--mime-encoding', file_name], + cwd=dir_path, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + ) + output, errors = p.communicate() + if errors: + raise RuntimeError("Mime encoding detection of the file '{}' has failed: {}".format( + file_path, + errors + )) + # output contains encoding ("binary", "us-ascii", ...) + encoding = output.strip() # strip newline at the end + return encoding == "binary" diff --git a/tests/fixtures/docpkg/docpkg.spec b/tests/fixtures/docpkg/docpkg.spec index e317fc7..cca2c78 100644 --- a/tests/fixtures/docpkg/docpkg.spec +++ b/tests/fixtures/docpkg/docpkg.spec @@ -9,6 +9,7 @@ Group: Applications/Productivity BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) Source0: hello-world.txt Source1: docpkg.tar.gz +Source2: source-without-extension %description This is a dummy description. diff --git a/tests/fixtures/docpkg/source-without-extension b/tests/fixtures/docpkg/source-without-extension new file mode 100644 index 0000000..6f87ec0 Binary files /dev/null and b/tests/fixtures/docpkg/source-without-extension differ diff --git a/tests/test_cli.py b/tests/test_cli.py index 425ddca..588a624 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1459,6 +1459,7 @@ class TestImportSrpm(LookasideCacheMock, CliTestCase): cli.import_srpm() docpkg_gz = 'docpkg.tar.gz' + source_without_extension = 'source-without-extension' diff_cached = cli.cmd.repo.git.diff('--cached') self.assertTrue('+- - New release 0.2-1' in diff_cached) self.assertTrue('+hello world' in diff_cached) @@ -1467,15 +1468,25 @@ class TestImportSrpm(LookasideCacheMock, CliTestCase): 'docpkg.spec', 'hello-world.txt', docpkg_gz, + source_without_extension, 'README.md'], search_dir=target_repo) self.assertFilesNotExist(['CHANGELOG.rst'], search_dir=target_repo) with open(os.path.join(target_repo, 'sources'), 'r') as f: self.assertEqual( - '{0} {1}'.format(self.hash_file(os.path.join(target_repo, docpkg_gz)), docpkg_gz), - f.read().strip()) + '{0} {1}\n{2} {3}'.format( + self.hash_file(os.path.join(target_repo, docpkg_gz)), + docpkg_gz, + self.hash_file(os.path.join(target_repo, source_without_extension)), + source_without_extension, + ), + f.read().strip() + ) with open(os.path.join(target_repo, '.gitignore'), 'r') as f: - self.assertEqual('/{0}'.format(docpkg_gz), f.read().strip()) - self.assertFilesUploaded([docpkg_gz]) + self.assertEqual( + '/{0}\n/{1}'.format(docpkg_gz, source_without_extension), + f.read().strip() + ) + self.assertFilesUploaded([docpkg_gz, source_without_extension]) def test_import(self): self.assert_import_srpm(self.chaos_repo)