Repository: pypa/setuptools_scm
Pull request #598 (2021-08-05 12:40:11)

Adding search_parent_directories to recursively search parent directory for git-root
Fixes https://github.com/pypa/setuptools_scm/issues/591

tdegeus: @RonnyPfannschmidt Concerning your second comment: not entirely sure that I understand what you are doing, could you give some pointers based on the edit?
tdegeus: So indeed, as the CI shows, it does not seem that `git.parse` is called (or at least not in time: form the code I don't fully get how `git.parse` is called...).
RonnyPfannschmidt: @tdegeus it seems it just returns none,

the helper in the _do_parse function tries all parse functions

i can dig into this on the weeken at earliest

tdegeus: I did some debugging @RonnyPfannschmidt . The problem is that it is not the result of `git.parse` that will determine if it will be used. Rather, there is a naive try here
https://github.com/pypa/setuptools_scm/blob/dfb4bea08dc03dcd560caf7e00b6b8aa837b2a33/src/setuptools_scm/discover.py#L8
with `path` equal to the initially specified (absolute) root. 

## Example

Consider that I have my package here
```
/path/to/mypackage/.git
```
and that I'm running from
```
/path/to/mypackage/src
```
then 
https://github.com/pypa/setuptools_scm/blob/dfb4bea08dc03dcd560caf7e00b6b8aa837b2a33/src/setuptools_scm/discover.py#L8
decides not to try parsing with *git* because 
```
/path/to/mypackage/src/.git
```
does not exist.

So even though `git.parse` does work and will recognise the right working directory, it is simply not considered. 

I think it is better that you comment how to improve this
tdegeus: Here is a proposed work-around, let me know what you think!
RonnyPfannschmidt: :facepalm: my appologoes, i completely forgot about that sanity check, which is not sane in the face of upward search
RonnyPfannschmidt: all on all considered, the best solution given the necessary workaround, might actually be to just walk the parent directories in that function 

then the search for parents would work for all scms and for git as well by default

what do you think about hat
tdegeus: I think that would be fine! Do you propose to keep walking the parent directories still as an option ?
RonnyPfannschmidt: @tdegeus yes, it should be a opt in for scripts - a future release may decide to enable it by default for the get_version function

right now people use it in pre-configured situations where always enabling it may cause a breakage/wrong result

so the path forwards is have it a opt in, and maybe later have it as a opt out for script usage
RonnyPfannschmidt: @tdegeus btw, i recommend installing pre-commit and useing it for this repo

i observe you having to fight the linting in the CI
 pre-commit is a tool that helps you sort this out before you even commit
tdegeus: > @tdegeus btw, i recommend installing pre-commit and useing it for this repo
> 
> i observe you having to fight the linting in the CI
> pre-commit is a tool that helps you sort this out before you even commit

I will look into it ;)
tdegeus: @RonnyPfannschmidt I implemented the search for the parent directly in `discovery`. What might make things nicer is to store the path in `config` on not re-search. I don't think `Configure` has an API to that end. Or am I looking over it?
tdegeus: @RonnyPfannschmidt This should do it, ready for your review.

