From fe638de275182cf9302eb42aecd327e01854969b Mon Sep 17 00:00:00 2001
From: Ondrej Nosek <onosek@redhat.com>
Date: Apr 30 2020 00:59:07 +0000
Subject: [PATCH 1/2] Passing additional arguments to underlaying commands


There are two groups of arguments:
 * standard arguments defined by individual parsers
 * extra arguments that relevant for some (individually allowed)
   commands. They are passed to underlying command (usually appended
   to the rest of arguments on commandline)
This commit adds a common way that allow user to use extra arguments
for x-pkg commands. These include mockbuild, clone, srpm, local,
prep, compile, install, verify-files.
Extra arguments should be placed at the end of the command
and separated by '--'. Example:
    rhpkg --release eng-rhel-7 srpm --arch amd64 -- --define "'python2 0'"

JIRA: RHELCMP-101
JIRA: RHELCMP-141
Fixes: #432
Fixes: #413

Signed-off-by: Ondrej Nosek <onosek@redhat.com>

---

diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py
index 791b66b..7da3de7 100644
--- a/pyrpkg/__init__.py
+++ b/pyrpkg/__init__.py
@@ -1465,7 +1465,7 @@ class Commands(object):
         return
 
     def clone(self, repo, path=None, branch=None, bare_dir=None,
-              anon=False, target=None, depth=None):
+              anon=False, target=None, depth=None, extra_args=None):
         """Clone a repo, optionally check out a specific branch.
 
         :param str repo: the name of the repository to clone.
@@ -1480,6 +1480,8 @@ class Commands(object):
             repo.
         :param int depth: create a shallow clone with a history truncated
             to the specified number of commits.
+        :param list extra_args: additional arguments that are passed to
+            the clone command.
         """
 
         if not path:
@@ -1517,7 +1519,10 @@ class Commands(object):
         if not bare_dir:
             # --bare and --origin are incompatible
             cmd.extend(['--origin', self.default_branch_remote])
-
+        if extra_args:
+            cmd.extend(extra_args)
+            self.log.debug("Extra args '{0}' are passed to git clone "
+                           "command".format(extra_args))
         if target:
             self.log.debug('Cloning into: %s', target)
             cmd.append(target)
@@ -1546,7 +1551,8 @@ class Commands(object):
             return repo.split("/")[-1]
         return repo
 
-    def clone_with_dirs(self, repo, anon=False, target=None, depth=None):
+    def clone_with_dirs(self, repo, anon=False, target=None, depth=None,
+                        extra_args=None):
         """Clone a repo old style with subdirs for each branch.
 
         :param str repo: name of the repository to clone.
@@ -1556,6 +1562,8 @@ class Commands(object):
             repo.
         :param int depth: create a shallow clone with a history truncated
             to the specified number of commits.
+        :param list extra_args: additional arguments that are passed to
+            the clone command.
         """
 
         self._push_url = None
@@ -1581,7 +1589,8 @@ class Commands(object):
 
         # Create a bare clone first. This gives us a good list of branches
         try:
-            self.clone(repo, top_path, bare_dir=repo_path, anon=anon, depth=depth)
+            self.clone(repo, top_path, bare_dir=repo_path, anon=anon, depth=depth,
+                       extra_args=extra_args)
         except Exception:
             # Clean out our directory
             shutil.rmtree(top_path)
@@ -2422,7 +2431,8 @@ class Commands(object):
         with open(os.path.join(self.path, 'clog'), 'w') as clog:
             clog.writelines(clog_lines)
 
-    def compile(self, arch=None, short=False, builddir=None, nocheck=False, define=None):
+    def compile(self, arch=None, short=False, builddir=None, nocheck=False,
+                define=None, extra_args=None):
         """Run rpmbuild -bc
 
         optionally for a specific arch, or short-circuit it,
@@ -2442,6 +2452,10 @@ class Commands(object):
         if define:
             for entry in define:
                 cmd.extend(['--define', entry])
+        if extra_args:
+            cmd.extend(extra_args)
+            self.log.debug("Extra args '{0}' are passed to rpmbuild "
+                           "command".format(extra_args))
         if short:
             cmd.append('--short-circuit')
         if nocheck:
@@ -2481,8 +2495,8 @@ class Commands(object):
         # This should have a try and catch koji errors
         self.kojisession.uploadWrapper(file, path, name=name, callback=callback)
 
-    def install(self, arch=None, short=False, builddir=None,
-                nocheck=False, buildrootdir=None, define=None):
+    def install(self, arch=None, short=False, builddir=None, nocheck=False,
+                buildrootdir=None, define=None, extra_args=None):
         """Run ``rpmbuild -bi``
 
         optionally for a specific arch, short-circuit it,
