Repository: pypa/setuptools_scm (python)

Issue:
git file finder: follow symlinks
Imagine the following directory structure where setup.py is in a subdirectory in the git repo, and contains a symlink to a parent directory.

```
package/module.py
setup/setup.py
setup/package -> ../package
```

Currently, the git file finder will ignore the symlinked directory as it does a `git ls-files <setup root>`.

Such a directory structure is useful in situations where one does not have freedom to arrange a canonical directory structure in the repo.

Would you review a PR that makes the git file finder follow symlinks (of course restricting links to files/directories that are themselves part of the same git repo).

Pull request:
New git file finder that support symlinks.
Closes #245 

Patch:
diff --git a/setup.py b/setup.py
index 7fc33f3..2b93690 100644
--- a/setup.py
+++ b/setup.py
@@ -82,7 +82,7 @@ arguments = dict(
 
         [setuptools_scm.files_command]
         .hg = setuptools_scm.hg:FILES_COMMAND
-        .git = setuptools_scm.git:list_files_in_archive
+        .git = setuptools_scm.git_file_finder:find_files
 
         [setuptools_scm.version_scheme]
         guess-next-dev = setuptools_scm.version:guess_next_dev_version
diff --git a/setuptools_scm/git.py b/setuptools_scm/git.py
index 6f35e78..df31358 100644
--- a/setuptools_scm/git.py
+++ b/setuptools_scm/git.py
@@ -2,8 +2,6 @@ from .utils import do_ex, trace, has_command
 from .version import meta
 
 from os.path import isfile, join
-import subprocess
-import tarfile
 import warnings
 
 
@@ -128,13 +126,3 @@ def parse(root, describe_command=DEFAULT_DESCRIBE, pre_parse=warn_on_shallow):
         return meta(tag, distance=number, node=node, dirty=dirty, branch=branch)
     else:
         return meta(tag, node=node, dirty=dirty, branch=branch)
-
-
-def list_files_in_archive(path):
-    """List the files that 'git archive' generates.
-    """
-    cmd = ['git', 'archive', 'HEAD']
-    proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, cwd=path)
-    tf = tarfile.open(fileobj=proc.stdout, mode='r|*')
-    return [member.name for member in tf.getmembers()
-            if member.type != tarfile.DIRTYPE]
diff --git a/setuptools_scm/git_file_finder.py b/setuptools_scm/git_file_finder.py
new file mode 100644
index 0000000..2c978d3
--- /dev/null
+++ b/setuptools_scm/git_file_finder.py
@@ -0,0 +1,65 @@
+import os
+import subprocess
+import tarfile
+
+
+def _git_toplevel(path):
+    try:
+        out = subprocess.check_output([
+            'git', 'rev-parse', '--show-toplevel',
+        ], cwd=(path or '.'), universal_newlines=True)
+        return os.path.normcase(os.path.realpath(out.strip()))
+    except subprocess.CalledProcessError:
+        # git returned error, we are not in a git repo
+        return None
+    except OSError:
+        # git command not found, probably
+        return None
+
+
+def _git_ls_files_and_dirs(toplevel):
+    # use git archive instead of git ls-file to honor
+    # export-ignore git attribute
+    cmd = ['git', 'archive', '--prefix', toplevel + os.path.sep, 'HEAD']
+    proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, cwd=toplevel)
+    tf = tarfile.open(fileobj=proc.stdout, mode='r|*')
+    git_files = set()
+    git_dirs = set([toplevel])
+    for member in tf.getmembers():
+        name = os.path.normcase(member.name).replace('/', os.path.sep)
+        if member.type == tarfile.DIRTYPE:
+            git_dirs.add(name)
+        else:
+            git_files.add(name)
+    return git_files, git_dirs
+
+
+def find_files(path=''):
+    """ setuptools compatible git file finder that follows symlinks
+
+    Spec here: http://setuptools.readthedocs.io/en/latest/setuptools.html#\
+        adding-support-for-revision-control-systems
+    """
+    toplevel = _git_toplevel(path)
+    if not toplevel:
+        return []
+    git_files, git_dirs = _git_ls_files_and_dirs(toplevel)
+    realpath = os.path.normcase(os.path.realpath(path))
+    assert realpath.startswith(toplevel)
+    assert realpath in git_dirs
+    seen = set()
+    res = []
+    for dirpath, dirnames, filenames in os.walk(realpath, followlinks=True):
+        # dirpath with symlinks resolved
+        realdirpath = os.path.normcase(os.path.realpath(dirpath))
+        if realdirpath not in git_dirs or realdirpath in seen:
+            dirnames[:] = []
+            continue
+        for filename in filenames:
+            # dirpath + filename with symlinks preserved
+            fullfilename = os.path.join(dirpath, filename)
+            if os.path.normcase(os.path.realpath(fullfilename)) in git_files:
+                res.append(
+                    os.path.join(path, os.path.relpath(fullfilename, path)))
+        seen.add(realdirpath)
+    return res


