search

iterative / scmrepo

SCM wrapper and fsspec filesystem for Git for use in DVC.
https://dvc.org
Apache License 2.0
21 stars 14 forks source link

scmrepo git driver hard codes hooks directory #111

Open liljenstolpe opened 2 years ago

liljenstolpe commented 2 years ago

I have an issue in iterative/dvc#7967, that appears is actually an issue with scmrepo.

In scmrepo, there is an assumption that hooks are in .git/hooks, and it is hardcoded as such (my guess is there might be other assumptions like that as well). That's all well and good, unless the repo happens to be cloned/integrated as a submodule. In that case, the '.git' directory is actually the parent repo's .git/modules/. Therefore, the hooks directory actually should be /.git/modules//hooks.

You can actually just use:

git rev-parse --git-path hooks

to find out whatever the hook directory is for a given repo (or any other path, btw). That saves you from having to build all of the logic depending on where things land.

Happy to propose some code, but this might be something that makes more sense to generalize

pmrowla commented 2 years ago

The issue here is that we cannot just directly call git rev-parse since we don't want to have a dependency on CLI git.

Basically what we really need is to implement the correct methods for getting GIT_DIR and GIT_COMMON_DIR in all of the scmrepo backends. Some of this functionality already exists in dulwich and libgit2/pygit2 but it gets more complicated when accounting for things like submodules and worktrees.

Then rather than the hard coded paths we would use the proper values for GIT_DIR in places like:

https://github.com/iterative/scmrepo/blob/708cd685a1bb737d87973625ece91ea301c95483/scmrepo/git/__init__.py#L111-L115

liljenstolpe commented 2 years ago

Greetings, I agree, I should have been more specific - make a programatic call the same way that git rev-parse does. :)