@@ -2496,6 +2510,8 @@ class Commands(object):
         :param str builddir: alternate builddir.
         :param bool nocheck: do not check.
         :param str buildrootdir: alternate buildrootdir.
+        :param list extra_args: additional arguments that are passed to
+            the rpmbuild command.
 
         .. versionadded: 1.56
            Parameter buildrootdir.
@@ -2512,6 +2528,10 @@ class Commands(object):
         if define:
             for entry in define:
                 cmd.extend(['--define', entry])
+        if extra_args:
+            cmd.extend(extra_args)
+            self.log.debug("Extra args '{0}' are passed to rpmbuild "
+                           "command".format(extra_args))
         if short:
             cmd.append('--short-circuit')
         if nocheck:
@@ -2573,7 +2593,7 @@ class Commands(object):
         return self.kojisession.listSideTags(basetag=base_tag, user=user)
 
     def local(self, localargs, arch=None, hashtype=None, builddir=None,
-              buildrootdir=None, define=None):
+              buildrootdir=None, define=None, extra_args=None):
         """rpmbuild locally for given arch.
 
         Takes localargs (passed to rpmbuild), arch to build for, and hashtype
@@ -2589,6 +2609,8 @@ class Commands(object):
             digests.
         :param str builddir: an alternative builddir.
         :param str buildrootdir: an alternative buildrootdir.
+        :param list extra_args: additional arguments that are passed to
+            the rpmbuild command.
         :raises rpkgError: if underlying `rpmbuild` fails.
 
         .. versionadded: 1.56
@@ -2603,26 +2625,30 @@ class Commands(object):
         if builddir:
             # Tack on a new builddir to the end of the defines
             cmd.append("--define '_builddir %s'" % os.path.abspath(builddir))
-        # Figure out the hash type to use
-        if not hashtype:
-            # Try to determine the dist
-            hashtype = self._guess_hashtype()
-        # This may need to get updated if we ever change our checksum default
-        if not hashtype == 'sha256':
-            cmd.extend(["--define '_source_filedigest_algorithm %s'"
-                        % hashtype,
-                        "--define '_binary_filedigest_algorithm %s'"
-                        % hashtype])
         if arch:
             cmd.extend(['--target', arch])
         if define:
             for entry in define:
                 cmd.extend(['--define', entry])
+        if extra_args:
+            cmd.extend(extra_args)
+            self.log.debug("Extra args '{0}' are passed to rpmbuild "
+                           "command".format(extra_args))
         if self.quiet:
             cmd.append('--quiet')
         if buildrootdir:
             cmd.append("--define '_buildrootdir {0}'".format(
                 os.path.abspath(buildrootdir)))
+        # Figure out the hash type to use
+        if not hashtype:
+            # Try to determine the dist
+            hashtype = self._guess_hashtype()
+        # This may need to get updated if we ever change our checksum default
+        if not hashtype == 'sha256':
+            cmd.extend(["--define '_source_filedigest_algorithm %s'"
+                        % hashtype,
+                        "--define '_binary_filedigest_algorithm %s'"
+                        % hashtype])
         cmd.extend(['-ba', os.path.join(self.path, self.spec)])
         logfile = '.build-%s-%s.log' % (self.ver, self.rel)
 
@@ -2884,7 +2910,8 @@ class Commands(object):
 
         self.repo.index.add(['sources', '.gitignore'])
 
-    def prep(self, arch=None, builddir=None, buildrootdir=None, define=None):
+    def prep(self, arch=None, builddir=None, buildrootdir=None, define=None,
+             extra_args=None):
         """Run ``rpmbuild -bp``
 
         :param str arch: optional to run prep section for a specific arch. By
@@ -2892,6 +2919,8 @@ class Commands(object):
         :param list define: an optional list of rpmbuild macros.
         :param str builddir: an alternative builddir.
         :param str buildrootdir: an alternative buildrootdir.
