Import windows fixes from a1s

.github/workflows/pythonpackage.yml

@@ -9,7 +9,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        os: [ubuntu-latest]
+        os: [ubuntu-latest, windows-latest]
         python-version: [3.6, 3.7, 3.8]
         experimental: [false]
 # See https://github.com/actions/toolkit/issues/399
@@ -33,7 +33,11 @@ jobs:
         run: |
           python -m pip install --upgrade pip
           pip install -U pip setuptools
-          pip install -U pip coverage codecov fastbencode flake8 testtools paramiko fastimport configobj cython testscenarios six docutils $TEST_REQUIRE sphinx sphinx_epytext launchpadlib patiencediff pyinotify git+https://github.com/dulwich/dulwich
+          pip install -U pip coverage codecov fastbencode flake8 testtools paramiko fastimport configobj cython testscenarios six docutils $TEST_REQUIRE sphinx sphinx_epytext launchpadlib patiencediff git+https://github.com/dulwich/dulwich
+      - name: Dependencies (Linux)
+        run: |
+          pip install -U pyinotify
+        if: "matrix.os == 'ubuntu-latest'"
       - name: Build docs
         run: |
           make docs PYTHON=python

breezy/_walkdirs_win32.pyx

@@ -58,14 +58,11 @@ cdef extern from "python-compat.h":
 
     int GetLastError()
 
-    # Wide character functions
-    DWORD wcslen(WCHAR *)
-
 
 cdef extern from "Python.h":
-    WCHAR *PyUnicode_AS_UNICODE(object)
-    Py_ssize_t PyUnicode_GET_SIZE(object)
-    object PyUnicode_FromUnicode(WCHAR *, Py_ssize_t)
+    WCHAR *PyUnicode_AsWideCharString(object, Py_ssize_t *size)
+    Py_ssize_t PyUnicode_GET_LENGTH(object)
+    object PyUnicode_FromWideChar(WCHAR *, Py_ssize_t)
     int PyList_Append(object, object) except -1
     object PyUnicode_AsUTF8String(object)
 
@@ -120,12 +117,6 @@ cdef class _Win32Stat:
                      self.st_mtime, self.st_ctime))
 
 
-cdef object _get_name(WIN32_FIND_DATAW *data):
-    """Extract the Unicode name for this file/dir."""
-    return PyUnicode_FromUnicode(data.cFileName,
-                                 wcslen(data.cFileName))
-
-
 cdef int _get_mode_bits(WIN32_FIND_DATAW *data): # cannot_raise
     cdef int mode_bits
 
@@ -213,25 +204,36 @@ cdef class Win32ReadDir:
     def read_dir(self, prefix, top):
         """Win32 implementation of DirReader.read_dir.
 
+        :param prefix: A utf8 prefix to be preprended to the path basenames.
+        :param top: A Unicode or UTF8-encoded path to read.
+
+        :return: A list of the directories contents. Each item contains:
+            (utf8_relpath, utf8_name, kind, lstatvalue, unicode_abspath)
+
         :seealso: DirReader.read_dir
+
         """
         cdef WIN32_FIND_DATAW search_data
         cdef HANDLE hFindFile
         cdef int last_err
         cdef WCHAR *query
         cdef int result
+        cdef Py_ssize_t length
 
+        global osutils
+        if osutils is None:
+            from . import osutils
         if prefix:
-            relprefix = prefix + '/'
+            relprefix = osutils.safe_utf8(prefix) + b'/'
         else:
-            relprefix = ''
-        top_slash = top + '/'
+            relprefix = b''
+        top_slash = osutils.safe_unicode(top) + '/'
 
         top_star = top_slash + '*'
 
         dirblock = []
 
-        query = PyUnicode_AS_UNICODE(top_star)
+        query = PyUnicode_AsWideCharString(top_star, &length)
         hFindFile = FindFirstFileW(query, &search_data)
         if hFindFile == INVALID_HANDLE_VALUE:
             # Raise an exception? This path doesn't seem to exist
