rpmautospec-plugin's git repo check is too strict

[dennisklein]

mock/mock/py/mockbuild/plugins/rpmautospec.py

Lines 61 to 62 in 4b3fb22

	distgit_git_dir = host_chroot_sources / ".git"
	if not distgit_git_dir.is_dir():

This check fails for git submodules and multiple git worktrees (in such cases the .git file may not be a directory). Since rpmautospec already depends on pygit2 I propose to improve the git repo check with pygit2.discover_repository() which already supports submodules and worktrees.

Test with python -c "import pygit2; import os; print(pygit2.discover_repository(os.getcwd()))"

Possible implementation:

if not pygit2.discover_repository(host_chroot_sources):

---

If pygit2 as a new dependency is not desired, one could fall back to the return code of git rev-parse --git-dir.

[praiskup]

@nphilipp fyi

[praiskup]

@sgallagher fyi

[praiskup]

@dennisklein thank you for reporting this!

[nphilipp]

@dennisklein good catch! In fact, we don’t want to have pygit2 as a dependency in this plugin – we changed it intentionally so that the bulk of the code is executed in the build root and outside has only minimal dependencies.

[nphilipp]

@praiskup I lean towards implementing this check in rpmautospec_core and adapting the mock plugin to use it. What do you think?

[nphilipp]

@dennisklein Your check is too lenient, though 😉 (but the error message in the plugin is misleading): it works anywhere in the worktree, but it should only continue there if at the root. I guess I’ll just check for the presence of .git and if it’s a plain file, check that it begins with gitdir: .

[dennisklein]

but it should only continue there if at the root

true, I didn't realize this aspect.

I guess I’ll just check for the presence of .git and if it’s a plain file, check that it begins with gitdir:

👍 Thank you!

[praiskup]

@praiskup I lean towards implementing this check in rpmautospec_core and adapting the mock plugin to use it. What do you think?

Not sure if the mock plugin needs an update? But sounds good.

[nphilipp]

I’ve looked into it more closely and it seems like just checking these couple files or directories doesn’t cover all the options how a git work tree can be defined. So to me the question is if we can live with not supporting these corner cases – which we could by delegating this step to git itself but then it would have to become a dependency (and I’m not sure if it’s worthwhile).

[dennisklein]

So to me the question is if we can live with not supporting these corner cases

To my current use case (submodules) it would be acceptable.

which we could by delegating this step to git itself

This sounds more future-proof as we would not be relying on implementation details of git.