Test patch:
diff --git a/testing/conftest.py b/testing/conftest.py
index 29e129c..6a155cd 100644
--- a/testing/conftest.py
+++ b/testing/conftest.py
@@ -42,6 +42,10 @@ class Wd(object):
         else:
             return given_reason
 
+    def add_and_commit(self, reason=None):
+        self(self.add_command)
+        self.commit(reason)
+
     def commit(self, reason=None):
         reason = self._reason(reason)
         self(self.commit_command, reason=reason)
diff --git a/testing/test_git.py b/testing/test_git.py
index 5762452..6b165e2 100644
--- a/testing/test_git.py
+++ b/testing/test_git.py
@@ -3,6 +3,7 @@ from setuptools_scm.utils import do
 from setuptools_scm import git
 import pytest
 from datetime import date
+from os.path import join as opj
 
 
 @pytest.fixture
@@ -123,7 +124,8 @@ def test_git_archive_export_ignore(wd):
              '/test1.txt -export-ignore\n/test2.txt export-ignore')
     wd('git add test1.txt test2.txt')
     wd.commit()
-    assert integration.find_files(str(wd.cwd)) == ['test1.txt']
+    with wd.cwd.as_cwd():
+        assert integration.find_files('.') == [opj('.', 'test1.txt')]
 
 
 @pytest.mark.issue(228)
@@ -132,7 +134,8 @@ def test_git_archive_subdirectory(wd):
     wd.write('foobar/test1.txt', 'test')
     wd('git add foobar')
     wd.commit()
-    assert integration.find_files(str(wd.cwd)) == ['foobar/test1.txt']
+    with wd.cwd.as_cwd():
+        assert integration.find_files('.') == [opj('.', 'foobar', 'test1.txt')]
 
 
 def test_git_feature_branch_increments_major(wd):
