From d5e895d13e95bfc1c3ca2e5d9c61ff8d368aebb8 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jul 06 2021 15:31:58 +0000 Subject: [PATCH 1/6] Tests: specify branch name on `git init` The tests expect a default git branch called `master`, this ensures that a user's global configuration or changed defaults don't override it (which makes tests fail). Signed-off-by: Nils Philippsen --- diff --git a/tests/commands/__init__.py b/tests/commands/__init__.py index b5cff9e..ae26ee0 100644 --- a/tests/commands/__init__.py +++ b/tests/commands/__init__.py @@ -58,7 +58,7 @@ class CommandTestCase(unittest.TestCase): # Create a bare Git repository moduledir = os.path.join(self.gitroot, module) os.makedirs(moduledir) - subprocess.check_call(['git', 'init', '--bare'], cwd=moduledir, + subprocess.check_call(['git', 'init', '--bare', '-b', 'master'], cwd=moduledir, stdout=subprocess.PIPE, stderr=subprocess.PIPE) # Clone it, and do the minimal Dist Git setup diff --git a/tests/test_cli.py b/tests/test_cli.py index 3f58c57..a40e26c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1444,7 +1444,7 @@ class TestImportSrpm(LookasideCacheMock, CliTestCase): self.chaos_repo = tempfile.mkdtemp(prefix='rpkg-tests-chaos-repo-') cmds = ( - ['git', 'init'], + ['git', 'init', '-b', 'master'], ['touch', 'README.md'], ['git', 'add', 'README.md'], ['git', 'config', 'user.name', 'tester'], diff --git a/tests/test_retire.py b/tests/test_retire.py index a1605c3..02c6976 100644 --- a/tests/test_retire.py +++ b/tests/test_retire.py @@ -31,7 +31,7 @@ class RetireTestCase(unittest.TestCase): def _setup_repo(self, origin): cmds = ( - ['git', 'init'], + ['git', 'init', '-b', 'master'], ['git', 'config', 'user.name', 'John Doe'], ['git', 'config', 'user.email', 'jdoe@example.com'], ['git', 'remote', 'add', 'origin', origin], diff --git a/tests/utils.py b/tests/utils.py index 0875cf3..088f79a 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -157,7 +157,7 @@ class RepoCreationMixin(object): f.write(spec_file) git_cmds = [ - ['git', 'init'], + ['git', 'init', '-b', 'master'], ['touch', 'sources', 'CHANGELOG.rst'], ['git', 'add', spec_file_path, 'sources', 'CHANGELOG.rst'], ['git', 'config', 'user.email', 'tester@example.com'], From 3826ee826c06e849d99c2a3a54ba9ec1f29a0de5 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jul 06 2021 15:31:58 +0000 Subject: [PATCH 2/6] Fix remaining Python3 SafeConfigParser warnings Signed-off-by: Nils Philippsen --- diff --git a/bin/rpkg b/bin/rpkg index 363a011..26cf0d2 100755 --- a/bin/rpkg +++ b/bin/rpkg @@ -15,6 +15,7 @@ import logging import os import sys +import six from six.moves import configparser import pyrpkg @@ -34,7 +35,11 @@ if not os.path.exists(args.config) and not other[-1] in ['--help', '-h']: sys.exit(1) # Setup a configuration object and read config file data -config = configparser.SafeConfigParser() +if six.PY2: + config = configparser.SafeConfigParser() +else: + # The SafeConfigParser class has been renamed to ConfigParser in Python 3.2. + config = configparser.ConfigParser() config.read(args.config) client = pyrpkg.cli.cliClient(config) diff --git a/tests/test_retire.py b/tests/test_retire.py index 02c6976..3f68a2a 100644 --- a/tests/test_retire.py +++ b/tests/test_retire.py @@ -6,6 +6,7 @@ import subprocess import tempfile import mock +import six from six.moves import configparser import pyrpkg.cli @@ -52,7 +53,11 @@ class RetireTestCase(unittest.TestCase): return out.strip() def _fake_client(self, args): - config = configparser.SafeConfigParser() + if six.PY2: + config = configparser.SafeConfigParser() + else: + # The SafeConfigParser class has been renamed to ConfigParser in Python 3.2. + config = configparser.ConfigParser() config.read(TEST_CONFIG) with mock.patch('sys.argv', new=args): client = pyrpkg.cli.cliClient(config, name='rpkg') From 6fad855280aabe5c3091137486094cf4e77e41d5 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jul 06 2021 15:31:58 +0000 Subject: [PATCH 3/6] Detect generic use of rpmautospec features Signed-off-by: Nils Philippsen --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index 55ec8a1..2376990 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -161,6 +161,8 @@ class Commands(object): self._rel = None # Whether the spec file uses %autorelease self._uses_autorelease = None + # Whether the spec file uses rpmautospec features (at all) + self._uses_rpmautospec = None # The cloned repo object self._repo = None # The rpm defines used when calling rpm @@ -646,6 +648,12 @@ class Commands(object): self.load_nameverrel() return self._uses_autorelease + @property + def uses_rpmautospec(self): + if self._uses_rpmautospec is None: + self.load_nameverrel() + return self._uses_rpmautospec + def load_nameverrel(self): """Set the release of a package.""" @@ -655,9 +663,11 @@ class Commands(object): self._uses_autorelease = specfile_uses_rpmautospec( specfile_path, check_autorelease=True, check_autochangelog=False ) + self._uses_rpmautospec = specfile_uses_rpmautospec(specfile_path) else: # Set to 0 so it evaluates false-ish but differs from (unset) None. self._uses_autorelease = 0 + self._uses_rpmautospec = 0 cmd = ['rpm'] cmd.extend(self.rpmdefines) From 938d5bdfc0d7c032b20e5da3325f0d93bdb5c4c8 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jul 06 2021 15:31:58 +0000 Subject: [PATCH 4/6] Preprocess spec files using rpmautospec features If spec files use rpmautospec features, preprocess them into a temporary directory and point rpmbuild at the pre-processed spec file. Signed-off-by: Nils Philippsen --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index 2376990..1ede227 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -72,6 +72,11 @@ try: except ImportError: specfile_uses_rpmautospec = None +try: + from rpmautospec import process_distgit as rpmautospec_process_distgit +except ImportError: + rpmautospec_process_distgit = None + class NullHandler(logging.Handler): """Null logger to avoid spurious messages, add a handler in app code""" @@ -2585,19 +2590,30 @@ class Commands(object): % 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) - - cmd = '%s 2>&1 | tee %s' % (' '.join(cmd), logfile) + specpath = os.path.join(self.path, self.spec) + tmpdir = None try: - # Since zsh is a widely used, which is supported by fedpkg - # actually, pipestatus is for checking the first command when zsh - # is used. - subprocess.check_call( - '%s; exit "${PIPESTATUS[0]} ${pipestatus[1]}"' % cmd, - shell=True) - except subprocess.CalledProcessError: - raise rpkgError(cmd) + if not self.uses_rpmautospec or not rpmautospec_process_distgit: + cmd.extend(['-ba', specpath]) + else: + tmpdir = tempfile.mkdtemp(prefix="rpkg-rpmautospec") + tmpspecpath = os.path.join(tmpdir, self.spec) + rpmautospec_process_distgit(specpath, tmpspecpath) + cmd.extend(['-ba', tmpspecpath]) + logfile = '.build-%s-%s.log' % (self.ver, self.rel) + + cmd = '%s 2>&1 | tee %s' % (' '.join(cmd), logfile) + try: + # Since zsh is a widely used, which is supported by fedpkg + # actually, pipestatus is for checking the first command when zsh + # is used. + subprocess.check_call( + '%s; exit "${PIPESTATUS[0]} ${pipestatus[1]}"' % cmd, + shell=True) + except subprocess.CalledProcessError: + raise rpkgError(cmd) + finally: + self._cleanup_tmp_dir(tmpdir) # Not to be confused with mockconfig the property def mock_config(self, target=None, arch=None): @@ -2960,8 +2976,19 @@ class Commands(object): % hashtype, "--define '_binary_filedigest_algorithm %s'" % hashtype]) - cmd.extend(['--nodeps', '-bs', os.path.join(self.path, self.spec)]) - self._run_command(cmd, shell=True) + specpath = os.path.join(self.path, self.spec) + tmpdir = None + try: + if not self.uses_rpmautospec or not rpmautospec_process_distgit: + cmd.extend(['--nodeps', '-bs', specpath]) + else: + tmpdir = tempfile.mkdtemp(prefix="rpkg-rpmautospec") + tmpspecpath = os.path.join(tmpdir, self.spec) + rpmautospec_process_distgit(specpath, tmpspecpath) + cmd.extend(['--nodeps', '-bs', tmpspecpath]) + self._run_command(cmd, shell=True) + finally: + self._cleanup_tmp_dir(tmpdir) def unused_patches(self): """Discover patches checked into source control that are not used From 7edb51c2e7d3c5582bea5e7123a2f2e3bb5f3d9c Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jul 06 2021 15:31:58 +0000 Subject: [PATCH 5/6] Reflect %autorelease when parsing spec files If the %autorelease macro is used, calculate the release number and pass it to the macro when parsing the spec file. Signed-off-by: Nils Philippsen --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index 1ede227..a7d834d 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -74,8 +74,10 @@ except ImportError: try: 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 class NullHandler(logging.Handler): @@ -662,6 +664,9 @@ class Commands(object): def load_nameverrel(self): """Set the release of a package.""" + cmd = ['rpm'] + cmd.extend(self.rpmdefines) + specfile_path = os.path.join(self.path, self.spec) if specfile_uses_rpmautospec: @@ -669,13 +674,14 @@ class Commands(object): specfile_path, check_autorelease=True, check_autochangelog=False ) self._uses_rpmautospec = specfile_uses_rpmautospec(specfile_path) + if self._uses_rpmautospec and rpmautospec_calculate_release_number: + release_number = rpmautospec_calculate_release_number(specfile_path) + cmd.append("--define '_rpmautospec_release_number %d'" % release_number) else: # Set to 0 so it evaluates false-ish but differs from (unset) None. self._uses_autorelease = 0 self._uses_rpmautospec = 0 - cmd = ['rpm'] - cmd.extend(self.rpmdefines) # We make sure there is a space at the end of our query so that # we can split it later. When there are subpackages, we get a # listing for each subpackage. We only care about the first. From fb9876cda299f7f5fb9788488d28d2f5052d878e Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Jul 06 2021 15:31:58 +0000 Subject: [PATCH 6/6] Add and augment tests for rpmautospec Signed-off-by: Nils Philippsen --- diff --git a/tests/test_commands.py b/tests/test_commands.py index 57b3bc8..2575dc6 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -74,9 +74,20 @@ class LoadNameVerRelTest(CommandTestCase): self.cmd = self.make_commands() self.checkout_branch(self.cmd.repo, 'eng-rhel-6') self.tempdir = tempfile.mkdtemp(prefix='rpkg_test_') + self._patchers = { + name: patch("pyrpkg.%s" % name) for name in ( + "specfile_uses_rpmautospec", + "rpmautospec_process_distgit", + "rpmautospec_calculate_release_number", + ) + } + self.mocks = {name: patcher.start() for name, patcher in self._patchers.items()} + self.mocks["specfile_uses_rpmautospec"].return_value = False def tearDown(self): super(LoadNameVerRelTest, self).tearDown() + for patcher in self._patchers.values(): + patcher.stop() shutil.rmtree(self.tempdir) def test_load_from_spec(self): @@ -118,6 +129,8 @@ class LoadNameVerRelTest(CommandTestCase): self.assertEqual('0', cmd._epoch) self.assertEqual('1.2', cmd._ver) self.assertEqual('2.el6', cmd._rel) + self.assertIs(False, cmd._uses_autorelease) + self.assertIs(False, cmd._uses_rpmautospec) @patch('pyrpkg.Commands.load_rpmdefines', new=mock_load_rpmdefines) @patch('pyrpkg.Commands.load_spec', @@ -145,6 +158,30 @@ class LoadNameVerRelTest(CommandTestCase): self.assertEqual('1.2', self.cmd._ver) self.assertEqual('2.el6', self.cmd._rel) + @patch("pyrpkg.specfile_uses_rpmautospec", new=None) + @patch("pyrpkg.rpmautospec_process_distgit", new=None) + @patch("pyrpkg.rpmautospec_calculate_release_number", new=None) + def test_load_with_rpmautospec_pkg_missing(self): + self.cmd.load_nameverrel() + self.assertIs(0, self.cmd._uses_autorelease) + self.assertIs(0, self.cmd._uses_rpmautospec) + + @patch("subprocess.Popen", wraps=subprocess.Popen) + def test_load_with_rpmautospec(self, wrapped_popen): + test_release_number = 123 + + self.mocks["specfile_uses_rpmautospec"].return_value = True + self.mocks["rpmautospec_process_distgit"].return_value = True + self.mocks["rpmautospec_calculate_release_number"].return_value = test_release_number + + self.cmd.load_nameverrel() + + self.assertIs(True, self.cmd._uses_autorelease) + self.assertIs(True, self.cmd._uses_rpmautospec) + self.assertEqual(1, wrapped_popen.call_count) + args, kwargs = wrapped_popen.call_args + self.assertIn("--define '_rpmautospec_release_number %d'" % test_release_number, args[0]) + class LoadBranchMergeTest(CommandTestCase): """Test case for testing Commands.load_branch_merge"""