From 9b557ca6d478b8eef5146203d8ca748ac85e4dde Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
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 <cqi@redhat.com>

---

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)

