Notification time stamped 2023-04-01 00:05:45 UTC
From bd87270301b52a4b0c99555aa070378a8c6cc736 Mon Sep 17 00:00:00 2001 From: Ondřej Nosek onosek@redhat.com Date: Mar 31 2023 23:54:46 +0000 Subject: A few patches mostly for `pre-push-check`
- Patch: Fix unittests for `clone` and pre-push hook script - Patch: pre-push hook script contains a user's config - Patch: A HEAD query into a lookaside cache - Patch: `pre-push-check` have to use spectool with --define - Patch: Add more information about pre-push hook - Patch: Update to spec file presence checking - Patch: More robust spec file presence checking
Signed-off-by: Ondřej Nosek onosek@redhat.com
---
diff --git a/0007-More-robust-spec-file-presence-checking.patch b/0007-More-robust-spec-file-presence-checking.patch new file mode 100644 index 0000000..a1dde64 --- /dev/null +++ b/0007-More-robust-spec-file-presence-checking.patch @@ -0,0 +1,292 @@ +From 1108810bdefd0d880517b274acd6a3bd0d4156e0 Mon Sep 17 00:00:00 2001 +From: Ondrej Nosek onosek@redhat.com +Date: Tue, 21 Mar 2023 02:44:04 +0100 +Subject: [PATCH 07/12] More robust spec file presence checking + +Some commands (verrel, sources, prep, import, ...) need to check +whether the dist-git repository is in the correct state. It means +at least the presence of the specfile. +In the beginning, rpkg detects layouts. Layouts determine the file +structure of the repository. For example, most commands can't +be executed for the RetiredLayout (there is no specfile). +When the repository directory exists, some layout can be always +detected. Therefore '--path' argument is now checked for +a valid directory. +The timeout change in the request fixes the new bandit's finding. + +Fixes: #663 +JIRA: RHELCMP-11387 + +Signed-off-by: Ondrej Nosek onosek@redhat.com +--- + pyrpkg/__init__.py | 9 ++++--- + pyrpkg/cli.py | 8 +++--- + pyrpkg/layout/__init__.py | 4 +-- + pyrpkg/utils.py | 14 ++++++++++ + tests/commands/test_push.py | 54 +++++++++++++++++++------------------ + tests/test_cli.py | 12 ++++++--- + 6 files changed, 63 insertions(+), 38 deletions(-) + +diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py +index 776cb21..028d195 100644 +--- a/pyrpkg/__init__.py ++++ b/pyrpkg/__init__.py +@@ -923,9 +923,8 @@ class Commands(object): + def load_spec(self): + """This sets the spec attribute""" + +- if self.layout is None: ++ if self.layout is None or isinstance(self.layout, layout.IncompleteLayout): + raise rpkgError('Spec file is not available') +- + if self.is_retired(): + raise rpkgError('This package or module is retired. The action has stopped.') + +@@ -1166,8 +1165,10 @@ class Commands(object): + + @property + def sources_filename(self): +- if self.layout is None: +- return os.path.join(self.path, 'sources') ++ if self.layout is None or isinstance(self.layout, layout.IncompleteLayout): ++ raise rpkgError('Spec file is not available') ++ if self.is_retired(): ++ raise rpkgError('This package or module is retired. The action has stopped.') + return os.path.join( + self.path, self.layout.sources_file_template.replace("{0.repo_name}", self.repo_name)) + +diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py +index c3672b3..1bcf6e4 100644 +--- a/pyrpkg/cli.py ++++ b/pyrpkg/cli.py +@@ -386,7 +386,7 @@ class cliClient(object): + help='Run Koji commands as a different user') + # Let the user define a path to work in rather than cwd + self.parser.add_argument('--path', default=None, +- type=utils.u, ++ type=utils.validate_path, + help='Define the directory to work in ' + '(defaults to cwd)') + # Verbosity +@@ -911,8 +911,9 @@ class cliClient(object): + if 'path' in args: + # Without "path", we can't really test... + url = '%(protocol)s://%(host)s/%(path)s/info/refs?service=git-receive-pack' % args +- resp = requests.head(url, auth=HTTPBasicAuth(args['username'], +- args['password'])) ++ resp = requests.head(url, ++ auth=HTTPBasicAuth(args['username'], args['password']), ++ timeout=15) + if resp.status_code == 401: + return self.oidc_client.report_token_issue() + +@@ -2363,6 +2364,7 @@ class cliClient(object): + + def import_srpm(self): + uploadfiles = self.cmd.import_srpm(self.args.srpm) ++ self.load_cmd() # to reload layouts - because a specfile could appear during import + if uploadfiles: + try: + self.cmd.upload(uploadfiles, replace=True, offline=self.args.offline) +diff --git a/pyrpkg/layout/__init__.py b/pyrpkg/layout/__init__.py +index 762af0d..850ddc2 100644 +--- a/pyrpkg/layout/__init__.py ++++ b/pyrpkg/layout/__init__.py +@@ -12,8 +12,8 @@ + from pyrpkg.errors import LayoutError + + from .base import MetaLayout +-from .layouts import (DistGitLayout, IncompleteLayout, # noqa: F401 +- RetiredLayout, SRPMLayout) ++from .layouts import (DistGitLayout, DistGitResultsDirLayout, # noqa: F401 ++ IncompleteLayout, RetiredLayout, SRPMLayout) + + + def build(path, hint=None): +diff --git a/pyrpkg/utils.py b/pyrpkg/utils.py +index ceb4906..3337bdb 100644 +--- a/pyrpkg/utils.py ++++ b/pyrpkg/utils.py +@@ -26,11 +26,25 @@ if six.PY3: + def u(s): + return s + ++ def validate_path(s): ++ abspath = os.path.abspath(s) ++ if os.path.exists(abspath): ++ return s ++ else: ++ raise argparse.ArgumentTypeError('given path '{0}' doesn't exist'.format(abspath)) ++ + getcwd = os.getcwd + else: + def u(s): + return s.decode('utf-8') + ++ def validate_path(s): ++ abspath = os.path.abspath(s.decode('utf-8')) ++ if os.path.exists(abspath): ++ return s.decode('utf-8') ++ else: ++ raise argparse.ArgumentTypeError('given path '{0}' doesn't exist'.format(abspath)) ++ + getcwd = os.getcwdu + + +diff --git a/tests/commands/test_push.py b/tests/commands/test_push.py +index ef8057a..79c3a8b 100644 +--- a/tests/commands/test_push.py ++++ b/tests/commands/test_push.py +@@ -1,9 +1,13 @@ + # -*- coding: utf-8 -*- + + import os ++import subprocess + + import git + ++import pyrpkg ++from pyrpkg.sources import SourcesFile ++ + from . import CommandTestCase + + SPECFILE_TEMPLATE = """Name: test +@@ -22,11 +26,6 @@ Test + %%install + rm -f $RPM_BUILD_ROOT%%{_sysconfdir}/""" + +-CLONE_CONFIG = ''' +- bz.default-component %(module)s +- sendemail.to %(module)s-owner@fedoraproject.org +-''' +- + + class CommandPushTestCase(CommandTestCase): + +@@ -45,28 +44,30 @@ class CommandPushTestCase(CommandTestCase): + + 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.kojiprofile, +- self.build_client, self.user, self.dist, +- self.target, self.quiet) +- cmd.clone_config_rpms = 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)) ++ moduledir = os.path.join(self.gitroot, self.module) ++ subprocess.check_call(['git', 'clone', 'file://%s' % moduledir], ++ cwd=self.path, stdout=subprocess.PIPE, ++ stderr=subprocess.PIPE) ++ ++ self.cloned_dir = os.path.join(self.path, self.module) ++ self.cmd = pyrpkg.Commands(self.cloned_dir, self.lookaside, ++ self.lookasidehash, ++ self.lookaside_cgi, self.gitbaseurl, ++ self.anongiturl, self.branchre, self.kojiprofile, ++ self.build_client, self.user, self.dist, ++ self.target, self.quiet) ++ os.chdir(self.cloned_dir) + + 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") ++ self.cmd.repo.index.add([spec_file]) ++ self.cmd.repo.index.commit("add SPEC") + + # Now, change directory to parent and test the push + os.chdir(self.path) +- cmd.push(no_verify=True) ++ self.cmd.push(no_verify=True) + + + class TestPushWithPatches(CommandTestCase): +@@ -76,18 +77,20 @@ class TestPushWithPatches(CommandTestCase): + + self.make_new_git(self.module) + +- import pyrpkg +- self.cmd = pyrpkg.Commands(self.path, self.lookaside, ++ moduledir = os.path.join(self.gitroot, self.module) ++ subprocess.check_call(['git', 'clone', 'file://%s' % moduledir], ++ cwd=self.path, stdout=subprocess.PIPE, ++ stderr=subprocess.PIPE) ++ ++ self.cloned_dir = os.path.join(self.path, self.module) ++ self.cmd = pyrpkg.Commands(self.cloned_dir, self.lookaside, + self.lookasidehash, + self.lookaside_cgi, self.gitbaseurl, + self.anongiturl, self.branchre, + self.kojiprofile, + self.build_client, self.user, self.dist, + self.target, self.quiet) +- self.cmd.clone_config_rpms = 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)) ++ os.chdir(self.cloned_dir) + + # Track SPEC and a.patch in git + spec_file = 'module.spec' +@@ -103,7 +106,6 @@ Patch3: d.path + 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') +diff --git a/tests/test_cli.py b/tests/test_cli.py +index df053aa..868ad1f 100644 +--- a/tests/test_cli.py ++++ b/tests/test_cli.py +@@ -1841,9 +1841,11 @@ class TestMockbuild(CliTestCase): + @patch('pyrpkg.Commands._config_dir_basic') + @patch('pyrpkg.Commands._config_dir_other') + @patch('os.path.exists', return_value=False) ++ @patch('pyrpkg.utils.validate_path') + def test_use_mock_config_got_from_koji( +- self, exists, config_dir_other, config_dir_basic): ++ self, validate_path, exists, config_dir_other, config_dir_basic): + mock_layout = layout.DistGitLayout(root_dir=self.cloned_repo_path) ++ validate_path.return_value = self.cloned_repo_path + with patch('pyrpkg.layout.build', return_value=mock_layout): + config_dir_basic.return_value = '/path/to/config-dir' + +@@ -1859,9 +1861,11 @@ class TestMockbuild(CliTestCase): + + @patch('pyrpkg.Commands._config_dir_basic') + @patch('os.path.exists', return_value=False) ++ @patch('pyrpkg.utils.validate_path') + def test_fail_to_store_mock_config_in_created_config_dir( +- self, exists, config_dir_basic): ++ self, validate_path, exists, config_dir_basic): + config_dir_basic.side_effect = rpkgError ++ validate_path.return_value = self.cloned_repo_path + + cli_cmd = ['rpkg', '--path', self.cloned_repo_path, + '--release', 'rhel-7', 'mockbuild'] +@@ -1870,10 +1874,12 @@ class TestMockbuild(CliTestCase): + @patch('pyrpkg.Commands._config_dir_basic') + @patch('pyrpkg.Commands._config_dir_other') + @patch('os.path.exists', return_value=False) ++ @patch('pyrpkg.utils.validate_path') + def test_fail_to_populate_mock_config( +- self, exists, config_dir_other, config_dir_basic): ++ self, validate_path, exists, config_dir_other, config_dir_basic): + config_dir_basic.return_value = '/path/to/config-dir' + config_dir_other.side_effect = rpkgError ++ validate_path.return_value = self.cloned_repo_path + + cli_cmd = ['rpkg', '--path', self.cloned_repo_path, + '--release', 'rhel-7', 'mockbuild'] +-- +2.39.2 + diff --git a/0008-Update-to-spec-file-presence-checking.patch b/0008-Update-to-spec-file-presence-checking.patch new file mode 100644 index 0000000..723415f --- /dev/null +++ b/0008-Update-to-spec-file-presence-checking.patch @@ -0,0 +1,43 @@ +From 791fd03b4de1324508583ab53c89cc67459db355 Mon Sep 17 00:00:00 2001 +From: Ondrej Nosek onosek@redhat.com +Date: Tue, 21 Mar 2023 13:44:38 +0100 +Subject: [PATCH 08/12] Update to spec file presence checking + +Using a different approach to checking the layout. Older way prevented +`retire` function working correctly. Layouts are detected at the +beginning of the run and the result stays the same, unlike the direct +checking files like dead.package in function `is_retired`. + +Fixes: #663 +JIRA: RHELCMP-11387 + +Signed-off-by: Ondrej Nosek onosek@redhat.com +--- + pyrpkg/__init__.py | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py +index 028d195..e8f4886 100644 +--- a/pyrpkg/__init__.py ++++ b/pyrpkg/__init__.py +@@ -925,7 +925,7 @@ class Commands(object): + + if self.layout is None or isinstance(self.layout, layout.IncompleteLayout): + raise rpkgError('Spec file is not available') +- if self.is_retired(): ++ if isinstance(self.layout, layout.RetiredLayout): + raise rpkgError('This package or module is retired. The action has stopped.') + + # Get a list of ".spec" files in the path we're looking at +@@ -1167,7 +1167,7 @@ class Commands(object): + def sources_filename(self): + if self.layout is None or isinstance(self.layout, layout.IncompleteLayout): + raise rpkgError('Spec file is not available') +- if self.is_retired(): ++ if isinstance(self.layout, layout.RetiredLayout): + raise rpkgError('This package or module is retired. The action has stopped.') + return os.path.join( + self.path, self.layout.sources_file_template.replace("{0.repo_name}", self.repo_name)) +-- +2.39.2 + diff --git a/0009-Add-more-information-about-pre-push-hook.patch b/0009-Add-more-information-about-pre-push-hook.patch new file mode 100644 index 0000000..60bad27 --- /dev/null +++ b/0009-Add-more-information-about-pre-push-hook.patch @@ -0,0 +1,44 @@ +From 0393dc39bf450cf20df9db63bac135c078f64a14 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= lsedlar@redhat.com +Date: Tue, 28 Mar 2023 08:53:30 +0200 +Subject: [PATCH 09/12] Add more information about pre-push hook +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +It's not obvious to many users where the check is coming from, and they +have the power to edit the script or delete it completely. Let's try to +improve that a bit. + +Signed-off-by: Lubomír Sedlář lsedlar@redhat.com +--- + pyrpkg/__init__.py | 6 +++++- + 1 file changed, 5 insertions(+), 1 deletion(-) + +diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py +index e8f4886..7a3c9c6 100644 +--- a/pyrpkg/__init__.py ++++ b/pyrpkg/__init__.py +@@ -1806,6 +1806,10 @@ class Commands(object): + hook_content = textwrap.dedent(""" + #!/bin/bash + ++ # This file was generated by {0} when cloning the repository. ++ # You can edit it to your liking or delete completely. It will not ++ # be recreated. ++ + _remote="$1" + _url="$2" + +@@ -4429,7 +4433,7 @@ class Commands(object): + return self._repo_name, version, release + + def pre_push_check(self, ref): +- show_hint = ('Hint: this check (pre-push hook script) can be bypassed by adding ' ++ show_hint = ('Hint: this check (.git/hooks/pre-push script) can be bypassed by adding ' + 'the argument '--no-verify' argument to the push command.') + try: + commit = self.repo.commit(ref) +-- +2.39.2 + diff --git a/0010-pre-push-check-have-to-use-spectool-with-define.patch b/0010-pre-push-check-have-to-use-spectool-with-define.patch new file mode 100644 index 0000000..bfe7094 --- /dev/null +++ b/0010-pre-push-check-have-to-use-spectool-with-define.patch @@ -0,0 +1,146 @@ +From d5be51eec99108c3809551b615064d0c5cbe628a Mon Sep 17 00:00:00 2001 +From: Ondrej Nosek onosek@redhat.com +Date: Tue, 28 Mar 2023 19:58:06 +0200 +Subject: [PATCH 10/12] `pre-push-check` have to use spectool with --define + +To get all defined source files and patches from the specfile, +the 'spectool' utility needs '--define' argument(s) to set specific +paths for the repository. + +JIRA: RHELCMP-11466 +Fixes: #672 + +Signed-off-by: Ondrej Nosek onosek@redhat.com +--- + pyrpkg/__init__.py | 57 +++++++++++++++------------ + tests/commands/test_pre_push_check.py | 3 +- + 2 files changed, 33 insertions(+), 27 deletions(-) + +diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py +index 7a3c9c6..584c141 100644 +--- a/pyrpkg/__init__.py ++++ b/pyrpkg/__init__.py +@@ -4442,30 +4442,41 @@ class Commands(object): + sys.exit(1) + + try: ++ clone_dir = tempfile.mkdtemp(prefix="pre_push_hook_") ++ for cmd in [ ++ ('git', 'clone', self.path, clone_dir), ++ ('git', 'checkout', ref), ++ ]: ++ ret, _, _ = self._run_command(cmd, cwd=clone_dir, ++ # suppress unwanted printing of command line messages ++ return_stdout=True, return_stderr=True) ++ if ret != 0: ++ self.log.error('Command '{0}' failed. Push operation ' ++ 'was cancelled.'.format(' '.join(cmd))) ++ self.log.warning(show_hint) ++ sys.exit(2) ++ ++ # get all source files from the specfile (including patches) + # Assume, that specfile names are same in the active branch + # and in the pushed branch (git checkout f37 && git push origin rawhide) + # in this case 'f37' is active branch and 'rawhide' is pushed branch. + specfile_path_absolute = os.path.join(self.layout.specdir, self.spec) + # convert to relative path + specfile_path = os.path.relpath(specfile_path_absolute, start=self.path) +- spec_content = self.repo.git.cat_file("-p", "{0}:{1}".format(ref, specfile_path)) +- except Exception: +- # It might be the case of an empty commit +- self.log.warning('Specfile doesn't exist. Push operation continues.') +- return +- +- # load specfile content from pushed branch and save it into a temporary file +- with tempfile.NamedTemporaryFile(mode="w+") as temporary_spec: +- temporary_spec.write(spec_content) +- temporary_spec.flush() +- # get all source files from the specfile (including patches) +- cmd = ('spectool', '-l', temporary_spec.name) +- ret, stdout, _ = self._run_command(cmd, return_text=True, return_stdout=True) ++ cmd = ['spectool', '-l', os.path.join(clone_dir, specfile_path)] ++ # extract just '--define' arguments from rpmdefines ++ for opt, val in zip(self.rpmdefines[0::2], self.rpmdefines[1::2]): ++ if opt == '--define': ++ cmd.extend((opt, val)) ++ ret, stdout, _ = self._run_command(cmd, cwd=clone_dir, ++ return_text=True, return_stdout=True) + if ret != 0: + self.log.error('Command '{0}' failed. Push operation ' + 'was cancelled.'.format(' '.join(cmd))) + self.log.warning(show_hint) +- sys.exit(2) ++ sys.exit(3) ++ finally: ++ self._cleanup_tmp_dir(clone_dir) + + source_files = [] + # extract source files from the spectool's output +@@ -4490,22 +4501,16 @@ class Commands(object): + sources_file_path_absolute = self.sources_filename + # convert to relative path + sources_file_path = os.path.relpath(sources_file_path_absolute, start=self.path) +- sources_file_content = self.repo.git.cat_file( +- '-p', '{0}:{1}'.format(ref, sources_file_path)) ++ ++ # parse 'sources' files content ++ sourcesf = SourcesFile(sources_file_path, self.source_entry_type) ++ sourcesf_entries = set(item.file for item in sourcesf.entries) + except Exception: + self.log.warning(''sources' file doesn't exist. Push operation continues.') + # NOTE: check doesn't fail when 'sources' file doesn't exist. Just skips the rest. + # it might be the case of the push without 'sources' = retiring the repository + return + +- # load 'sources' file content from pushed branch and save it into a temporary file +- with tempfile.NamedTemporaryFile(mode="w+") as temporary_sources_file: +- temporary_sources_file.write(sources_file_content) +- temporary_sources_file.flush() +- # parse 'sources' files content +- sourcesf = SourcesFile(temporary_sources_file.name, self.source_entry_type) +- sourcesf_entries = set(item.file for item in sourcesf.entries) +- + # list of all files (their relative paths) in the commit + repo_entries = set(item.path for item in commit.tree.traverse() if item.type != "tree") + +@@ -4518,7 +4523,7 @@ class Commands(object): + 'nor tracked in git. ' + 'Push operation was cancelled'.format(source_file)) + self.log.warning(show_hint) +- sys.exit(3) ++ sys.exit(4) + + # verify all file entries in 'sources' were uploaded to the lookaside cache + for entry in sourcesf.entries: +@@ -4532,6 +4537,6 @@ class Commands(object): + self.log.error('Source file (or tarball) '{}' wasn't uploaded to the lookaside ' + 'cache. Push operation was cancelled.'.format(filename)) + self.log.warning(show_hint) +- sys.exit(4) ++ sys.exit(5) + + return 0 # The push operation continues +diff --git a/tests/commands/test_pre_push_check.py b/tests/commands/test_pre_push_check.py +index 5e314b9..ee151c1 100644 +--- a/tests/commands/test_pre_push_check.py ++++ b/tests/commands/test_pre_push_check.py +@@ -37,6 +37,7 @@ class TestPrePushCheck(CommandTestCase): + def setUp(self): + super(TestPrePushCheck, self).setUp() + ++ self.dist = "rhel-8" + self.make_new_git(self.module) + + moduledir = os.path.join(self.gitroot, self.module) +@@ -87,7 +88,7 @@ Patch3: d.patch + with self.assertRaises(SystemExit) as exc: + self.cmd.pre_push_check("HEAD") + +- self.assertEqual(exc.exception.code, 3) ++ self.assertEqual(exc.exception.code, 4) + log_error.assert_called_once_with("Source file 'b.patch' was neither listed in the " + "'sources' file nor tracked in git. Push operation " + "was cancelled") +-- +2.39.2 + diff --git a/0011-A-HEAD-query-into-a-lookaside-cache.patch b/0011-A-HEAD-query-into-a-lookaside-cache.patch new file mode 100644 index 0000000..a3281b2 --- /dev/null +++ b/0011-A-HEAD-query-into-a-lookaside-cache.patch @@ -0,0 +1,97 @@ +From 77cd608e596af94811c22a16ff58a265d9c7381e Mon Sep 17 00:00:00 2001 +From: Ondrej Nosek onosek@redhat.com +Date: Fri, 31 Mar 2023 14:09:09 +0200 +Subject: [PATCH 11/12] A HEAD query into a lookaside cache + +A query about whether some file is present in the lookaside cache was +under authentication and it prevented using command `pre-push-check` +for those without the 'packager' permission. +Added another method (based on HTTP HEAD), that allows the same check +without authentication. + +JIRA: RHELCMP-11485 +Fixes: https://pagure.io/fedpkg/issue/513 + +Signed-off-by: Ondrej Nosek onosek@redhat.com +--- + pyrpkg/__init__.py | 2 +- + pyrpkg/lookaside.py | 36 ++++++++++++++++++++++++++++++++++-- + 2 files changed, 35 insertions(+), 3 deletions(-) + +diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py +index 584c141..15203b7 100644 +--- a/pyrpkg/__init__.py ++++ b/pyrpkg/__init__.py +@@ -4529,7 +4529,7 @@ class Commands(object): + for entry in sourcesf.entries: + filename = entry.file + hash = entry.hash +- file_exists_in_lookaside = self.lookasidecache.remote_file_exists( ++ file_exists_in_lookaside = self.lookasidecache.remote_file_exists_head( + self.ns_repo_name if self.lookaside_namespaced else self.repo_name, + filename, + hash) +diff --git a/pyrpkg/lookaside.py b/pyrpkg/lookaside.py +index 90f0f1e..ecbf12b 100644 +--- a/pyrpkg/lookaside.py ++++ b/pyrpkg/lookaside.py +@@ -22,7 +22,7 @@ import sys + + import pycurl + import six +-from six.moves import http_client ++from six.moves import http_client, urllib + + from .errors import (AlreadyUploadedError, DownloadError, InvalidHashType, + UploadError) +@@ -157,7 +157,7 @@ class CGILookasideCache(object): + return + + self.log.info("Downloading %s", filename) +- urled_file = filename.replace(' ', '%20') ++ urled_file = urllib.parse.quote(filename) + url = self.get_download_url(name, urled_file, hash, hashtype, **kwargs) + if isinstance(url, six.text_type): + url = url.encode('utf-8') +@@ -200,6 +200,38 @@ class CGILookasideCache(object): + if not self.file_is_valid(outfile, hash, hashtype=hashtype): + raise DownloadError('%s failed checksum' % filename) + ++ def remote_file_exists_head(self, name, filename, hash): ++ """Verify whether a file exists on the lookaside cache. ++ Uses a HTTP HEAD request and doesn't require authentication. ++ ++ :param str name: The name of the module. (usually the name of the ++ SRPM). This can include the namespace as well (depending on what ++ the server side expects). ++ :param str filename: The name of the file to check for. ++ :param str hash: The known good hash of the file. ++ """ ++ ++ urled_file = urllib.parse.quote(filename) ++ url = self.get_download_url(name, urled_file, hash, self.hashtype) ++ ++ c = pycurl.Curl() ++ c.setopt(pycurl.URL, url) ++ c.setopt(pycurl.NOBODY, True) ++ c.setopt(pycurl.FOLLOWLOCATION, 1) ++ ++ try: ++ c.perform() ++ status = c.getinfo(pycurl.RESPONSE_CODE) ++ except Exception as e: ++ raise DownloadError(e) ++ finally: ++ c.close() ++ ++ if status != 200: ++ self.log.debug('Unavailable file '%s' at %s' % (filename, url)) ++ return False ++ return True ++ + def remote_file_exists(self, name, filename, hash): + """Verify whether a file exists on the lookaside cache + +-- +2.39.2 + diff --git a/0012-pre-push-hook-script-contains-a-user-s-config.patch b/0012-pre-push-hook-script-contains-a-user-s-config.patch new file mode 100644 index 0000000..ff2676d --- /dev/null +++ b/0012-pre-push-hook-script-contains-a-user-s-config.patch @@ -0,0 +1,197 @@ +From 1f03eb9102f765c36cc201a499d815732e67dd39 Mon Sep 17 00:00:00 2001 +From: Ondrej Nosek onosek@redhat.com +Date: Mon, 27 Mar 2023 23:34:12 +0200 +Subject: [PATCH 12/12] pre-push hook script contains a user's config + +When the `clone` command is called with an argument +-C|--config <config_file> +this argument is placed to the generated pre-push script. + +Fixes: #667 +JIRA: RHELCMP-11394 + +Signed-off-by: Ondrej Nosek onosek@redhat.com +--- + pyrpkg/__init__.py | 23 ++++++++++++------- + pyrpkg/cli.py | 6 +++-- + tests/commands/test_clone.py | 44 ++++++++++++++++++++++++++++++++++++ + 3 files changed, 63 insertions(+), 10 deletions(-) + +diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py +index 15203b7..9996402 100644 +--- a/pyrpkg/__init__.py ++++ b/pyrpkg/__init__.py +@@ -1566,7 +1566,8 @@ class Commands(object): + return + + def clone(self, repo, path=None, branch=None, bare_dir=None, +- anon=False, target=None, depth=None, extra_args=None): ++ anon=False, target=None, depth=None, extra_args=None, ++ config_path=None): + """Clone a repo, optionally check out a specific branch. + + :param str repo: the name of the repository to clone. +@@ -1583,6 +1584,7 @@ class Commands(object): + to the specified number of commits. + :param list extra_args: additional arguments that are passed to + the clone command. ++ :param str config_path: path to the global config file + """ + + if not path: +@@ -1638,7 +1640,7 @@ class Commands(object): + + if not bare_dir: + self._add_git_excludes(os.path.join(path, git_dir)) +- self._add_git_pre_push_hook(os.path.join(path, git_dir)) ++ self._add_git_pre_push_hook(os.path.join(path, git_dir), config_path) + + return + +@@ -1654,7 +1656,7 @@ class Commands(object): + return repo + + def clone_with_dirs(self, repo, anon=False, target=None, depth=None, +- extra_args=None): ++ extra_args=None, config_path=None): + """Clone a repo old style with subdirs for each branch. + + :param str repo: name of the repository to clone. +@@ -1666,6 +1668,7 @@ class Commands(object): + to the specified number of commits. + :param list extra_args: additional arguments that are passed to + the clone command. ++ :param str config_path: path to the global config file + """ + + self._push_url = None +@@ -1724,7 +1727,7 @@ class Commands(object): + + # Add excludes + self._add_git_excludes(branch_path) +- self._add_git_pre_push_hook(branch_path) ++ self._add_git_pre_push_hook(branch_path, config_path) + except (git.GitCommandError, OSError) as e: + raise rpkgError('Could not locally clone %s from %s: %s' + % (branch, repo_path, e)) +@@ -1787,7 +1790,7 @@ class Commands(object): + git_excludes.write() + self.log.debug('Git-excludes patterns were added into %s' % git_excludes_path) + +- def _add_git_pre_push_hook(self, conf_dir): ++ def _add_git_pre_push_hook(self, repo_dir, config_path=None): + """ + Create pre-push hook script and write it in the location: + <repository_directory>/.git/hooks/pre-push +@@ -1803,6 +1806,10 @@ class Commands(object): + self.log.debug('Pre-push hook script was NOT added - missing ' + 'the packaging tool like fedpkg, rhpkg, ...') + return ++ ++ # in case the clone command run with 'x-pkg -C <config_path> clone <repo_name>' ++ config_arg = ' -C "{0}"'.format(os.path.realpath(config_path)) if config_path else "" ++ + hook_content = textwrap.dedent(""" + #!/bin/bash + +@@ -1818,7 +1825,7 @@ class Commands(object): + do + command -v {0} >/dev/null 2>&1 || {{ echo >&2 "Warning: '{0}' is missing, \ + pre-push check is omitted. See .git/hooks/pre-push"; exit 0; }} +- {0} pre-push-check "$local_sha" ++ {0}{1} pre-push-check "$local_sha" + ret_code=$? + if [ $ret_code -ne 0 ] && [ $exit_code -eq 0 ]; then + exit_code=$ret_code +@@ -1826,8 +1833,8 @@ class Commands(object): + done + + exit $exit_code +- """).strip().format(tool_name) +- git_pre_push_hook_path = os.path.join(conf_dir, '.git/hooks/pre-push') ++ """).strip().format(tool_name, config_arg) ++ git_pre_push_hook_path = os.path.join(repo_dir, '.git/hooks/pre-push') + if not os.path.exists(os.path.dirname(git_pre_push_hook_path)): + # prepare ".git/hooks" directory if it is missing + os.makedirs(os.path.dirname(git_pre_push_hook_path)) +diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py +index 1bcf6e4..3d8ce33 100644 +--- a/pyrpkg/cli.py ++++ b/pyrpkg/cli.py +@@ -2182,14 +2182,16 @@ class cliClient(object): + anon=self.args.anonymous, + target=self.args.clone_target, + depth=self.args.depth, +- extra_args=self.extra_args) ++ extra_args=self.extra_args, ++ config_path=self.args.config) + 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, +- extra_args=self.extra_args) ++ extra_args=self.extra_args, ++ config_path=self.args.config) + + def commit(self): + if self.args.with_changelog and not self.args.message: +diff --git a/tests/commands/test_clone.py b/tests/commands/test_clone.py +index f741864..6ef1300 100644 +--- a/tests/commands/test_clone.py ++++ b/tests/commands/test_clone.py +@@ -95,6 +95,50 @@ class CommandCloneTestCase(CommandTestCase): + + shutil.rmtree(altpath) + ++ def test_clone_anonymous_pre_push_hook(self): ++ self.make_new_git(self.module) ++ ++ altpath = tempfile.mkdtemp(prefix='rpkg-tests.') ++ ++ cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash, ++ self.lookaside_cgi, self.gitbaseurl, ++ self.anongiturl, self.branchre, self.kojiprofile, ++ self.build_client, self.user, self.dist, ++ self.target, self.quiet) ++ cmd.clone(self.module, anon=True, config_path=None) ++ ++ moduledir = os.path.join(self.path, self.module) ++ self.assertTrue(os.path.isfile(os.path.join(moduledir, '.git/hooks/pre-push'))) ++ ++ with open(os.path.join(moduledir, '.git/hooks/pre-push')) as git_hook_script: ++ content = git_hook_script.read() ++ pattern = '__main__.py pre-push-check "$local_sha"' ++ self.assertIn(pattern, content) ++ ++ shutil.rmtree(altpath) ++ ++ def test_clone_anonymous_pre_push_hook_config(self): ++ self.make_new_git(self.module) ++ ++ altpath = tempfile.mkdtemp(prefix='rpkg-tests.') ++ ++ cmd = pyrpkg.Commands(self.path, self.lookaside, self.lookasidehash, ++ self.lookaside_cgi, self.gitbaseurl, ++ self.anongiturl, self.branchre, self.kojiprofile, ++ self.build_client, self.user, self.dist, ++ self.target, self.quiet) ++ cmd.clone(self.module, anon=True, config_path="/home/conf/rhpkg.conf") ++ ++ moduledir = os.path.join(self.path, self.module) ++ self.assertTrue(os.path.isfile(os.path.join(moduledir, '.git/hooks/pre-push'))) ++ ++ with open(os.path.join(moduledir, '.git/hooks/pre-push')) as git_hook_script: ++ content = git_hook_script.read() ++ pattern = '__main__.py -C "/home/conf/rhpkg.conf" pre-push-check "$local_sha"' ++ self.assertIn(pattern, content) ++ ++ shutil.rmtree(altpath) ++ + def test_clone_anonymous_with_branch(self): + self.make_new_git(self.module, + branches=['rpkg-tests-1', 'rpkg-tests-2']) +-- +2.39.2 + diff --git a/0013-Fix-unittests-for-clone-and-pre-push-hook-script.patch b/0013-Fix-unittests-for-clone-and-pre-push-hook-script.patch new file mode 100644 index 0000000..991fb22 --- /dev/null +++ b/0013-Fix-unittests-for-clone-and-pre-push-hook-script.patch @@ -0,0 +1,49 @@ +From 1d82b7eaf98e695689a7dc10bd308030e3c13eea Mon Sep 17 00:00:00 2001 +From: Ondrej Nosek onosek@redhat.com +Date: Sat, 1 Apr 2023 01:34:34 +0200 +Subject: [PATCH] Fix unittests for `clone` and pre-push hook script + +Signed-off-by: Ondrej Nosek onosek@redhat.com +--- + tests/commands/test_clone.py | 8 ++++++-- + 1 file changed, 6 insertions(+), 2 deletions(-) + +diff --git a/tests/commands/test_clone.py b/tests/commands/test_clone.py +index 6ef1300..85fdfd1 100644 +--- a/tests/commands/test_clone.py ++++ b/tests/commands/test_clone.py +@@ -1,5 +1,6 @@ + import os + import shutil ++import sys + import tempfile + + import git +@@ -110,9 +111,10 @@ class CommandCloneTestCase(CommandTestCase): + moduledir = os.path.join(self.path, self.module) + self.assertTrue(os.path.isfile(os.path.join(moduledir, '.git/hooks/pre-push'))) + ++ clonned_by = os.path.basename(sys.argv[0]) + with open(os.path.join(moduledir, '.git/hooks/pre-push')) as git_hook_script: + content = git_hook_script.read() +- pattern = '__main__.py pre-push-check "$local_sha"' ++ pattern = '{0} pre-push-check "$local_sha"'.format(clonned_by) + self.assertIn(pattern, content) + + shutil.rmtree(altpath) +@@ -132,9 +134,11 @@ class CommandCloneTestCase(CommandTestCase): + moduledir = os.path.join(self.path, self.module) + self.assertTrue(os.path.isfile(os.path.join(moduledir, '.git/hooks/pre-push'))) + ++ clonned_by = os.path.basename(sys.argv[0]) + with open(os.path.join(moduledir, '.git/hooks/pre-push')) as git_hook_script: + content = git_hook_script.read() +- pattern = '__main__.py -C "/home/conf/rhpkg.conf" pre-push-check "$local_sha"' ++ pattern = '{0} -C "/home/conf/rhpkg.conf" pre-push-check ' \ ++ '"$local_sha"'.format(clonned_by) + self.assertIn(pattern, content) + + shutil.rmtree(altpath) +-- +2.39.2 + diff --git a/rpkg.spec b/rpkg.spec index 50604bf..290e345 100644 --- a/rpkg.spec +++ b/rpkg.spec @@ -1,6 +1,6 @@ Name: rpkg Version: 1.66 -Release: 4%{?dist} +Release: 5%{?dist}
Summary: Python library for interacting with rpm+git License: GPLv2+ and LGPLv2 @@ -40,6 +40,13 @@ Patch3: 0003-Remove-Environment-Markers-syntax.patch Patch4: 0004-Process-source-URLs-with-fragment-in-pre-push-hook.patch Patch5: 0005-container-build-update-signing-intent-help-for-OSBS-.patch Patch6: 0006-Do-not-generate-pre-push-hook-script-in-some-cases.patch +Patch7: 0007-More-robust-spec-file-presence-checking.patch +Patch8: 0008-Update-to-spec-file-presence-checking.patch +Patch9: 0009-Add-more-information-about-pre-push-hook.patch +Patch10: 0010-pre-push-check-have-to-use-spectool-with-define.patch +Patch11: 0011-A-HEAD-query-into-a-lookaside-cache.patch +Patch12: 0012-pre-push-hook-script-contains-a-user-s-config.patch +Patch13: 0013-Fix-unittests-for-clone-and-pre-push-hook-script.patch
%description Python library for interacting with rpm+git @@ -256,6 +263,15 @@ example_cli_dir=$RPM_BUILD_ROOT%{_datadir}/%{name}/examples/cli
%changelog +* Sat Apr 1 2023 Ondřej Nosek onosek@redhat.com - 1.66-5 +- Patch: Fix unittests for `clone` and pre-push hook script +- Patch: pre-push hook script contains a user's config +- Patch: A HEAD query into a lookaside cache +- Patch: `pre-push-check` have to use spectool with --define +- Patch: Add more information about pre-push hook +- Patch: Update to spec file presence checking +- Patch: More robust spec file presence checking + * Fri Mar 10 2023 Ondřej Nosek onosek@redhat.com - 1.66-4 - Patch: Do not generate pre-push hook script in some cases
https://src.fedoraproject.org/rpms/rpkg/c/bd87270301b52a4b0c99555aa070378a8c...