Unlike the first attempt (#643), this one executes functionality in Python, unlike the original bash solution.
fedpkg pre-push-check, which is executed in the git hook script can be even executed separately.

Why run the check for each pushed branch if neither the branch name nor the commit sha? It will repeatedly check whatever is currently checked out.

Maybe the pre-push-check should receive local_sha as argument, and use git to verify file contents at that revision?

The use case for that is that it's possible to check out a branch locally, but push a different branch. If I do git switch f37 and git push origin rawhide, I would expect the check to still verify the commit being pushed, not what I have checked out.

I don't know if this assumption is safe. Take fedora-release as an example. No source is in lookaside.

Could the check instead verify that each listed source file is either listed in sources, or committed in git?

This might make sense for patches too.

Or see python-rpm-generators -- neither of the RPM sources is in the lookaside cache, but there is a tarball in a lookaside cache for the CI tests.

Thanks for your inputs. The new version check takes one argument with a git reference.
Also checks whether a source file (or patch) is either tracked or listed in 'sources'.
Still, there is still one assumption used: specfile name is determined in the local branch instead of a pushed branch (hopefully main specfile name is rarely changed). Do you think it is worth making it 100% for the price duplicating the functionality?

Still, there is still one assumption used: specfile name is determined in the local branch instead of a pushed branch (hopefully main specfile name is rarely changed).

You mean the spec file name is determined from whatever is currently checked out, which may not be what is pushed? That's not a problem in my opinion. It should be so rare that disabling the check should be sufficient in such case.

That might be a good improvement: if the hook denies the push, the output could suggest that it can be disabled by passing --no-verify option to push command (xpkg push should learn to pass that option to git).

How about prefixing the unused variables with underscore? ShellCheck complains about them, but I think they have value as documentation for the hook interface.

ShellCheck also wants read -r to be used to not mangle backslashes (which should not really be a problem here).

The $local_sha in the the pre-push-check call should be quoted.

The exit in the loop will cause only the first branch to be checked. I think it's fine to abort on the first problematic branch, but all branches should be checked.

@lsedlar: Actually I have just remembered the case that might not be so rare. When rhpkg operates from the master branch, which is empty.
Determining the specfile is a functionality, that was changed a few times in history and I didn't want to duplicate it.

Why tuple? I would expect set.

Why tuple? I would expect set.

Can be there some duplicities?

Actually I have just remembered the case that might not be so rare. When rhpkg operates from the master branch, which is empty.

True, that might happen. I can easily imagine making changes in multiple branches and then pushing them all in one command or while the last modified branch is checked out. But why would someone check out unused master branch?

I still think it's okay to bail out in such case. The functionality is supposed to be a convenience for users. They can explicitly opt out, or they can do weird stuff that makes it not work correctly.

I completely agree with not duplicating loading spec file from work tree and from a specific commit.

Why tuple? I would expect set.

Can be there some duplicities?

I don't think so. The order is not significant either.

recent updates:

• ShellCheck fixes
• bash script exits with overall return code after checking all branches
• --no-verify argument to the "xpkg push" command
• show the hint about the possibility of bypassing the pre-push check when the check fails
• minor fix in regular expression for extracting sources and patches
• tuple-->set; more suitable structure for items
• use sys.exit(ret_code) instead of return to affect '$?' in bash

additional functionality:
hook script executes checking with: " pre-push-check"
is hard-coded in the hook script during the clone of the repository. When a user pushes the changes manually after is missing, it would fail. Therefore there is a check for the presence of the in the system.

Unit tests were added.