How do you check the sources availability in look-a-side cache? You try to build a srpm on every push?

fedpkg sources was my first thought, on the second though, that is probably not enough, so fedpkg srpm is the way to go.

Actually, I think something like fedpkg srpm would be very nice check anyway. In the rpms namespace context, I can't imagine any case where it would be reasonable to push something into gist-git without making sure it is possible to create SRPM out of the commit.

fedpkg prep would be faster to run.

prep might need additional deps.

fedpkg sources was my first thought, on the second though, that is probably not enough

This will catch only situations where the source file is updated with a new hash, but the sources have not actually been uploaded. If the sources file is not updated at all, this will proceed with downloading the outdated ones.

fedpkg prep would be faster to run.

Or scratch build ;) but for the start, I'd be glad if my initial issue, that sources missing from look-a-side cache was covered. Because ultimately, all the issues would be solved if commit to master branch is disabled and only PRs with CI enabled are allowed.

Maybe the check could be as simple as spectool -f foo.spec output compared to sources file, because this is the more likely scenario then having updated sources file without uploaded sources.

Maybe the check could be as simple as spectool -f foo.spec output compared to sources file, because this is the more likely scenario then having updated sources file without uploaded sources.

@vondruch has a really good idea here; we don't have to make such a hook perfect as long as we can catch the 90% case.

For example: spectool -l libmodulemd.spec yields:

Source0: https://github.com/fedora-modularity/libmodulemd/releases/download/libmodulemd-2.12.0/modulemd-2.12.0.tar.xz

If we parse that down to the filename, we can then just to grep -q modulemd-2.12.0.tar.xz sources. If that returns non-zero, we know that this filename was not properly added to the sources file. That alone would solve the majority of cases of forgotten source uploads.

If we have a situation where someone has submitted the sources file but has not uploaded the content to the lookaside cache (for example, merging the rawhide branch of libmodulemd to the epel7 branch of libmodulemd2), this is also very simple to detect. The URL in the lookaside cache is well-known and can be derived from the contents of the sources file. So all we really need to do on the git hook side would be to open an HTTP GET request to that path and check the response header (without doing a full download).

These solutions wouldn't catch every possible edge-case, but they would likely catch the overwhelming majority.

I'm pretty leary security wise having the git server parse spec files. ;( Seems like it would be a recipe for security doom. Or are you suggesting a local hook?

Could we add more checks in fedpkg? or do people usually hit this with bare git commands?

I like the idea of catching this... :)

Or do people usually hit this with bare git commands?

I think that is pretty common. I use git push almost exclusively.
But maybe we could do a local hook, but not in fedpkg, but in .git/pre-push?

I also push with git push but I clone with fedpkg clone so if fedpkg added a local pre-push hook that would be great.

As well as catching missing sources, I'd like the hook to catch missing patches. It's fairly common for somebody to add PatchN: and %patchN to the spec file, but forget to git add the patch. A local hook to check that needs to be a bit smarter about what it checks, because the patch file will exist locally.

at the end patches and sources are all the same: they either need to be committed or in the lookaside cache

The local pre-push hook sounds good.

To bikeshed even further: Let's make it pre-commit. Commits should be self-contained.

Ideally, but it's harmless to have broken, partial commits in a local branch. It's only a problem when you push that to src.fedoraproject.org. I don't see a good reason to prevent people committing whatever junk they want, as long as they don't push it in that state.

(We might even want to allow pushing broken commits to personal forks and to other remotes, with a warning, and only reject when src.fedoraproject.org/rpms/$name is being updated)

From reading this ticket it seems that the consensus is a .git/pre-push hook on the local repository. That makes this ticket a good RFE for fedpkg (or possibly rpkg?), so I would suggest transferring it there to start the discussion with the fedpkg maintainers and see if they would drive or accept this patch.

https://pagure.io/fedpkg/issue/491

Solution based on your suggestions:
https://pagure.io/rpkg/pull-request/647

Commit cf95ca22 relates to this ticket