+        :param list extra_args: additional arguments that are passed to
+            the rpmbuild command.
 
         .. versionadded: 1.56
            Parameter buildrootdir.
@@ -2908,6 +2937,10 @@ class Commands(object):
         if define:
             for entry in define:
                 cmd.extend(['--define', entry])
+        if extra_args:
+            cmd.extend(extra_args)
+            self.log.debug("Extra args '{0}' are passed to rpmbuild "
+                           "command".format(extra_args))
         if self.quiet:
             cmd.append('--quiet')
         if buildrootdir:
@@ -2917,7 +2950,8 @@ class Commands(object):
         # Run the command
         self._run_command(cmd, shell=True)
 
-    def srpm(self, hashtype=None, define=None, builddir=None, buildrootdir=None, arch=None):
+    def srpm(self, hashtype=None, define=None, builddir=None, buildrootdir=None,
+             arch=None, extra_args=None):
         """Create an srpm using hashtype from content
 
         Requires sources already downloaded. The generated SRPM file will be
@@ -2930,6 +2964,8 @@ class Commands(object):
         :param str buildrootdir: optionally define an alternate buildrootdir.
         :param str arch: optional to run prep section for a specific arch. By
             default, local system arch will be used.
+        :param list extra_args: additional arguments that are passed to
+            the rpmbuild command.
         """
 
         self.srpmname = os.path.join(self.path,
@@ -2942,19 +2978,23 @@ class Commands(object):
 
         cmd = ['rpmbuild']
         cmd.extend(self.rpmdefines)
-        if self.quiet:
-            cmd.append('--quiet')
-        if define:
-            for entry in define:
-                cmd.extend(['--define', entry])
         if builddir:
             # Tack on a new builddir to the end of the defines
             cmd.append("--define '_builddir %s'" % os.path.abspath(builddir))
+        if arch:
+            cmd.extend(['--target', arch])
+        if define:
+            for entry in define:
+                cmd.extend(['--define', entry])
+        if extra_args:
+            cmd.extend(extra_args)
+            self.log.debug("Extra args '{0}' are passed to rpmbuild "
+                           "command".format(extra_args))
+        if self.quiet:
+            cmd.append('--quiet')
         if buildrootdir:
             cmd.append("--define '_buildrootdir {0}'".format(
                 os.path.abspath(buildrootdir)))
-        if arch:
-            cmd.extend(['--target', arch])
 
         # Figure out which hashtype to use, if not provided one
         if not hashtype:
@@ -3025,12 +3065,15 @@ class Commands(object):
                 line_num += 1
         return [line_num, offset - offset_inc + 1]
 
-    def verify_files(self, builddir=None, buildrootdir=None, define=None):
+    def verify_files(self, builddir=None, buildrootdir=None, define=None,
+                     extra_args=None):
         """Run `rpmbuild -bl` to verify the `%files` section
 
         :param list define: an optional list of rpmbuild macros.
         :param str builddir: optionally define an alternate builddir.
         :param str buildrootdir: optionally define an alternate buildrootdir.
+        :param list extra_args: additional arguments that are passed to
+            the rpmbuild command.
 
         .. versionadded:: 1.56
            Parameter buildrootdir.
@@ -3039,17 +3082,21 @@ class Commands(object):
         # setup the rpm command
         cmd = ['rpmbuild']
         cmd.extend(self.rpmdefines)
-        if define:
-            for entry in define:
-                cmd.extend(['--define', entry])
         if builddir:
             # Tack on a new builddir to the end of the defines
             cmd.append("--define '_builddir %s'" % os.path.abspath(builddir))
+        if define:
+            for entry in define:
+                cmd.extend(['--define', entry])
+        if extra_args:
+            cmd.extend(extra_args)
+            self.log.debug("Extra args '{0}' are passed to rpmbuild "
+                           "command".format(extra_args))
+        if self.quiet:
+            cmd.append('--quiet')
         if buildrootdir:
             cmd.append("--define '_buildrootdir {0}'".format(
                 os.path.abspath(buildrootdir)))
-        if self.quiet:
-            cmd.append('--quiet')
         cmd.extend(['-bl', os.path.join(self.path, self.spec)])
         # Run the command
         self._run_command(cmd, shell=True)
diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py
index d13f815..61e2ae0 100644
--- a/pyrpkg/cli.py
+++ b/pyrpkg/cli.py
@@ -320,7 +320,7 @@ class cliClient(object):
             if re.match(r"^clone_config_\w+$", key):
                 clone_config = items.get(key)
                 if not clone_config:
-                    self.log.debug("No clone config is set for '{}'".format(key))
+                    self.log.debug("No clone config is set for '{0}'".format(key))
                 setattr(self._cmd, key, clone_config)
 
     # This function loads the extra stuff once we figure out what site
@@ -540,6 +540,10 @@ class cliClient(object):
         self.rpm_parser_common.add_argument(
             '--define', help='Pass custom macros to rpmbuild, may specify multiple times',
             action='append')
+        self.rpm_parser_common.add_argument(
+            "extra_args", default=None, nargs=argparse.REMAINDER,
+            help="Custom arguments that are passed to the 'rpmbuild'. "
+                 "Use '--' to separate them from other arguments.")
 
     def register_build(self):
         """Register the build target"""
@@ -634,6 +638,7 @@ class cliClient(object):
                         'the configured repository base URL. By default it '
                         'will also checkout the master branch for your '
                         'working copy.')
+
         # Allow an old style clone with subdirs for branches
         clone_parser.add_argument(
             '--branches', '-B', action='store_true',
@@ -652,6 +657,10 @@ class cliClient(object):
         clone_parser.add_argument(
             "clone_target", default=None, nargs="?",
             help='Directory in which to clone the repository')
+        clone_parser.add_argument(
+            "extra_args", default=None, nargs=argparse.REMAINDER,
+            help="Custom arguments that are passed to the 'git clone'. "
+                 "Use '--' to separate them from other arguments.")
 
         def validator_integer_string(raw_value):
             """checks if input is string that contains integer number"""
@@ -1111,6 +1120,10 @@ class cliClient(object):
             help='Pass disablerepo option to yum/dnf (may be used more than once)')
         mockbuild_parser.add_argument(
             '--enable-network', action='store_true', help='Enable networking')
+        mockbuild_parser.add_argument(
+            "extra_args", default=None, nargs=argparse.REMAINDER,
+            help="Custom arguments that are passed to the 'mock'. "
+                 "Use '--' to separate them from other arguments.")
         mockbuild_parser.set_defaults(command=self.mockbuild)
 
     def register_mock_config(self):
@@ -1937,13 +1950,15 @@ class cliClient(object):
             self.cmd.clone_with_dirs(self.args.repo[0],
                                      anon=self.args.anonymous,
                                      target=self.args.clone_target,
-                                     depth=self.args.depth)
+                                     depth=self.args.depth,
+                                     extra_args=self.extra_args)
         else:
             self.cmd.clone(self.args.repo[0],
                            branch=self.args.branch,
                            anon=self.args.anonymous,
                            target=self.args.clone_target,
-                           depth=self.args.depth)
+                           depth=self.args.depth,
+                           extra_args=self.extra_args)
 
     def commit(self):
         if self.args.with_changelog and not self.args.message:
@@ -1993,21 +2008,12 @@ class cliClient(object):
 
     def compile(self):
         self.sources()
-
-        arch = None
-        define = None
-        short = False
-        nocheck = False
-        if self.args.arch:
-            arch = self.args.arch
-        if self.args.define:
-            define = self.args.define
-        if self.args.short_circuit:
-            short = True
-        if self.args.nocheck:
-            nocheck = True
-        self.cmd.compile(arch=arch, define=define, short=short,
-                         builddir=self.args.builddir, nocheck=nocheck)
+        self.cmd.compile(builddir=self.args.builddir,
+                         arch=self.args.arch,
+                         define=self.args.define,
+                         extra_args=self.extra_args,
+                         short=self.args.short_circuit,
+                         nocheck=self.args.nocheck,)
 
     def container_build_koji(self):
         # Keep it around for backward compatibility
@@ -2145,11 +2151,13 @@ class cliClient(object):
 
     def install(self):
         self.sources()
-        self.cmd.install(arch=self.args.arch, define=self.args.define,
+        self.cmd.install(builddir=self.args.builddir,
+                         arch=self.args.arch,
+                         define=self.args.define,
+                         extra_args=self.extra_args,
                          short=self.args.short_circuit,
-                         builddir=self.args.builddir,
                          nocheck=self.args.nocheck,
-                         buildrootdir=self.args.buildrootdir)
+                         buildrootdir=self.args.buildrootdir,)
 
     def lint(self):
         self.cmd.lint(self.args.info, self.args.rpmlintconf)
@@ -2174,11 +2182,12 @@ class cliClient(object):
                 localargs.extend(['--without', arg])
 
         self.cmd.local(localargs,
+                       builddir=self.args.builddir,
                        arch=self.args.arch,
                        define=self.args.define,
-                       hashtype=self.args.hash,
-                       builddir=self.args.builddir,
-                       buildrootdir=self.args.buildrootdir)
+                       extra_args=self.extra_args,
+                       buildrootdir=self.args.buildrootdir,
+                       hashtype=self.args.hash,)
 
     def mockbuild(self):
         try:
@@ -2213,6 +2222,11 @@ class cliClient(object):
         if self.args.enable_network:
             mockargs.append('--enable-network')
 
+        if self.extra_args:
+            mockargs.extend(self.extra_args)
+            self.log.debug("Extra args '{0}' are passed to mock "
+                           "command".format(self.extra_args))
+
         # Pick up any mockargs from the env
         try:
             mockargs += os.environ['MOCKARGS'].split()
@@ -2535,9 +2549,11 @@ class cliClient(object):
 
     def prep(self):
         self.sources()
-        self.cmd.prep(arch=self.args.arch, define=self.args.define,
-                      builddir=self.args.builddir,
-                      buildrootdir=self.args.buildrootdir)
+        self.cmd.prep(builddir=self.args.builddir,
+                      arch=self.args.arch,
+                      define=self.args.define,
+                      extra_args=self.extra_args,
+                      buildrootdir=self.args.buildrootdir,)
 
     def pull(self):
         self.cmd.pull(rebase=self.args.rebase,
@@ -2605,11 +2621,11 @@ class cliClient(object):
         # are for all arches and not set via argparse.
         self.cmd.srpm(
             builddir=getattr(self.args, 'builddir', None),
-            define=getattr(self.args, 'define', None),
-            buildrootdir=getattr(self.args, 'buildroot', None),
             arch=getattr(self.args, 'arch', None),
-            hashtype=self.args.hash
-        )
+            define=getattr(self.args, 'define', None),
+            extra_args=self.extra_args,
+            buildrootdir=getattr(self.args, 'buildrootdir', None),
+            hashtype=self.args.hash,)
 
     def switch_branch(self):
         if self.args.branch:
@@ -2647,8 +2663,9 @@ class cliClient(object):
         print('\n'.join(unused))
 
     def verify_files(self):
-        self.cmd.verify_files(define=self.args.define,
-                              builddir=self.args.builddir,
+        self.cmd.verify_files(builddir=self.args.builddir,
+                              define=self.args.define,
+                              extra_args=self.extra_args,
                               buildrootdir=self.args.buildrootdir)
 
     def verrel(self):
@@ -2702,7 +2719,24 @@ class cliClient(object):
         if argcomplete is not None:
             argcomplete.autocomplete(self.parser)
         # Parse the args
-        self.args = self.parser.parse_args()
+        # There are two groups of arguments:
+        # * standard arguments defined by individual parsers
+        # * extra arguments that are relevant for some (individually allowed)
+        #   commands. They are passed to underlying command (usually appended
+        #   to the rest of arguments on commandline)
+
+        argv = sys.argv[1:]
+        if "--" in argv:
+            i = argv.index("--")
+            self.args = self.parser.parse_args(argv[:i])
+            self.extra_args = argv[i+1:]
+        else:
+            self.args = self.parser.parse_args()
+            self.extra_args = getattr(self.args, 'extra_args', None)
+            if self.extra_args:
+                print("Warning: extra_args in the command are not separated "
+                      "with '--'. It might not work correctly.\n",
+                      file=sys.stderr)
 
         # This is due to a difference argparse behavior to Python 2 version.
         # In Python 3, argparse will proceed to here without reporting
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 588a624..1b8af92 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -813,8 +813,9 @@ class TestVerifyFiles(CliTestCase):
         spec = os.path.join(cli.cmd.path, cli.cmd.spec)
         rpmbuild = ['rpmbuild'] + cli.cmd.rpmdefines + [
             "--define '_builddir %s'" % builddir,
+            '--quiet',
             "--define '_buildrootdir %s'" % buildrootdir,
-            '--quiet', '-bl', spec
+            '-bl', spec
         ]
         _run_command.assert_called_once_with(rpmbuild, shell=True)
 

From bc70dc209d2e9ecfa7269c5634fbd0aff88c949d Mon Sep 17 00:00:00 2001
From: Ondrej Nosek <onosek@redhat.com>
Date: Apr 30 2020 01:16:57 +0000
Subject: [PATCH 2/2] Unittests for passing additional arguments


Signed-off-by: Ondrej Nosek <onosek@redhat.com>

---

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 1b8af92..20749ca 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -577,6 +577,52 @@ class TestPull(CliTestCase):
         self.assertEqual('', cli.cmd.repo.git.log('--merges'))
 
 
+class TestClone(CliTestCase):
+    """Test clone command"""
+    def shortDescription(self):
+        return None
+
+    @patch('sys.stderr', new=StringIO())
+    @patch('pyrpkg.Commands._clone_config', new_callable=Mock())
+    @patch('pyrpkg.Commands._run_command')
+    def test_extra_args(self, _run_command, _clone_config):
+        cli_cmd = ['rpkg', '--user', 'dude', '--path', self.cloned_repo_path,
+                   'clone', 'repository_name', '--', '--progress']
+
+        with patch('sys.argv', new=cli_cmd):
+            cli = self.new_cli()
+            cli.clone()
+
+        expected_cmd = ['git', 'clone', 'ssh://dude@localhost/repository_name',
+                        '--origin', 'origin', '--progress']
+        _run_command.assert_called_once_with(expected_cmd,
+                                             cwd=self.cloned_repo_path)
+        output = sys.stderr.getvalue().strip()
+        self.assertEqual('', output)
+
+    @patch('sys.stderr', new=StringIO())
+    @patch('pyrpkg.Commands._clone_config', new_callable=Mock())
+    @patch('pyrpkg.Commands._run_command')
+    def test_extra_args_incorrect(self, _run_command, _clone_config):
+        """Missing '--' separator between standard arguments and extra
+        arguments. This can be valid command but might have different
+        and unexpected behaviour. Warning is shown."""
+
+        cli_cmd = ['rpkg', '--user', 'dude', '--path', self.cloned_repo_path,
+                   'clone', 'repository_name', '--progress']
+
+        with patch('sys.argv', new=cli_cmd):
+            cli = self.new_cli()
+            cli.clone()
+
+        expected_cmd = ['git', 'clone', 'ssh://dude@localhost/repository_name',
+                        '--origin', 'origin', '--progress']
+        _run_command.assert_called_once_with(expected_cmd,
+                                             cwd=self.cloned_repo_path)
+        output = sys.stderr.getvalue().strip()
+        self.assertIn('extra_args in the command are not separated', output)
+
+
 class TestSrpm(CliTestCase):
     """Test srpm command"""
 
@@ -595,14 +641,28 @@ class TestSrpm(CliTestCase):
     @patch('pyrpkg.Commands._run_command')
     def test_srpm_with_options(self, _run_command):
         cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6', 'srpm',
-                   '--define', 'macro1 meansthis']
+                   '--define', '"macro1 meansthis"']
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
             cli.srpm()
 
         expected_cmd = ['rpmbuild'] + cli.cmd.rpmdefines + \
-            ['--define', 'macro1 meansthis', '--nodeps', '-bs',
+            ['--define', '"macro1 meansthis"', '--nodeps', '-bs',
+             os.path.join(cli.cmd.path, cli.cmd.spec)]
+        _run_command.assert_called_once_with(expected_cmd, shell=True)
+
+    @patch('pyrpkg.Commands._run_command')
+    def test_srpm_with_extra_args(self, _run_command):
+        cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6', 'srpm',
+                   '--', '--define', '"name body"', '--undefine', 'python']
+
+        with patch('sys.argv', new=cli_cmd):
+            cli = self.new_cli()
+            cli.srpm()
+
+        expected_cmd = ['rpmbuild'] + cli.cmd.rpmdefines + \
+            ['--define', '"name body"', '--undefine', 'python', '--nodeps', '-bs',
              os.path.join(cli.cmd.path, cli.cmd.spec)]
         _run_command.assert_called_once_with(expected_cmd, shell=True)
 
@@ -630,7 +690,7 @@ class TestCompile(CliTestCase):
 
         cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6', '-q', 'compile',
                    '--builddir', builddir, '--short-circuit', '--arch', 'i686', '--nocheck',
-                   '--define', 'macro1 meansthis']
+                   '--define', 'macro1 meansthis', '--', '--rmspec']
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
@@ -639,7 +699,7 @@ class TestCompile(CliTestCase):
         spec = os.path.join(cli.cmd.path, cli.cmd.spec)
         rpmbuild = ['rpmbuild'] + cli.cmd.rpmdefines + \
             ["--define '_builddir %s'" % builddir, '--target', 'i686',
-             '--define', 'macro1 meansthis', '--short-circuit',
+             '--define', 'macro1 meansthis', '--rmspec', '--short-circuit',
              '--nocheck', '--quiet', '-bc', spec]
 
         _run_command.assert_called_once_with(rpmbuild, shell=True)
@@ -669,7 +729,7 @@ class TestPrep(CliTestCase):
         cli_cmd = [
             'rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6',
             '-q', 'compile', '--arch', 'i686', '--builddir', builddir,
-            '--buildrootdir', buildrootdir
+            '--buildrootdir', buildrootdir, '--', '-v'
         ]
 
         with patch('sys.argv', new=cli_cmd):
@@ -678,7 +738,7 @@ class TestPrep(CliTestCase):
 
         spec = os.path.join(cli.cmd.path, cli.cmd.spec)
         rpmbuild = ['rpmbuild'] + cli.cmd.rpmdefines + [
-            "--define '_builddir %s'" % builddir, '--target', 'i686',
+            "--define '_builddir %s'" % builddir, '--target', 'i686', '-v',
             '--quiet', "--define '_buildrootdir %s'" % buildrootdir,
             '--nodeps', '-bp', spec
         ]
@@ -756,7 +816,8 @@ class TestLocal(CliTestCase):
         cli_cmd = [
             'rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6',
             '-q', 'local', '--builddir', builddir, '--arch', 'i686',
-            '--with', 'a', '--without', 'b', '--buildrootdir', buildrootdir]
+            '--with', 'a', '--without', 'b', '--buildrootdir', buildrootdir,
+            '--', '--clean']
 
         with patch('sys.argv', new=cli_cmd):
             cli = self.new_cli()
@@ -766,7 +827,7 @@ class TestLocal(CliTestCase):
         rpmbuild = ['rpmbuild'] + cli.cmd.rpmdefines + [
             '--with', 'a', '--without', 'b',
             "--define '_builddir %s'" % builddir,
-            '--target', 'i686', '--quiet',
+            '--target', 'i686', '--clean', '--quiet',
             "--define '_buildrootdir %s'" % buildrootdir,
             '-ba', spec
         ]
@@ -1622,6 +1683,33 @@ class TestMockbuild(CliTestCase):
         ]
         self.mock_run_command.assert_called_with(expected_cmd)
 
+    def test_extra_args(self):
+        cli_cmd = ['rpkg', '--path', self.cloned_repo_path,
+                   '--release', 'rhel-6', 'mockbuild',
+                   '--root', '/etc/mock/some-root',
+                   '--with', 'a',
+                   '--', '--with', 'd', '--without', '"bb"']
+        cli = self.mockbuild(cli_cmd)
+
+        expected_cmd = ['mock', '--with', 'a', '--with', 'd',
+                        '--without', '"bb"', '-r', '/etc/mock/some-root',
+                        '--resultdir', cli.cmd.mock_results_dir, '--rebuild',
+                        cli.cmd.srpmname]
+        self.mock_run_command.assert_called_with(expected_cmd)
+
+    def test_extra_args_new_arguments(self):
+        cli_cmd = ['rpkg', '--path', self.cloned_repo_path,
+                   '--release', 'rhel-6', 'mockbuild',
+                   '--root', '/etc/mock/some-root',
+                   '--', '--nocheck', '--arch=amd64', '-q']
+        cli = self.mockbuild(cli_cmd)
+
+        expected_cmd = ['mock', '--nocheck', '--arch=amd64', '-q',
+                        '-r', '/etc/mock/some-root',
+                        '--resultdir', cli.cmd.mock_results_dir, '--rebuild',
+                        cli.cmd.srpmname]
+        self.mock_run_command.assert_called_with(expected_cmd)
+
 
 class TestCoprBuild(CliTestCase):
     """Test copr command"""