From 55caa85a1831cc18aac4634fc7aecb91ba414095 Mon Sep 17 00:00:00 2001 From: Dominik Rumian Date: Aug 04 2021 10:15:30 +0000 Subject: better new-sources output when all sources already exist Fixes: #533 JIRA: RHELCMP-5529 Signed-off-by: Dominik Rumian --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index 6510275..ae82697 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -31,7 +31,9 @@ from itertools import groupby from multiprocessing.dummy import Pool as ThreadPool from operator import itemgetter +import cccolutils import git +import koji import requests import rpm import six @@ -39,11 +41,9 @@ import yaml from six.moves import configparser, urllib from six.moves.urllib.parse import urljoin -import cccolutils -import koji from pyrpkg import layout -from pyrpkg.errors import (HashtypeMixingError, UnknownTargetError, - rpkgAuthError, rpkgError) +from pyrpkg.errors import (AlreadyUploadedError, HashtypeMixingError, + UnknownTargetError, rpkgAuthError, rpkgError) from pyrpkg.lookaside import CGILookasideCache from pyrpkg.sources import SourcesFile from pyrpkg.utils import (cached_property, extract_srpm, find_me, @@ -73,8 +73,9 @@ except ImportError: specfile_uses_rpmautospec = None try: + from rpmautospec import \ + calculate_release_number as rpmautospec_calculate_release_number from rpmautospec import process_distgit as rpmautospec_process_distgit - from rpmautospec import calculate_release_number as rpmautospec_calculate_release_number except ImportError: rpmautospec_process_distgit = None rpmautospec_calculate_release_number = None @@ -2828,6 +2829,7 @@ class Commands(object): sourcesf = SourcesFile(self.sources_filename, self.source_entry_type, replace=replace) gitignore = GitIgnore(os.path.join(self.path, '.gitignore')) + all_files_already_uploaded = True for f in files: # TODO: Skip empty file needed? @@ -2859,15 +2861,23 @@ class Commands(object): "Hint: Use git for text files like the spec file, patches or helper scripts. " "Use the lookaside cache for binary blobs, usually upstream source " "tarballs.".format(f)) - self.lookasidecache.upload( - self.ns_repo_name if self.lookaside_namespaced else self.repo_name, - f, file_hash, offline=offline) + try: + self.lookasidecache.upload( + self.ns_repo_name if self.lookaside_namespaced else self.repo_name, + f, file_hash, offline=offline) + except AlreadyUploadedError: + pass + else: + all_files_already_uploaded = False sourcesf.write() gitignore.write() self.repo.index.add(['sources', '.gitignore']) + if all_files_already_uploaded: + raise AlreadyUploadedError('File already uploaded') + def prep(self, arch=None, builddir=None, buildrootdir=None, define=None, extra_args=None): """Run ``rpmbuild -bp`` diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py index c7599ee..f64a97f 100644 --- a/pyrpkg/cli.py +++ b/pyrpkg/cli.py @@ -23,16 +23,17 @@ import sys import textwrap from gettext import gettext as _ # For `_ArgumentParser' +import koji_cli.lib import requests import rpm import six from requests.auth import HTTPBasicAuth from six.moves import configparser -import koji_cli.lib import pyrpkg.utils as utils -from pyrpkg import Modulemd -from pyrpkg import rpkgError +from pyrpkg import Modulemd, rpkgError + +from .errors import AlreadyUploadedError # argcomplete might not be available for all products which # use rpkg as a library (for example centpkg) @@ -2552,12 +2553,16 @@ class cliClient(object): if not os.path.isfile(file): raise Exception('Path does not exist or is ' 'not a file: %s' % file) - self.cmd.upload( - self.args.files, - replace=self.args.replace, - offline=self.args.offline,) - self.log.info("Source upload succeeded. Don't forget to commit the " - "sources file") + try: + self.cmd.upload( + self.args.files, + replace=self.args.replace, + offline=self.args.offline,) + except AlreadyUploadedError: + self.log.info("All sources were already uploaded.") + else: + self.log.info("Source upload succeeded. Don't forget to commit the " + "sources file") def upload(self): self.new_sources() diff --git a/pyrpkg/errors.py b/pyrpkg/errors.py index 49b71d4..787becf 100644 --- a/pyrpkg/errors.py +++ b/pyrpkg/errors.py @@ -67,3 +67,8 @@ class UploadError(rpkgError): class LayoutError(rpkgError): """Raised when something went wrong while parsing/loading a layout""" pass + + +class AlreadyUploadedError(rpkgError): + """Raised when file is already uploaded""" + pass diff --git a/pyrpkg/lookaside.py b/pyrpkg/lookaside.py index b112957..0ae8540 100644 --- a/pyrpkg/lookaside.py +++ b/pyrpkg/lookaside.py @@ -24,7 +24,8 @@ import pycurl import six from six.moves import http_client -from .errors import DownloadError, InvalidHashType, UploadError +from .errors import (AlreadyUploadedError, DownloadError, InvalidHashType, + UploadError) class CGILookasideCache(object): @@ -295,7 +296,7 @@ class CGILookasideCache(object): if self.remote_file_exists(name, filename, hash): self.log.info("File already uploaded: %s", filepath) - return + raise AlreadyUploadedError('File already uploaded') self.log.info("Uploading: %s", filepath) post_data = [ diff --git a/tests/commands/test_check_repo.py b/tests/commands/test_check_repo.py index b782ea6..fa9fb1c 100644 --- a/tests/commands/test_check_repo.py +++ b/tests/commands/test_check_repo.py @@ -6,9 +6,8 @@ import sys import tempfile from mock import patch -from six.moves import StringIO - from pyrpkg.errors import rpkgError +from six.moves import StringIO from . import CommandTestCase diff --git a/tests/commands/test_clone.py b/tests/commands/test_clone.py index 3739ac5..f741864 100644 --- a/tests/commands/test_clone.py +++ b/tests/commands/test_clone.py @@ -3,7 +3,6 @@ import shutil import tempfile import git - import pyrpkg from . import CommandTestCase diff --git a/tests/test_cli.py b/tests/test_cli.py index 5273e1a..dfc8bd8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import errno +import filecmp import glob import hashlib import logging @@ -12,15 +13,16 @@ import sys import tempfile import git +import koji_cli.lib import six +import utils from mock import ANY, Mock, PropertyMock, call, mock_open, patch from six.moves import StringIO, configparser, http_client +from utils import CommandTestCase, FakeThreadPool -import koji_cli.lib import pyrpkg.cli -import utils from pyrpkg import Commands, Modulemd, layout, rpkgError -from utils import CommandTestCase, FakeThreadPool +from pyrpkg.errors import AlreadyUploadedError try: import unittest2 as unittest @@ -1229,10 +1231,17 @@ class LookasideCacheMock(object): def lookasidecache_upload(self, repo_name, filepath, hash, offline): filename = os.path.basename(filepath) storage_filename = os.path.join(self.lookasidecache_storage, filename) + if self._remote_file_exists(filepath, storage_filename): + raise AlreadyUploadedError("File already uploaded.") with open(storage_filename, 'wb') as fout: with open(filepath, 'rb') as fin: fout.write(fin.read()) + def _remote_file_exists(self, local_file, remote_file): + if os.path.isfile(remote_file): + return filecmp.cmp(local_file, remote_file) + return False + def lookasidecache_download(self, name, filename, hash, outfile, hashtype=None, **kwargs): with open(outfile, 'w') as f: f.write('binary data') @@ -1330,6 +1339,22 @@ class TestUpload(LookasideCacheMock, CliTestCase): r'.+README.rst which has different checksum.+', cli.upload) + @patch('pyrpkg.log.info') + def test_upload_file_twice(self, log_info): + cli_cmd = [ + 'rpkg', '--path', self.cloned_repo_path, 'upload', self.readme_patch + ] + + with patch('sys.argv', new=cli_cmd): + cli = self.new_cli() + with patch('pyrpkg.lookaside.CGILookasideCache.upload', + new=self.lookasidecache_upload): + cli.upload() + cli.upload() + + log_info.assert_has_calls( + call("All sources were already uploaded.")) + class TestSources(LookasideCacheMock, CliTestCase): @@ -1503,7 +1528,11 @@ class TestImportSrpm(LookasideCacheMock, CliTestCase): def test_import(self): self.assert_import_srpm(self.chaos_repo) - self.assert_import_srpm(self.cloned_repo_path) + # Exception is not a functionality issue. There is no problem with + # uploading same file twice in a test however, the upload method checks + # if file has been already uploaded. + self.assertRaisesRegex(AlreadyUploadedError, r'File already uploaded', + self.assert_import_srpm, self.cloned_repo_path) def test_import_gating_and_rpmlintrc_exception(self): # Add three additional files to the repo. Former gating.yaml and package.rpmlintrc are diff --git a/tests/test_commands.py b/tests/test_commands.py index 2575dc6..587127a 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -12,8 +12,8 @@ import git import rpm import six from mock import Mock, PropertyMock, call, mock_open, patch - from pyrpkg import rpkgError + from utils import CommandTestCase diff --git a/tests/test_flatpak_build.py b/tests/test_flatpak_build.py index ac7e9af..5098ab7 100644 --- a/tests/test_flatpak_build.py +++ b/tests/test_flatpak_build.py @@ -4,8 +4,8 @@ from textwrap import dedent import requests from mock import Mock, patch - from pyrpkg import Modulemd + from utils import CommandTestCase try: diff --git a/tests/test_lookaside.py b/tests/test_lookaside.py index 7e22d93..6946b79 100644 --- a/tests/test_lookaside.py +++ b/tests/test_lookaside.py @@ -15,8 +15,8 @@ import unittest import mock import pycurl - -from pyrpkg.errors import DownloadError, InvalidHashType, UploadError +from pyrpkg.errors import (AlreadyUploadedError, DownloadError, + InvalidHashType, UploadError) from pyrpkg.lookaside import CGILookasideCache old_stat = os.stat @@ -510,7 +510,10 @@ class CGILookasideCacheTestCase(unittest.TestCase): hash = 'thehash' with mock.patch.object(lc, 'remote_file_exists', lambda *x: True): - lc.upload('pyrpkg', 'pyrpkg-0.0.tar.xz', hash) + # self.assertRaises(AlreadyUploadedError, lc.upload, 'pyrpkg', + # 'pyrpkg-0.tar.xz', hash) + self.assertRaisesRegex(AlreadyUploadedError, r'File already uploaded', + lc.upload, 'pyrpkg', 'pyrpkg-0.tar.xz', hash) self.assertEqual(curl.perform.call_count, 0) self.assertEqual(curl.setopt.call_count, 0) diff --git a/tests/test_retire.py b/tests/test_retire.py index 5868cfe..c41327c 100644 --- a/tests/test_retire.py +++ b/tests/test_retire.py @@ -6,11 +6,10 @@ import subprocess import tempfile import mock -import six -from six.moves import configparser - import pyrpkg.cli +import six from pyrpkg.errors import rpkgError +from six.moves import configparser # For running tests with Python 2 try: diff --git a/tests/test_side_tag.py b/tests/test_side_tag.py index 2ce7f11..766d9e9 100644 --- a/tests/test_side_tag.py +++ b/tests/test_side_tag.py @@ -3,12 +3,12 @@ import logging import os +import koji import mock +import pyrpkg.cli import six from six.moves import StringIO, configparser -import koji -import pyrpkg.cli from utils import CommandTestCase if six.PY2: diff --git a/tests/test_utils.py b/tests/test_utils.py index 7054b9e..557cf66 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -4,9 +4,9 @@ import unittest import warnings import mock - from pyrpkg.utils import (cached_property, is_file_in_directory, is_file_tracked, log_result, warn_deprecated) + from utils import CommandTestCase diff --git a/tests/utils.py b/tests/utils.py index c686040..f9625fd 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -7,7 +7,6 @@ import sys import tempfile import six - from pyrpkg import Commands # For running tests with Python 2