@@ -244,7 +246,7 @@ cdef class Win32ReadDir:
                 if _should_skip(&search_data):
                     result = FindNextFileW(hFindFile, &search_data)
                     continue
-                name_unicode = _get_name(&search_data)
+                name_unicode = PyUnicode_FromWideChar(search_data.cFileName, -1)
                 name_utf8 = PyUnicode_AsUTF8String(name_unicode)
                 PyList_Append(dirblock,
                     (relprefix + name_utf8, name_utf8,

breezy/osutils.py

@@ -24,6 +24,7 @@
 
 from .lazy_import import lazy_import
 lazy_import(globals(), """
+import ctypes
 from datetime import datetime
 import getpass
 import locale
@@ -1304,7 +1305,7 @@ def _cicp_canonical_relpath(base, path):
 # filesystems), for example, so could probably benefit from the same basic
 # support there.  For now though, only Windows and OSX get that support, and
 # they get it for *all* file-systems!
-if sys.platform in ('win32', 'darwin'):
+if sys.platform in ('win32', 'darwin', 'cygwin'):
     canonical_relpath = _cicp_canonical_relpath
 else:
     canonical_relpath = relpath

breezy/plugins/upload/tests/test_upload.py

@@ -109,6 +109,16 @@ def assertUpFileEqual(self, content, path, base=upload_dir):
         self.assertFileEqual(content, osutils.pathjoin(base, path))
 
     def assertUpPathModeEqual(self, path, expected_mode, base=upload_dir):
+        # FIXME: at present, the upload is tested locally,
+        # so if local FS doesn't support the mode bits,
+        # all mode tests will fail.  The only mode bit
+        # that is reported as unsupported by the osutils module
+        # is the executable bit.  So skip if not supports_executable.
+        # This should better be tested with a dummy transport
+        # and not an actual file system.
+        if not osutils.supports_executable(path):
+            self.skipTest("Cannot determine mode bits for %s" % path)
+            return
         # FIXME: the tests needing that assertion should depend on the server
         # ability to handle chmod so that they don't fail (or be skipped)
         # against servers that can't. Note that some breezy transports define
@@ -387,6 +397,9 @@ def test_change_dir_into_file(self):
         self.assertUpFileEqual(b'bar', 'hello')
 
     def _test_make_file_executable(self, file_name):
+        if not osutils.supports_executable(file_name)
+            self.skipTest("%s cannot be marked executable" % filename)
+            return
         self.make_branch_and_working_tree()
         self.add_file(file_name, b'foo')
         self.chmod_file(file_name, 0o664)

breezy/tests/__init__.py

@@ -2848,7 +2848,15 @@ def setUp(self):
 
     def check_file_contents(self, filename, expect):
         self.log("check contents of file %s" % filename)
-        with open(filename, 'rb') as f:
+        # This seems to be called for text files only,
+        # but most of the existing tests check the contents
+        # against binary strings.  Now there are cases
+        # when we need to normalize the line endings; expect a string then.
+        if isinstance(expect, str):
+            args = {"mode": "rt", "encoding": "utf-8"}
+        else:
+            args = {"mode": "rb"}
+        with open(filename, **args) as f:
             contents = f.read()
         if contents != expect:
             self.log("expected: %r" % expect)
@@ -4467,11 +4475,9 @@ def _rmtree_temp_dir(dirname, test_id=None):
         if test_id is not None:
             ui.ui_factory.clear_term()
             sys.stderr.write('\nWhile running: %s\n' % (test_id,))
-        # Ugly, but the last thing we want here is fail, so bear with it.
-        printable_e = str(e).decode(osutils.get_user_encoding(), 'replace'
-                                    ).encode('ascii', 'replace')
+        # 21-nov-2021 [a1s] Revert revision 5231: in Python3, str has no decode
         sys.stderr.write('Unable to remove testing dir %s\n%s'
-                         % (os.path.basename(dirname), printable_e))
+                         % (os.path.basename(dirname), e))
 
 
 def probe_unicode_in_user_encoding():

breezy/tests/test__walkdirs_win32.py

@@ -37,9 +37,7 @@ class TestWin32Finder(tests.TestCaseInTempDir):
 
     def setUp(self):
         super(TestWin32Finder, self).setUp()
-        from ._walkdirs_win32 import (
-            Win32ReadDir,
-            )
+        from .._walkdirs_win32 import Win32ReadDir
         self.reader = Win32ReadDir()
 
     def _remove_stat_from_dirblock(self, dirblock):
@@ -58,45 +56,44 @@ def assertWalkdirs(self, expected, top, prefix=''):
         finally:
             osutils._selected_dir_reader = old_selected_dir_reader
 
-    def assertReadDir(self, expected, prefix, top_unicode):
+    def assertReadDir(self, expected, prefix, top):
         result = self._remove_stat_from_dirblock(
-            self.reader.read_dir(prefix, top_unicode))
+            self.reader.read_dir(prefix, top))
         self.assertEqual(expected, result)
 
     def test_top_prefix_to_starting_dir(self):
         # preparing an iteration should create a unicode native path.
         self.assertEqual(
-            ('prefix', None, None, None, u'\x12'),
+            (b'prefix', None, None, None, u'\x12'),
             self.reader.top_prefix_to_starting_dir(
                 u'\x12'.encode('utf8'), 'prefix'))
 
     def test_empty_directory(self):
-        self.assertReadDir([], 'prefix', u'.')
-        self.assertWalkdirs([(('', u'.'), [])], u'.')
+        self.assertReadDir([], b'prefix', b'.')
+        self.assertWalkdirs([((b'', u'.'), [])], b'.')
 
     def test_file(self):
         self.build_tree(['foo'])
-        self.assertReadDir([('foo', 'foo', 'file', u'./foo')],
+        self.assertReadDir([(b'foo', b'foo', 'file', u'./foo')],
                            '', u'.')
 
     def test_directory(self):
         self.build_tree(['bar/'])
-        self.assertReadDir([('bar', 'bar', 'directory', u'./bar')],
+        self.assertReadDir([(b'bar', b'bar', 'directory', u'./bar')],
                            '', u'.')
 
     def test_prefix(self):
         self.build_tree(['bar/', 'baf'])
         self.assertReadDir([
-            ('xxx/baf', 'baf', 'file', u'./baf'),
-            ('xxx/bar', 'bar', 'directory', u'./bar'),
+            (b'xxx/baf', b'baf', 'file', u'./baf'),
+            (b'xxx/bar', b'bar', 'directory', u'./bar'),
             ],
             'xxx', u'.')
 
     def test_missing_dir(self):
         e = self.assertRaises(WindowsError,
-                              self.reader.read_dir, 'prefix', u'no_such_dir')
-        self.assertEqual(errno.ENOENT, e.errno)
-        self.assertEqual(3, e.winerror)
+            self.reader.read_dir, b'prefix', u'no_such_dir')
+        # 3 is ERROR_PATH_NOT_FOUND, see WinError.h
         self.assertEqual((3, u'no_such_dir/*'), e.args)
 
 
@@ -106,7 +103,7 @@ class Test_Win32Stat(tests.TestCaseInTempDir):
 
     def setUp(self):
         super(Test_Win32Stat, self).setUp()
-        from ._walkdirs_win32 import lstat
+        from .._walkdirs_win32 import lstat
         self.win32_lstat = lstat
 
     def test_zero_members_present(self):

breezy/tests/test_bedding.py

@@ -134,6 +134,15 @@ def test_ensure_config_dir_exists(self):
 class TestDefaultMailDomain(tests.TestCaseInTempDir):
     """Test retrieving default domain from mailname file"""
 
+    def setUp(self):
+        if sys.platform == 'win32':
+            # The function deliberately returns None on Windows.
+            # Could run C:\Windows\System32\whoami.exe /upn, but that
+            # only works when the user is logged in to an AD domain.
+            # Why we don't just read /etc/maildomain, same as on all platforms?
+            raise tests.TestNotApplicable
+        super(TestDefaultMailDomain, self).setUp()
+
     def test_default_mail_domain_simple(self):
         with open('simple', 'w') as f:
             f.write("domainname.com\n")

breezy/tests/test_config.py

@@ -1012,12 +1012,12 @@ def test_config_creates_local(self):
         """Creating a new entry in config uses a local path."""
         branch = self.make_branch('branch', format='knit')
         branch.set_push_location('http://foobar')
-        local_path = osutils.getcwd().encode('utf8')
+        local_path = osutils.getcwd()
         # Surprisingly ConfigObj doesn't create a trailing newline
         self.check_file_contents(bedding.locations_config_path(),
-                                 b'[%s/branch]\n'
-                                 b'push_location = http://foobar\n'
-                                 b'push_location:policy = norecurse\n'
+                                 '[%s/branch]\n'
+                                 'push_location = http://foobar\n'
+                                 'push_location:policy = norecurse\n'
                                  % (local_path,))
 
     def test_autonick_urlencoded(self):
@@ -3190,11 +3190,22 @@ def test_branch_name_colo(self):
 
     def test_branch_name_basename(self):
         store = self.get_store(self)
-        store._load_from_string(dedent("""\
-            [/]
-            push_location=my{branchname}
-        """).encode('ascii'))
-        matcher = config.LocationMatcher(store, 'file:///parent/example%3c')
+        if sys.platform == "win32":
+            # Win32 file urls start with file:///x:/,
+            # where x is a valid drive letter
+            store._load_from_string(dedent("""\
+                [C:/]
+                push_location=my{branchname}
+            """).encode('ascii'))
+            matcher = config.LocationMatcher(store,
+                'file:///c:/parent/example%3c')
+        else:
+            store._load_from_string(dedent("""\
+                [/]
+                push_location=my{branchname}
+            """).encode('ascii'))
+            matcher = config.LocationMatcher(store,
+                'file:///parent/example%3c')
         self.assertEqual('example<', matcher.branch_name)
         ((_, section),) = matcher.get_sections()
         self.assertEqual('example<', section.locals['branchname'])
@@ -3220,19 +3231,31 @@ def test_empty(self):
 
     def test_url_vs_local_paths(self):
         # The matcher location is an url and the section names are local paths
-        self.assertSectionIDs(['/foo/bar', '/foo'],
-                              'file:///foo/bar/baz', b'''\
-[/foo]
-[/foo/bar]
-''')
+        if sys.platform == "win32":
+            # Win32 file urls start with file:///x:/,
+            # where x is a valid drive letter
+            self.assertSectionIDs(['C:/foo/bar', 'C:/foo'],
+                'file:///c:/foo/bar/baz',
+                b'[C:/foo]\n'
+                b'[C:/foo/bar]\n')
+        else:
+            self.assertSectionIDs(['/foo/bar', '/foo'],
+                'file:///foo/bar/baz',
+                b'[/foo]\n'
+                b'[/foo/bar]\n')
 
     def test_local_path_vs_url(self):
         # The matcher location is a local path and the section names are urls
-        self.assertSectionIDs(['file:///foo/bar', 'file:///foo'],
-                              '/foo/bar/baz', b'''\
-[file:///foo]
-[file:///foo/bar]
-''')
+        if sys.platform == "win32":
+            self.assertSectionIDs(['file:///C:/foo/bar', 'file:///C:/foo'],
+                'C:/foo/bar/baz',
+                b'[file:///C:/foo]\n'
+                b'[file:///C:/foo/bar]\n')
+        else:
+            self.assertSectionIDs(['file:///foo/bar', 'file:///foo'],
+                '/foo/bar/baz',
+                b'[file:///foo]\n'
+                b'[file:///foo/bar]\n')
 
     def test_no_name_section_included_when_present(self):
         # Note that other tests will cover the case where the no-name section