From 9b557ca6d478b8eef5146203d8ca748ac85e4dde Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jul 13 2016 08:18:07 +0000 Subject: Warning untracked patches when push When push to dist-git, there will be a warning for untracked patches that are not checked into git and tracked by sources both. Also, fix some pep8 errors in pyrpkg.__init__.py Signed-off-by: Chenxiong Qi --- diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py index 60640d2..4d95148 100644 --- a/src/pyrpkg/__init__.py +++ b/src/pyrpkg/__init__.py @@ -28,8 +28,9 @@ import git import tempfile import fnmatch import posixpath -import git import six +import glob +from ConfigParser import ConfigParser from six.moves import configparser from six.moves import urllib # Try to import krb, it's OK if it fails @@ -39,12 +40,11 @@ except ImportError: pass from pyrpkg.errors import HashtypeMixingError, rpkgError, rpkgAuthError, \ - UnknownTargetError + UnknownTargetError from .gitignore import GitIgnore from pyrpkg.lookaside import CGILookasideCache from pyrpkg.sources import SourcesFile from pyrpkg.utils import cached_property, warn_deprecated, log_result -from pyrpkg.utils import u, getcwd from osbs.api import OSBS from osbs.conf import Configuration @@ -79,7 +79,8 @@ class Commands(object): def __init__(self, path, lookaside, lookasidehash, lookaside_cgi, gitbaseurl, anongiturl, branchre, kojiconfig, build_client, user=None, - dist=None, target=None, quiet=False, distgit_namespaced=False): + dist=None, target=None, quiet=False, + distgit_namespaced=False): """Init the object and some configuration details.""" # Path to operate on, most often pwd @@ -230,19 +231,19 @@ class Commands(object): defaults = { 'server': None, 'topurl': 'http://localhost/kojiroot', - 'weburl' : 'http://localhost/koji', + 'weburl': 'http://localhost/koji', 'cert': '~/.koji/client.crt', 'ca': '~/.koji/clientca.crt', 'serverca': '~/.koji/serverca.crt', 'authtype': None, 'krbservice': None, - 'timeout' : None, - 'keepalive' : True, + 'timeout': None, + 'keepalive': True, 'max_retries': None, 'retry_interval': None, - 'anon_retry' : True, - 'offline_retry' : None, - 'offline_retry_interval' : None, + 'anon_retry': True, + 'offline_retry': None, + 'offline_retry_interval': None, 'use_fast_upload': None, 'debug': None, 'debug_xmlrpc': None @@ -261,12 +262,12 @@ class Commands(object): if name in ('keepalive', 'anon_retry', 'offline_retry', 'use_fast_upload', 'debug', 'debug_xmlrpc'): - defaults[name] = config.getboolean(os.path.basename( - self.build_client), name) + defaults[name] = config.getboolean( + os.path.basename(self.build_client), name) elif name in ('timeout', 'max_retries', 'retry_interval', 'offline_retry_interval'): - defaults[name] = config.getint(os.path.basename( - self.build_client), name) + defaults[name] = config.getint( + os.path.basename(self.build_client), name) else: defaults[name] = value if not defaults['server']: @@ -313,8 +314,8 @@ class Commands(object): for (_, _, ssl_reason) in error.message: # Use heuristic. Some OpenSSL libs doesn't store error # codes - if ('certificate revoked' in ssl_reason or - 'certificate expired' in ssl_reason): + if 'certificate revoked' in ssl_reason or \ + 'certificate expired' in ssl_reason: self.log.info("Certificate is revoked or expired.") raise rpkgAuthError('Could not auth with koji. Login ' 'failed: %s' % error) @@ -400,16 +401,15 @@ class Commands(object): def load_push_url(self): """Find the pushurl or url of remote of branch we're on.""" try: - url = self.repo.git.remote('get-url', '--push', - self.branch_remote) + url = self.repo.git.remote('get-url', '--push', self.branch_remote) except git.GitCommandError as e: try: - url = self.repo.git.config('--get', 'remote.%s.pushurl' - % self.branch_remote) + url = self.repo.git.config( + '--get', 'remote.%s.pushurl' % self.branch_remote) except git.GitCommandError as e: try: - url = self.repo.git.config('--get', 'remote.%s.url' - % self.branch_remote) + url = self.repo.git.config( + '--get', 'remote.%s.url' % self.branch_remote) except git.GitCommandError as e: raise rpkgError('Unable to find remote push url: %s' % e) if isinstance(url, six.text_type): @@ -596,7 +596,8 @@ class Commands(object): self._ns_module_name = ns_module_name return except rpkgError: - self.log.warning('Failed to get ns_module_name from Git url or pushurl') + self.log.warning( + 'Failed to get ns_module_name from Git url or pushurl') @property def nvr(self): @@ -1325,7 +1326,8 @@ class Commands(object): self._push_url = None self._branch_remote = None # Get the full path of, and git object for, our directory of branches - top_path = os.path.join(self.path, target or self.get_base_module(module)) + top_path = os.path.join(self.path, + target or self.get_base_module(module)) top_git = git.Git(top_path) repo_path = os.path.join(top_path, 'rpkg.git') @@ -1723,6 +1725,22 @@ class Commands(object): self._run_command(cmd, cwd=self.path) return + def find_untracked_patches(self): + """Find patches that are not tracked by git and sources both""" + file_pattern = os.path.join(self.path, '*.patch') + patches_in_repo = [os.path.basename(filename) for filename + in glob.glob(file_pattern)] + + git_tree = self.repo.head.commit.tree + sources_file = SourcesFile(self.sources_filename, + self.source_entry_type) + + patches_not_tracked = [ + patch for patch in patches_in_repo + if patch not in git_tree and patch not in sources_file] + + return patches_not_tracked + def push(self, force=False): """Push changes to the remote repository""" @@ -1731,29 +1749,18 @@ class Commands(object): self.load_branch_merge() except: self.log.warning('Current branch cannot be pushed anywhere!') - # check missing patches - ts = rpm.TransactionSet() - specfile = ts.parseSpec(os.path.join(self.path, self.spec)) - missing_patches = [] - for source in specfile.sources: - if source[0].endswith('.patch'): - patch = source[0] - hdc = self.repo.head.commit.tree - try: - # check if patch is in the repository - hdc[patch] - except KeyError: - missing_patches.append(patch) - if missing_patches: - if not force: - raise rpkgError("%s contains untracked patches.\n%s\nConsider " - "to add them to repository or use --force " - "option" % (self.spec, missing_patches)) + + untracked_patches = self.find_untracked_patches() + if untracked_patches: + self.log.warning( + 'Patches %s %s not tracked within either git or sources', + ', '.join(untracked_patches), + 'is' if len(untracked_patches) == 1 else 'are') + cmd = ['git', 'push'] if self.quiet: cmd.append('-q') self._run_command(cmd, cwd=self.path) - return def sources(self, outdir=None): """Download source files""" @@ -1821,7 +1828,8 @@ class Commands(object): self.log.info("Switched to branch '%s'" % branch) except Exception as err: # This needs to be finer grained I think... - raise rpkgError('Could not check out %s\n%s' % (branch, err.stderr)) + raise rpkgError('Could not check out %s\n%s' % (branch, + err.stderr)) return def file_exists(self, pkg_name, filename, checksum): @@ -2580,9 +2588,9 @@ class Commands(object): raise RuntimeError("Build has failed.") def container_build_koji(self, target_override=False, opts={}, - kojiconfig=None, build_client=None, - koji_task_watcher=None, - nowait=False): + kojiconfig=None, build_client=None, + koji_task_watcher=None, + nowait=False): # check if repo is dirty and all commits are pushed self.check_repo() docker_target = self.target @@ -2625,15 +2633,17 @@ class Commands(object): task_opts, priority=priority) self.log.info('Created task: %s' % task_id) - self.log.info('Task info: %s/taskinfo?taskID=%s' % (self.kojiweburl, - task_id)) + self.log.info('Task info: %s/taskinfo?taskID=%s', + self.kojiweburl, task_id) if not nowait: rv = koji_task_watcher(self.kojisession, [task_id]) if rv == 0: result = self.kojisession.getTaskResult(task_id) try: - result["koji_builds"] = ["%s/buildinfo?buildID=%s" % (self.kojiweburl, build_id) - for build_id in result.get("koji_builds", [])] + result["koji_builds"] = [ + "%s/buildinfo?buildID=%s" % (self.kojiweburl, + build_id) + for build_id in result.get("koji_builds", [])] except TypeError: pass log_result(self.log.info, result) @@ -2642,7 +2652,8 @@ class Commands(object): (self.build_client, self.kojiconfig) = koji_session_backup self.load_kojisession() - def container_build_setup(self, get_autorebuild=None, set_autorebuild=None): + def container_build_setup(self, get_autorebuild=None, + set_autorebuild=None): cfp = ConfigParser.SafeConfigParser() if os.path.exists(self.osbs_config_filename): cfp.read(self.osbs_config_filename) diff --git a/src/pyrpkg/sources.py b/src/pyrpkg/sources.py index 1486c53..a443f38 100644 --- a/src/pyrpkg/sources.py +++ b/src/pyrpkg/sources.py @@ -44,6 +44,12 @@ class SourcesFile(object): if entry and entry not in self.entries: self.entries.append(entry) + def __contains__(self, filename): + for entry in self.entries: + if entry.file == filename: + return True + return False + def parse_line(self, line): stripped = line.strip() diff --git a/test/commands/test_push.py b/test/commands/test_push.py index 5a0c69c..0c681f7 100644 --- a/test/commands/test_push.py +++ b/test/commands/test_push.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import os +import git from . import CommandTestCase @@ -28,128 +29,20 @@ CLONE_CONFIG = ''' class CommandPushTestCase(CommandTestCase): - def test_push_without_patches(self): - self.make_new_git(self.module) - - import pyrpkg - cmd = pyrpkg.Commands(self.path, self.lookaside, - self.lookasidehash, - self.lookaside_cgi, self.gitbaseurl, - self.anongiturl, self.branchre, self.kojiconfig, - self.build_client, self.user, self.dist, - self.target, self.quiet) - cmd.clone_config = CLONE_CONFIG - cmd.clone(self.module, anon=True) - cmd.path = os.path.join(self.path, self.module) - os.chdir(os.path.join(self.path, self.module)) - - specfile_path = self.module + ".spec" - - # specfile with no patches - with open(specfile_path, 'w') as specfile: - specfile.write(SPECFILE_TEMPLATE % "") - specfile.close() - - try: - cmd.push() - except pyrpkg.rpkgError: - self.fail("No unpushed patches. This shouldn't raise exception") - - def test_push_one_uncommitted_patch(self): - self.make_new_git(self.module) - - import pyrpkg - cmd = pyrpkg.Commands(self.path, self.lookaside, - self.lookasidehash, - self.lookaside_cgi, self.gitbaseurl, - self.anongiturl, self.branchre, self.kojiconfig, - self.build_client, self.user, self.dist, - self.target, self.quiet) - cmd.clone_config = CLONE_CONFIG - cmd.clone(self.module, anon=True) - cmd.path = os.path.join(self.path, self.module) - os.chdir(os.path.join(self.path, self.module)) - - specfile_path = self.module + ".spec" - - # add uncommitted patch - with open(specfile_path, 'w') as specfile: - specfile.write(SPECFILE_TEMPLATE % "Patch: test.patch") - specfile.close() - - with open("test.patch", 'w') as f: - f.close() - - def raises(): - cmd.push() - - self.assertRaises(pyrpkg.rpkgError, raises) - - def test_push_uncommitted_patch_with_force_option(self): - self.make_new_git(self.module) - - import pyrpkg - cmd = pyrpkg.Commands(self.path, self.lookaside, - self.lookasidehash, - self.lookaside_cgi, self.gitbaseurl, - self.anongiturl, self.branchre, self.kojiconfig, - self.build_client, self.user, self.dist, - self.target, self.quiet) - cmd.clone_config = CLONE_CONFIG - cmd.clone(self.module, anon=True) - cmd.path = os.path.join(self.path, self.module) - os.chdir(os.path.join(self.path, self.module)) - - specfile_path = self.module + ".spec" - with open(specfile_path, 'w') as specfile: - specfile.write(SPECFILE_TEMPLATE % "Patch: test.patch") - specfile.close() + def setUp(self): + # Tests within this case would change working directory. Changing back + # to original directory to avoid any potential problems. + self.original_dir = os.path.abspath(os.curdir) + super(CommandPushTestCase, self).setUp() - with open("test.patch", 'w') as f: - f.close() + def tearDown(self): + os.chdir(self.original_dir) + super(CommandPushTestCase, self).tearDown() - # Don't check uncommitted patches - try: - cmd.push(force=True) - except pyrpkg.rpkgError: - self.fail("No unpushed patches. This shouldn't raise exception") - - def test_push_committed_patch(self): - self.make_new_git(self.module) - - import pyrpkg - cmd = pyrpkg.Commands(self.path, self.lookaside, - self.lookasidehash, - self.lookaside_cgi, self.gitbaseurl, - self.anongiturl, self.branchre, self.kojiconfig, - self.build_client, self.user, self.dist, - self.target, self.quiet) - cmd.clone_config = CLONE_CONFIG - cmd.clone(self.module, anon=True) - cmd.path = os.path.join(self.path, self.module) - os.chdir(os.path.join(self.path, self.module)) - - specfile_path = self.module + ".spec" - - # add patch and commit it - patch = "test.patch" - with open(specfile_path, 'w') as specfile: - specfile.write(SPECFILE_TEMPLATE % ("Patch: %s" % patch)) - specfile.close() - - with open("test.patch", 'w') as f: - f.close() - - cmd.repo.index.add([specfile_path, patch]) - cmd.repo.index.commit("add Patch") - - try: - cmd.push() - except pyrpkg.rpkgError: - self.fail("No unpushed patches. This shouldn't raise exception") + def test_push_outside_repo(self): + """push from outside repo with --path option""" - def test_push_part_committed_patches(self): self.make_new_git(self.module) import pyrpkg @@ -164,55 +57,81 @@ class CommandPushTestCase(CommandTestCase): cmd.path = os.path.join(self.path, self.module) os.chdir(os.path.join(self.path, self.module)) - specfile_path = self.module + ".spec" - - # add two patches and commit only one - patch = "test.patch" - patch2 = "test2.patch" - - with open(specfile_path, 'w') as specfile: - specfile.write(SPECFILE_TEMPLATE % ("Patch: %s\nPatch1: %s" % - (patch, patch2))) - specfile.close() + spec_file = 'module.spec' + with open(spec_file, 'w') as f: + f.write(SPECFILE_TEMPLATE % '') - with open(patch, 'w') as f: - f.close() - with open(patch2, 'w') as f: - f.close() + cmd.repo.index.add([spec_file]) + cmd.repo.index.commit("add SPEC") - # add only one patch - cmd.repo.index.add([specfile_path, patch]) - cmd.repo.index.commit("add Patch") + # Now, change directory to parent and test the push + os.chdir(self.path) + cmd.push() - def raises(): - cmd.push() - self.assertRaises(pyrpkg.rpkgError, raises) +class TestPushWithPatches(CommandTestCase): - def test_push_outside_repo(self): - """push from outside repo with --path option""" + def setUp(self): + super(TestPushWithPatches, self).setUp() self.make_new_git(self.module) import pyrpkg - cmd = pyrpkg.Commands(self.path, self.lookaside, - self.lookasidehash, - self.lookaside_cgi, self.gitbaseurl, - self.anongiturl, self.branchre, self.kojiconfig, - self.build_client, self.user, self.dist, - self.target, self.quiet) - cmd.clone_config = CLONE_CONFIG - cmd.clone(self.module, anon=True) - cmd.path = os.path.join(self.path, self.module) + self.cmd = pyrpkg.Commands(self.path, self.lookaside, + self.lookasidehash, + self.lookaside_cgi, self.gitbaseurl, + self.anongiturl, self.branchre, + self.kojiconfig, + self.build_client, self.user, self.dist, + self.target, self.quiet) + self.cmd.clone_config = CLONE_CONFIG + self.cmd.clone(self.module, anon=True) + self.cmd.path = os.path.join(self.path, self.module) os.chdir(os.path.join(self.path, self.module)) + # Track SPEC and a.patch in git spec_file = 'module.spec' with open(spec_file, 'w') as f: - f.write(SPECFILE_TEMPLATE % '') - - cmd.repo.index.add([spec_file]) - cmd.repo.index.commit("add SPEC") - - # Now, change directory to parent and test the push - os.chdir(self.path) - cmd.push() + f.write(SPECFILE_TEMPLATE % '''Patch0: a.patch +Patch1: b.path +Patch2: c.path +Patch3: d.path +''') + + for patch_file in ('a.patch', 'b.patch', 'c.patch', 'd.patch'): + with open(patch_file, 'w') as f: + f.write(patch_file) + + # Track c.patch in sources + from pyrpkg.sources import SourcesFile + sources_file = SourcesFile(self.cmd.sources_filename, + self.cmd.source_entry_type) + file_hash = self.cmd.lookasidecache.hash_file('c.patch') + sources_file.add_entry(self.cmd.lookasidehash, 'c.patch', file_hash) + sources_file.write() + + self.cmd.repo.index.add([spec_file, 'a.patch', 'sources']) + self.cmd.repo.index.commit('add SPEC and patches') + + def test_find_untracked_patches(self): + untracked_patches = self.cmd.find_untracked_patches() + untracked_patches.sort() + self.assertEqual(['b.patch', 'd.patch'], untracked_patches) + + def test_push_not_blocked_by_untracked_patches(self): + self.cmd.push() + + # Verify added files are pushed to origin + origin_repo_path = self.cmd.repo.git.config( + '--get', 'remote.origin.url').replace('file://', '') + origin_repo = git.Repo(origin_repo_path) + git_tree = origin_repo.head.commit.tree + self.assertTrue('a.patch' in git_tree) + self.assertTrue('b.patch' not in git_tree) + self.assertTrue('c.patch' not in git_tree) + self.assertTrue('d.patch' not in git_tree) + + sources_content = origin_repo.git.show('master:sources').strip() + with open('sources', 'r') as f: + expected_sources_content = f.read().strip() + self.assertEqual(expected_sources_content, sources_content)