From 35cd3c7a3625afcfca253b15a81373a2686483df Mon Sep 17 00:00:00 2001 From: Ondrej Nosek Date: Jul 27 2022 02:30:03 +0000 Subject: rpmdefines changes depending on rpkg 'rpmdefines' structure needs a different format after it was changed in rpkg (RHELCMP-9241). Also fixes some bandit security findings. Relates: https://pagure.io/rpkg/pull-request/620 JIRA: RHELCMP-9646 Signed-off-by: Ondrej Nosek --- diff --git a/fedpkg/__init__.py b/fedpkg/__init__.py index f041b97..067867d 100644 --- a/fedpkg/__init__.py +++ b/fedpkg/__init__.py @@ -125,8 +125,8 @@ class Commands(pyrpkg.Commands): self.override = 'eln-override' self._distunset = 'fedora' extra_rpmdefines = [ - "--define 'el%s 1'" % macros['rhel'], - "--define 'rhel %s'" % macros['rhel'], + "--define", "el%s 1" % macros['rhel'], + "--define", "rhel %s" % macros['rhel'], ] # rawhide (previously master branch) elif re.match(r'(rawhide|main)$', self.branch_merge): @@ -141,25 +141,24 @@ class Commands(pyrpkg.Commands): raise pyrpkg.rpkgError('Could not find the release/dist from branch name ' '%s\nPlease specify with --release' % self.branch_merge) - self._rpmdefines = ["--define '_sourcedir %s'" % self.layout.sourcedir, - "--define '_specdir %s'" % self.layout.specdir, - "--define '_builddir %s'" % self.layout.builddir, - "--define '_srcrpmdir %s'" % self.layout.srcrpmdir, - "--define '_rpmdir %s'" % self.layout.rpmdir, - "--define '_rpmfilename %s'" % self.layout.rpmfilename, - "--define 'dist %%{?distprefix}.%s'" % self._disttag, - "--define '%s %s'" % (self._distvar, - self._distval), - "--eval '%%undefine %s'" % self._distunset, - "--define '%s 1'" % self._disttag.replace(".", "_")] + self._rpmdefines = ["--define", "_sourcedir %s" % self.layout.sourcedir, + "--define", "_specdir %s" % self.layout.specdir, + "--define", "_builddir %s" % self.layout.builddir, + "--define", "_srcrpmdir %s" % self.layout.srcrpmdir, + "--define", "_rpmdir %s" % self.layout.rpmdir, + "--define", "_rpmfilename %s" % self.layout.rpmfilename, + "--define", "dist %%{?distprefix}.%s" % self._disttag, + "--define", "%s %s" % (self._distvar, self._distval), + "--eval", "%%undefine %s" % self._distunset, + "--define", "%s 1" % self._disttag.replace(".", "_")] # TODO: consider removing macro "%s 1; it has unknown/dubious functionality" if self._runtime_disttag: if self._disttag != self._runtime_disttag: # This means that the runtime is known, and is different from # the target, so we need to unset the _runtime_disttag - self._rpmdefines.append("--eval '%%undefine %s'" % - self._runtime_disttag) + self._rpmdefines.extend(["--eval", "%%undefine %s" % + self._runtime_disttag]) if extra_rpmdefines: self._rpmdefines.extend(extra_rpmdefines) diff --git a/fedpkg/cli.py b/fedpkg/cli.py index d24e83c..638e557 100644 --- a/fedpkg/cli.py +++ b/fedpkg/cli.py @@ -873,7 +873,7 @@ class fedpkgClient(cliClient): if not self.args.update_type or not self.args.notes: # Open the template in a text editor editor = os.getenv('EDITOR', 'vi') - self.cmd._run_command([editor, template_file], shell=True) + self.cmd._run_command([editor, template_file], shell=False) # Check to see if we got a template written out. Bail otherwise if not os.path.isfile(template_file): diff --git a/test/test_cli.py b/test/test_cli.py index f956dcc..43bd20a 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -278,7 +278,7 @@ class TestUpdate(CliTestCase): self.assert_bodhi_update, cli) self.mock_run_command.assert_called_once_with( - ['vi', 'bodhi.template'], shell=True) + ['vi', 'bodhi.template'], shell=False) def test_request_update(self): cli_cmd = ['fedpkg', '--path', self.cloned_repo_path, 'update'] @@ -287,7 +287,7 @@ class TestUpdate(CliTestCase): self.assert_bodhi_update(cli, update_type='enhancement') self.mock_run_command.assert_called_once_with( - ['vi', 'bodhi.template'], shell=True) + ['vi', 'bodhi.template'], shell=False) def test_request_update_with_karma(self): cli_cmd = [ @@ -316,7 +316,7 @@ class TestUpdate(CliTestCase): self.assert_bodhi_update(cli, update_type='enhancement') self.mock_run_command.assert_called_once_with( - ['vi', 'bodhi.template'], shell=True) + ['vi', 'bodhi.template'], shell=False) def test_missing_update_type_in_template(self): cli_cmd = ['fedpkg-stage', '--path', self.cloned_repo_path, 'update'] @@ -369,7 +369,7 @@ class TestUpdate(CliTestCase): self.assert_bodhi_update( cli, update_type='enhancement', notes='Mass rebuild') self.mock_run_command.assert_called_once_with( - ['vi', 'bodhi.template'], shell=True) + ['vi', 'bodhi.template'], shell=False) def test_show_editor_if_notes_is_omitted(self): cli_cmd = [ @@ -381,7 +381,7 @@ class TestUpdate(CliTestCase): self.assert_bodhi_update(cli, update_type='bugfix') self.mock_run_command.assert_called_once_with( - ['vi', 'bodhi.template'], shell=True) + ['vi', 'bodhi.template'], shell=False) def test_create_with_notes_in_multiple_lines(self): cli_cmd = [ diff --git a/test/test_commands.py b/test/test_commands.py index c7467ae..1ea3e2d 100644 --- a/test/test_commands.py +++ b/test/test_commands.py @@ -185,17 +185,17 @@ class TestLoadRpmDefines(CommandTestCase): def assert_rpmdefines(self, extra_rpmdefines=[]): """Assert Commands._rpmdefines after calling load_rpmdefines""" expected_rpmdefines = [ - "--define '_sourcedir %s'" % self.cmd.layout.sourcedir, - "--define '_specdir %s'" % self.cmd.layout.specdir, - "--define '_builddir %s'" % self.cmd.layout.builddir, - "--define '_srcrpmdir %s'" % self.cmd.layout.srcrpmdir, - "--define '_rpmdir %s'" % self.cmd.layout.rpmdir, - "--define '_rpmfilename %s'" % self.cmd.layout.rpmfilename, - "--define 'dist %%{?distprefix}.%s'" % self.cmd._disttag, - "--define '%s %s'" % (self.cmd._distvar, self.cmd._distval), - "--eval '%%undefine %s'" % self.cmd._distunset, - "--define '%s 1'" % self.cmd._disttag.replace(".", "_"), - "--eval '%%undefine %s'" % self.cmd._runtime_disttag + "--define", "_sourcedir %s" % self.cmd.layout.sourcedir, + "--define", "_specdir %s" % self.cmd.layout.specdir, + "--define", "_builddir %s" % self.cmd.layout.builddir, + "--define", "_srcrpmdir %s" % self.cmd.layout.srcrpmdir, + "--define", "_rpmdir %s" % self.cmd.layout.rpmdir, + "--define", "_rpmfilename %s" % self.cmd.layout.rpmfilename, + "--define", "dist %%{?distprefix}.%s" % self.cmd._disttag, + "--define", "%s %s" % (self.cmd._distvar, self.cmd._distval), + "--eval", "%%undefine %s" % self.cmd._distunset, + "--define", "%s 1" % self.cmd._disttag.replace(".", "_"), + "--eval", "%%undefine %s" % self.cmd._runtime_disttag ] expected_rpmdefines.extend(extra_rpmdefines) self.assertEqual(expected_rpmdefines, self.cmd._rpmdefines) @@ -338,8 +338,8 @@ class TestLoadRpmDefines(CommandTestCase): self.assertEqual('fedora', self.cmd._distunset) extra_rpmdefines = [ - "--define 'el9 1'", - "--define 'rhel 9'", + "--define", "el9 1", + "--define", "rhel 9", ] self.assert_rpmdefines(extra_rpmdefines)