(I noticed in the CI `188 passed, 1 skipped, 1 xfailed, 1 xpassed` isn't that worrisome?)
tdegeus: P.S. the `walk_roots` could be simpler, and I think that that is what you have in mind. This is what I was thinking about
```python
def walk_potential_roots(root, search_parents = True):

    if not search_parents:
        yield root
        return

    while True:
        root, tail = os.path.split(root)
        if not tail:
            return
        yield root
```
but one needs the unsplitted `root` on the first iteration with `search_parents = True`, but that I couldn't think of how to do. Maybe you have a better idea? 
RonnyPfannschmidt: Always yield root, assign root to tail before the loop and then have a while tail loop in a if block 
tdegeus: Thanks @RonnyPfannschmidt  that was indeed what needed to be done.

I couldn't help myself to leave an elegant code ;)
tdegeus: BTW I exclusively use git so I don't feel super confident to do it, but the other `parse` functions could be extended as well, at least using `config.parent`.
RonnyPfannschmidt: @tdegeus thats ok  do you want to re-base a little to "beautify" the commits or should i merge

Patch:
diff --git a/src/setuptools_scm/__init__.py b/src/setuptools_scm/__init__.py
index e874aaa..74bc772 100644
--- a/src/setuptools_scm/__init__.py
+++ b/src/setuptools_scm/__init__.py
@@ -67,7 +67,8 @@ def _version_from_entrypoints(config, fallback=False):
     else:
         entrypoint = "setuptools_scm.parse_scm"
         root = config.absolute_root
-    for ep in iter_matching_entrypoints(root, entrypoint):
+
+    for ep in iter_matching_entrypoints(root, entrypoint, config):
         version = _call_entrypoint_fn(root, config, ep.load())
 
         if version:
@@ -162,6 +163,7 @@ def get_version(
     dist_name=None,
     version_cls=None,
     normalize=True,
+    search_parent_directories=False,
 ):
     """
     If supplied, relative_to should be a file from which root may
diff --git a/src/setuptools_scm/config.py b/src/setuptools_scm/config.py
index 173c7e8..7ecb1c6 100644
--- a/src/setuptools_scm/config.py
+++ b/src/setuptools_scm/config.py
@@ -75,6 +75,7 @@ class Configuration:
         dist_name=None,
         version_cls=None,
         normalize=True,
+        search_parent_directories=False,
     ):
         # TODO:
         self._relative_to = relative_to
@@ -92,6 +93,8 @@ class Configuration:
         self.tag_regex = tag_regex
         self.git_describe_command = git_describe_command
         self.dist_name = dist_name
+        self.search_parent_directories = search_parent_directories
+        self.parent = None
 
         if not normalize:
             # `normalize = False` means `version_cls = NonNormalizedVersion`
diff --git a/src/setuptools_scm/discover.py b/src/setuptools_scm/discover.py
index 9b9a509..afddc92 100644
--- a/src/setuptools_scm/discover.py
+++ b/src/setuptools_scm/discover.py
@@ -2,11 +2,54 @@ import os
 from .utils import trace, iter_entry_points
 
 
-def iter_matching_entrypoints(path, entrypoint):
-    trace("looking for ep", entrypoint, path)
-    for ep in iter_entry_points(entrypoint):
-        if os.path.exists(os.path.join(path, ep.name)):
-            if os.path.isabs(ep.name):
-                trace("ignoring bad ep", ep)
-            trace("found ep", ep)
-            yield ep
+def walk_potential_roots(root, search_parents=True):
+    """
+    Iterate though a path and each of its parents.
+    :param root: File path.
+    :param search_parents: If ``False`` the parents are not considered.
+    """
+
+    if not search_parents:
+        yield root
+        return
+
+    tail = root
+
+    while tail:
+        yield root
+        root, tail = os.path.split(root)
+
+
+def match_entrypoint(root, name):
+    """
+    Consider a ``root`` as entry-point.
+    :param root: File path.
+    :param name: Subdirectory name.
+    :return: ``True`` if a subdirectory ``name`` exits in ``root``.
+    """
+
+    if os.path.exists(os.path.join(root, name)):
+        if not os.path.isabs(name):
+            return True
+        trace("ignoring bad ep", name)
+
+    return False
+
+
+def iter_matching_entrypoints(root, entrypoint, config):
+    """
+    Consider different entry-points in ``root`` and optionally its parents.
+    :param root: File path.
+    :param entrypoint: Entry-point to consider.
+    :param config: Configuration,
+        read ``search_parent_directories``, write found parent to ``parent``.
+    """
+
+    trace("looking for ep", entrypoint, root)
+
+    for wd in walk_potential_roots(root, config.search_parent_directories):
+        for ep in iter_entry_points(entrypoint):
+            if match_entrypoint(wd, ep.name):
+                trace("found ep", ep, "in", wd)
+                config.parent = wd
+                yield ep
diff --git a/src/setuptools_scm/git.py b/src/setuptools_scm/git.py
index 7089a97..3721b07 100644
--- a/src/setuptools_scm/git.py
+++ b/src/setuptools_scm/git.py
@@ -104,6 +104,27 @@ def fail_on_shallow(wd):
         )
 
 
+def get_working_directory(config):
+    """
+    Return the working directory (``GitWorkdir``).
+    """
+
+    if config.parent:
+        return GitWorkdir.from_potential_worktree(config.parent)
+
+    if config.search_parent_directories:
+        return search_parent(config.absolute_root)
+
+    wd = GitWorkdir.from_potential_worktree(config.absolute_root)
+
+    if wd is not None:
+        return wd
+
+    from .hg_git import GitWorkdirHgClient
+
+    return GitWorkdirHgClient.from_potential_worktree(config.absolute_root)
+
+
 def parse(root, describe_command=None, pre_parse=warn_on_shallow, config=None):
     """
     :param pre_parse: experimental pre_parse action, may change at any time
@@ -111,13 +132,11 @@ def parse(root, describe_command=None, pre_parse=warn_on_shallow, config=None):
     if not config:
         config = Configuration(root=root)
 
-    wd = GitWorkdir.from_potential_worktree(config.absolute_root)
-    if wd is None:
-        from .hg_git import GitWorkdirHgClient
+    wd = get_working_directory(config)
 
-        wd = GitWorkdirHgClient.from_potential_worktree(config.absolute_root)
     if wd is None:
         return
+
     if pre_parse:
         pre_parse(wd)
 
@@ -171,3 +190,30 @@ def _git_parse_describe(describe_output):
     tag, number, node = describe_output.rsplit("-", 2)
     number = int(number)
     return tag, number, node, dirty
+
+
+def search_parent(dirname):
+    """
+    Walk up the path to find the `.git` directory.
+    :param dirname: Directory from which to start searching.
+    """
+
+    # Code based on:
+    # https://github.com/gitpython-developers/GitPython/blob/main/git/repo/base.py
+
+    curpath = os.path.abspath(dirname)
+
+    while curpath:
+
+        try:
+            wd = GitWorkdir.from_potential_worktree(curpath)
+        except Exception:
+            wd = None
+
+        if wd is not None:
+            return wd
+
+        curpath, tail = os.path.split(curpath)
+
+        if not tail:
+            return None


Test patch:
diff --git a/testing/test_git.py b/testing/test_git.py
index 46da3f7..1928990 100644
--- a/testing/test_git.py
+++ b/testing/test_git.py
@@ -52,6 +52,19 @@ setup(use_scm_version={"root": "../..",
     assert res == "0.1.dev0"
 
 
+def test_root_search_parent_directories(tmpdir, wd, monkeypatch):
+    monkeypatch.delenv("SETUPTOOLS_SCM_DEBUG")
+    p = wd.cwd.joinpath("sub/package")
+    p.mkdir(parents=True)
+    p.joinpath("setup.py").write_text(
+        """from setuptools import setup
+setup(use_scm_version={"search_parent_directories": True})
+"""
+    )
+    res = do((sys.executable, "setup.py", "--version"), p)
+    assert res == "0.1.dev0"
+
+
 def test_git_gone(wd, monkeypatch):
     monkeypatch.setenv("PATH", str(wd.cwd / "not-existing"))
     with pytest.raises(EnvironmentError, match="'git' was not found"):