diff --git a/testing/test_git_file_finder.py b/testing/test_git_file_finder.py
new file mode 100644
index 0000000..fb7d6f1
--- /dev/null
+++ b/testing/test_git_file_finder.py
@@ -0,0 +1,133 @@
+import os
+import sys
+
+import pytest
+
+from setuptools_scm.git_file_finder import find_files
+
+
+@pytest.fixture
+def inwd(wd):
+    wd('git init')
+    wd('git config user.email test@example.com')
+    wd('git config user.name "a test"')
+    wd.add_command = 'git add .'
+    wd.commit_command = 'git commit -m test-{reason}'
+    (wd.cwd / 'file1').ensure(file=True)
+    adir = (wd.cwd / 'adir').ensure(dir=True)
+    (adir / 'filea').ensure(file=True)
+    bdir = (wd.cwd / 'bdir').ensure(dir=True)
+    (bdir / 'fileb').ensure(file=True)
+    wd.add_and_commit()
+    with wd.cwd.as_cwd():
+        yield wd
+
+
+def _sep(paths):
+    return {
+        path.replace('/', os.path.sep)
+        for path in paths
+    }
+
+
+def test_basic(inwd):
+    assert set(find_files()) == _sep({
+        'file1',
+        'adir/filea',
+        'bdir/fileb',
+    })
+    assert set(find_files('.')) == _sep({
+        './file1',
+        './adir/filea',
+        './bdir/fileb',
+    })
+    assert set(find_files('adir')) == _sep({
+        'adir/filea',
+    })
+
+
+def test_case(inwd):
+    (inwd.cwd / 'CamelFile').ensure(file=True)
+    (inwd.cwd / 'file2').ensure(file=True)
+    inwd.add_and_commit()
+    assert set(find_files()) == _sep({
+        'CamelFile',
+        'file2',
+        'file1',
+        'adir/filea',
+        'bdir/fileb',
+    })
+
+
+@pytest.mark.skipif(sys.platform == 'win32',
+                    reason="symlinks to dir not supported")
+def test_symlink_dir(inwd):
+    (inwd.cwd / 'adir' / 'bdirlink').mksymlinkto('../bdir')
+    inwd.add_and_commit()
+    assert set(find_files('adir')) == _sep({
+        'adir/filea',
+        'adir/bdirlink/fileb',
+    })
+
+
+@pytest.mark.skipif(sys.platform == 'win32',
+                    reason="symlinks to files not supported on windows")
+def test_symlink_file(inwd):
+    (inwd.cwd / 'adir' / 'file1link').mksymlinkto('../file1')
+    inwd.add_and_commit()
+    assert set(find_files('adir')) == _sep({
+        'adir/filea',
+        'adir/file1link',
+    })
+
+
+@pytest.mark.skipif(sys.platform == 'win32',
+                    reason="symlinks to dir not supported")
+def test_symlink_loop(inwd):
+    (inwd.cwd / 'adir' / 'loop').mksymlinkto('../adir')
+    inwd.add_and_commit()
+    assert set(find_files('adir')) == _sep({
+        'adir/filea',
+    })
+
+
+@pytest.mark.skipif(sys.platform == 'win32',
+                    reason="symlinks to dir not supported")
+def test_symlink_dir_out_of_git(inwd):
+    (inwd.cwd / 'adir' / 'outsidedirlink').\
+        mksymlinkto(os.path.join(__file__, '..'))
+    inwd.add_and_commit()
+    assert set(find_files('adir')) == _sep({
+        'adir/filea',
+    })
+
+
+@pytest.mark.skipif(sys.platform == 'win32',
+                    reason="symlinks to files not supported on windows")
+def test_symlink_file_out_of_git(inwd):
+    (inwd.cwd / 'adir' / 'outsidefilelink').mksymlinkto(__file__)
+    inwd.add_and_commit()
+    assert set(find_files('adir')) == _sep({
+        'adir/filea',
+    })
+
+
+def test_empty_root(inwd):
+    subdir = inwd.cwd / 'cdir' / 'subdir'
+    subdir.ensure(dir=True)
+    (subdir / 'filec').ensure(file=True)
+    inwd.add_and_commit()
+    assert set(find_files('cdir')) == _sep({
+        'cdir/subdir/filec',
+    })
+
+
+def test_empty_subdir(inwd):
+    subdir = inwd.cwd / 'adir' / 'emptysubdir' / 'subdir'
+    subdir.ensure(dir=True)
+    (subdir / 'xfile').ensure(file=True)
+    inwd.add_and_commit()
+    assert set(find_files('adir')) == _sep({
+        'adir/filea',
+        'adir/emptysubdir/subdir/xfile',
+    })
diff --git a/testing/test_regressions.py b/testing/test_regressions.py
index 7abd659..8e4f396 100644
--- a/testing/test_regressions.py
+++ b/testing/test_regressions.py
@@ -53,8 +53,8 @@ def test_pip_egg_info(tmpdir, monkeypatch):
 def test_pip_download(tmpdir, monkeypatch):
     monkeypatch.chdir(tmpdir)
     subprocess.check_call([
-        sys.executable, '-c',
-        'import pip;pip.main()', 'download', 'lz4==0.9.0',
+        sys.executable, '-m',
+        'pip', 'download', 'lz4==0.9.0',
     ])