public inbox for git-commits@fedoraproject.org
help / color / mirror / Atom feed
To: git-commits@fedoraproject.org
Subject: [rpms/rpkg] 1.70-1: A few patches mostly for `pre-push-check`
Date: Mon, 10 Aug 2026 21:44:33 GMT	[thread overview]
Message-ID: <178639827370.1.6588226425333558803.rpms-rpkg-bd87270301b5@fedoraproject.org> (raw)

            A new commit has been pushed.

            Repo   : rpms/rpkg
            Branch : 1.70-1
            Commit : bd87270301b52a4b0c99555aa070378a8c6cc736
            Author : Ondřej Nosek <onosek@redhat.com>
            Date   : 2023-03-31T23:54:46+00:00
            Stats  : +885/-1 in 8 file(s)
            URL    : https://src.fedoraproject.org/rpms/rpkg/c/bd87270301b52a4b0c99555aa070378a8c6cc736?branch=1.70-1

            Log:
            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
 

             reply	other threads:[~2026-08-10 21:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-10 21:44  [this message]
2026-08-10 21:44 [rpms/rpkg] 1.70-1: A few patches mostly for `pre-push-check` 

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=178639827370.1.6588226425333558803.rpms-rpkg-bd87270301b5@fedoraproject.org \
    --to=git-commits@fedoraproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox