Fix GitHub actions on Windows and Mac OSX . (#767)

* Use PathLib for resolving paths.
* Use pathlib for .add as well, add more debugging info for hooks.
* Handle FileNotFoundError on Python 3.5.

.github/workflows/pythonpackage.yml

@@ -10,6 +10,16 @@ jobs:
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
         python-version: [3.5, 3.6, 3.7, 3.8, pypy3]
+        exclude:
+          # sqlite3 exit handling seems to get in the way
+          - os: macos-latest
+            python-version: pypy3
+          # doesn't support passing in bytestrings to os.scandir
+          - os: windows-latest
+            python-version: pypy3
+          # path encoding
+          - os: windows-latest
+            python-version: 3.5
       fail-fast: false
 
     steps:

dulwich/porcelain.py

@@ -64,6 +64,7 @@ from contextlib import (
 from io import BytesIO, RawIOBase
 import datetime
 import os
+from pathlib import Path
 import posixpath
 import shutil
 import stat
@@ -192,7 +193,7 @@ def open_repo_closing(path_or_repo):
     return closing(Repo(path_or_repo))
 
 
-def path_to_tree_path(repopath, path):
+def path_to_tree_path(repopath, path, tree_encoding=DEFAULT_ENCODING):
     """Convert a path to a path usable in an index, e.g. bytes and relative to
     the repository root.
 
@@ -201,16 +202,13 @@ def path_to_tree_path(repopath, path):
       path: A path, absolute or relative to the cwd
     Returns: A path formatted for use in e.g. an index
     """
-    if not isinstance(path, bytes):
-        path = os.fsencode(path)
-    if not isinstance(repopath, bytes):
-        repopath = os.fsencode(repopath)
-    treepath = os.path.relpath(path, repopath)
-    if treepath.startswith(b'..'):
-        raise ValueError('Path %r not in repo path (%r)' % (path, repopath))
-    if os.path.sep != '/':
-        treepath = treepath.replace(os.path.sep.encode('ascii'), b'/')
-    return treepath
+    path = Path(path).resolve()
+    repopath = Path(repopath).resolve()
+    relpath = path.relative_to(repopath)
+    if sys.platform == 'win32':
+        return str(relpath).replace(os.path.sep, '/').encode(tree_encoding)
+    else:
+        return bytes(relpath)
 
 
 def archive(repo, committish=None, outstream=default_bytes_out_stream,
@@ -396,17 +394,18 @@ def add(repo=".", paths=None):
     """
     ignored = set()
     with open_repo_closing(repo) as r:
+        repo_path = Path(r.path).resolve()
         ignore_manager = IgnoreFilterManager.from_repo(r)
         if not paths:
             paths = list(
-                get_untracked_paths(os.getcwd(), r.path, r.open_index()))
+                get_untracked_paths(
+                    str(Path(os.getcwd()).resolve()),
+                    str(repo_path), r.open_index()))
         relpaths = []
         if not isinstance(paths, list):
             paths = [paths]
         for p in paths:
-            relpath = os.path.relpath(p, r.path)
-            if relpath.startswith('..' + os.path.sep):
-                raise ValueError('path %r is not in repo' % relpath)
+            relpath = str(Path(p).resolve().relative_to(repo_path))
             # FIXME: Support patterns, directories.
             if ignore_manager.is_ignored(relpath):
                 ignored.add(relpath)

dulwich/tests/test_porcelain.py

@@ -385,7 +385,12 @@ class AddTests(PorcelainTestCase):
         cwd = os.getcwd()
         try:
             os.chdir(self.repo.path)
-            porcelain.add(self.repo.path)
+            self.assertEqual(
+                set(['foo', 'blah', 'adir', '.git']),
+                set(os.listdir('.')))
+            self.assertEqual(
+                (['foo', os.path.join('adir', 'afile')], set()),
+                porcelain.add(self.repo.path))
         finally:
             os.chdir(cwd)
 
@@ -449,7 +454,7 @@ class AddTests(PorcelainTestCase):
             porcelain.add, self.repo,
             paths=[os.path.join(self.test_dir, "foo")])
         self.assertRaises(
-            ValueError,
+            (ValueError, FileNotFoundError),
             porcelain.add, self.repo,
             paths=["../foo"])
         self.assertEqual([], list(self.repo.open_index()))
@@ -1736,11 +1741,20 @@ class DescribeTests(PorcelainTestCase):
                 porcelain.describe(self.repo.path))
 
 
-class HelperTests(PorcelainTestCase):
+class PathToTreeTests(PorcelainTestCase):
+
+    def setUp(self):
+        super(PathToTreeTests, self).setUp()
+        self.fp = os.path.join(self.test_dir, 'bar')
+        with open(self.fp, 'w') as f:
+            f.write('something')
+        oldcwd = os.getcwd()
+        self.addCleanup(os.chdir, oldcwd)
+        os.chdir(self.test_dir)
 
     def test_path_to_tree_path_base(self):
         self.assertEqual(
-            b'bar', porcelain.path_to_tree_path('/home/foo', '/home/foo/bar'))
+            b'bar', porcelain.path_to_tree_path(self.test_dir, self.fp))
         self.assertEqual(b'bar', porcelain.path_to_tree_path('.', './bar'))
         self.assertEqual(b'bar', porcelain.path_to_tree_path('.', 'bar'))
         cwd = os.getcwd()
@@ -1749,13 +1763,12 @@ class HelperTests(PorcelainTestCase):
         self.assertEqual(b'bar', porcelain.path_to_tree_path(cwd, 'bar'))
 
     def test_path_to_tree_path_syntax(self):
-        self.assertEqual(b'bar', porcelain.path_to_tree_path(b'.', './bar'))
-        self.assertEqual(b'bar', porcelain.path_to_tree_path('.', b'./bar'))
-        self.assertEqual(b'bar', porcelain.path_to_tree_path(b'.', b'./bar'))
+        self.assertEqual(b'bar', porcelain.path_to_tree_path('.', './bar'))
 
     def test_path_to_tree_path_error(self):
         with self.assertRaises(ValueError):
-            porcelain.path_to_tree_path('/home/foo/', '/home/bar/baz')
+            with tempfile.TemporaryDirectory() as od:
+                porcelain.path_to_tree_path(od, self.fp)
 
     def test_path_to_tree_path_rel(self):
         cwd = os.getcwd()
@@ -1763,6 +1776,8 @@ class HelperTests(PorcelainTestCase):
         os.mkdir(os.path.join(self.repo.path, 'foo/bar'))
         try:
             os.chdir(os.path.join(self.repo.path, 'foo/bar'))
+            with open('baz', 'w') as f:
+                f.write('contents')
             self.assertEqual(b'bar/baz', porcelain.path_to_tree_path(
                 '..', 'baz'))
             self.assertEqual(b'bar/baz', porcelain.path_to_tree_path(