Merge lp:~a1s/brz/3.2 into lp:brz/3.2

[Aleksandr Smyshliaev]

This is an ongoing effort to make Breezy work on Windows

[Aleksandr Smyshliaev (a1s)]

This includes changes from the merge proposal https://code.launchpad.net/~a1s/brz/3.2-windows-test_osutils/+merge/412025

The other thing hat has been fixed after that is passing the regression tests run by brz selftes test_config.

[lp:~a1s/brz/3.2]

7564. By Aleksandr Smyshliaev on 2021-12-09

Merge brz/3.2

7565. By Aleksandr Smyshliaev on 2021-12-09

Get rid of osutils.open_file: in Python 3.4 and newer file descriptors are non-inheritable by default

7566. By Aleksandr Smyshliaev on 2021-12-09

Use relative import paths in test__walkdirs_win32

[lp:~a1s/brz/3.2]

7567. By Aleksandr Smyshliaev on 2022-01-04

Fix issues discussed in https://code.launchpad.net/~a1s/brz/3.2/+merge/412781

7568. By Aleksandr Smyshliaev on 2022-01-04

Merge lp:brz/3.2

[Aleksandr Smyshliaev (a1s)]

Revision 7568 is not ready to be merged, sorry. It went OK on Windows, but then I found that it's broken on *nix. I'm going to fix it and make another commit in a day or two.

[lp:~a1s/brz/3.2]

7569. By Aleksandr Smyshliaev on 2022-01-05

Add win32utils.list_fs_types()

7570. By Aleksandr Smyshliaev on 2022-01-06

Fix regression on *nix

[Aleksandr Smyshliaev (a1s)]

Ok, now this is tested on Linux/Python 3.10. Both test_config and test_osutils run without errors.

On Windows, test_config runs without errors, and test_osutils has two failures in test_osutils.Test_CICPCanonicalRelpath. Those I want to leave for another time.

I've replied to all questions asked here in diff comments.

Please check if this can be merged now.

[Jelmer Vernooij (jelmer)]

It would still be useful to split this merge proposal into several separate ones, e.g. one with the filesystem type changes, one with the symlink changes, one with the config changes, one with the walkdir fixes, etc.

[Aleksandr Smyshliaev (a1s)]

> It would still be useful to split this merge proposal into several separate
> ones, e.g. one with the filesystem type changes, one with the symlink changes,
> one with the config changes, one with the walkdir fixes, etc.

I surely see the point in submitting each atomic change in a different merge proposal.

The problem is, I'm not sure the regression would run with just one of those things changed and others left as is. Perhaps I would try that if the merge proposals are reviewed in hours, not weeks.

At the moment, my first priority is getting qbrz work on Linux, and then I'm going to take a pause to handle my RL issues. I'd really like to have Breezy working on Windows, but right now Bazaar 2.6 covers my needs there.

[Jelmer Vernooij (jelmer)]

> > It would still be useful to split this merge proposal into several separate
> > ones, e.g. one with the filesystem type changes, one with the symlink
> changes,
> > one with the config changes, one with the walkdir fixes, etc.
>
> I surely see the point in submitting each atomic change in a different merge
> proposal.
>
> The problem is, I'm not sure the regression would run with just one of those
> things changed and others left as is. Perhaps I would try that if the merge
> proposals are reviewed in hours, not weeks.
The delay in reviews is actually a function of the size of the merge proposals; for the smaller ones that are easy to understand the full implications of, I usually just process them as part of my regular email reading but for larger ones I need to schedule some time to sit down and understand them.

Jelmer

[Aleksandr Smyshliaev (a1s)]

I do understand. I'm sorry there are too many things to address at once.

[lp:~a1s/brz/3.2]

7571. By Aleksandr Smyshliaev on 2022-01-14

Merge lp:brz/3.2

7572. By Aleksandr Smyshliaev on 2022-01-18

Merge lp:brz/3.2

Unmerged revisions

7572. By Aleksandr Smyshliaev on 2022-01-18

Merge lp:brz/3.2

7571. By Aleksandr Smyshliaev on 2022-01-14

Merge lp:brz/3.2

7570. By Aleksandr Smyshliaev on 2022-01-06

Fix regression on *nix

7569. By Aleksandr Smyshliaev on 2022-01-05

Add win32utils.list_fs_types()

7568. By Aleksandr Smyshliaev on 2022-01-04

Merge lp:brz/3.2

7567. By Aleksandr Smyshliaev on 2022-01-04

Fix issues discussed in https://code.launchpad.net/~a1s/brz/3.2/+merge/412781

7566. By Aleksandr Smyshliaev on 2021-12-09

Use relative import paths in test__walkdirs_win32

7565. By Aleksandr Smyshliaev on 2021-12-09

Get rid of osutils.open_file: in Python 3.4 and newer file descriptors are non-inheritable by default

7564. By Aleksandr Smyshliaev on 2021-12-09

Merge brz/3.2

7563. By Aleksandr Smyshliaev on 2021-12-04

Fix regression tests run by selftest test_config on Windows

=== modified file 'breezy/_walkdirs_win32.pyx'
--- breezy/_walkdirs_win32.pyx	2020-06-08 19:42:53 +0000
+++ breezy/_walkdirs_win32.pyx	2022-01-18 07:08:36 +0000
@@ -58,14 +58,11 @@
 
     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 @@
                      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 @@
     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 @@
                 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,

=== modified file 'breezy/osutils.py'
--- breezy/osutils.py	2022-01-14 16:48:37 +0000
+++ breezy/osutils.py	2022-01-18 07:08:36 +0000
@@ -24,6 +24,7 @@
 
 from .lazy_import import lazy_import
 lazy_import(globals(), """
+import ctypes
 from datetime import datetime
 import getpass
 import locale
@@ -1301,7 +1302,7 @@
 # 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

=== modified file 'breezy/plugins/upload/tests/test_upload.py'
--- breezy/plugins/upload/tests/test_upload.py	2021-12-08 18:46:11 +0000
+++ breezy/plugins/upload/tests/test_upload.py	2022-01-18 07:08:36 +0000
@@ -109,6 +109,16 @@
         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 @@
         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)

=== modified file 'breezy/tests/__init__.py'
--- breezy/tests/__init__.py	2022-01-09 13:12:27 +0000
+++ breezy/tests/__init__.py	2022-01-18 07:08:36 +0000
@@ -2654,15 +2654,9 @@
         """
         root = TestCaseWithMemoryTransport.TEST_ROOT
         try:
-            # Make sure we get a readable and accessible home for brz.log
-            # and/or config files, and not fallback to weird defaults (see
-            # http://pad.lv/825027).
-            self.assertIs(None, os.environ.get('BRZ_HOME', None))
-            os.environ['BRZ_HOME'] = root
             from breezy.bzr.bzrdir import BzrDirMetaFormat1
             wt = controldir.ControlDir.create_standalone_workingtree(
                 root, format=BzrDirMetaFormat1())
-            del os.environ['BRZ_HOME']
         except Exception as e:
             self.fail("Fail to initialize the safety net: %r\n" % (e,))
         # Hack for speed: remember the raw bytes of the dirstate file so that
@@ -2696,11 +2690,31 @@
                                                     suffix='.tmp'))
             TestCaseWithMemoryTransport.TEST_ROOT = root
 
+            # Set BRZ_HOME to the test directory so that we never interfere
+            # with any configuration files existing on the system.
+            #
+            # Note that we need both BRZ_HOME AND BZR_HOME:
+            # BRZ_HOME may be set in the environment and we have to replace it,
+            # but then bedding doesn't find breezy config in the temporary
+            # directory, and falls back to BZR_HOME.
+            #
+            # This needs to be done before ._create_safety_net is called
+            # so that the working tree created there doesn't mess with
+            # the system environment.
+            self.overrideEnv('BRZ_HOME', root)
+            self.overrideEnv('BZR_HOME', root)
+
             self._create_safety_net()
 
             # The same directory is used by all tests, and we're not
             # specifically told when all tests are finished.  This will do.
             atexit.register(_rmtree_temp_dir, root)
+        else:
+            # Re-using existing safety net directory;
+            # isolate the environment (see comment above)
+            root = TestCaseWithMemoryTransport.TEST_ROOT
+            self.overrideEnv('BRZ_HOME', root)
+            self.overrideEnv('BZR_HOME', root)
 
         self.permit_dir(TestCaseWithMemoryTransport.TEST_ROOT)
         self.addCleanup(self._check_safety_net)
@@ -2848,7 +2862,15 @@
 
     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 +4489,9 @@
         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():

=== modified file 'breezy/tests/test__walkdirs_win32.py'
--- breezy/tests/test__walkdirs_win32.py	2018-11-12 01:41:38 +0000
+++ breezy/tests/test__walkdirs_win32.py	2022-01-18 07:08:36 +0000
@@ -37,9 +37,7 @@
 
     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 @@
         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 @@
 
     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):

=== modified file 'breezy/tests/test_bedding.py'
--- breezy/tests/test_bedding.py	2022-01-14 00:23:44 +0000
+++ breezy/tests/test_bedding.py	2022-01-18 07:08:36 +0000
@@ -43,11 +43,17 @@
         self.overrideEnv('HOME', '/home/bogus')
         self.overrideEnv('XDG_CACHE_HOME', '')
         if sys.platform == 'win32':
-            self.overrideEnv(
-                'BRZ_HOME',
-                r'C:\Documents and Settings\bogus\Application Data')
-            self.brz_home = \
-                'C:/Documents and Settings/bogus/Application Data/breezy'
+            # On Windows, the search sequence is:
+            # 1. BRZ_HOME if breezy config exists or else
+            #    if there's no existing bazaar config.
+            # 2. AppData\Roaming (from special shell folders if possible;
+            #    if not, then from APPDATA environment variable.)
+            # The HOME environment variable (set above) is never checked.
+            # Let's take BRZ_HOME, as it is the first.
+            # (Also disable BZR_HOME to not find existing bazaar config.)
+            self.overrideEnv('BRZ_HOME', r'C:\Users\bogus\AppData')
+            self.overrideEnv('BZR_HOME', r'C:\Users\bogus\AppData')
+            self.brz_home = 'C:/Users/bogus/AppData/breezy'
         else:
             self.brz_home = '/home/bogus/.config/breezy'
 
@@ -73,11 +79,19 @@
 class TestConfigPathFallback(tests.TestCaseInTempDir):
 
     def setUp(self):
+        if sys.platform == 'win32':
+            raise tests.TestNotApplicable(
+                'Fallback config paths are different on this platform')
         super(TestConfigPathFallback, self).setUp()
         self.overrideEnv('HOME', self.test_dir)
         self.overrideEnv('XDG_CACHE_HOME', '')
         self.bzr_home = os.path.join(self.test_dir, '.bazaar')
         os.mkdir(self.bzr_home)
+        # The safety net made by super() has set BZR_HOME and BRZ_HOME to the
+        # temporary root shared by all tests.  As they take precedence, we need
+        # to erase the variables in order to check the HOME created here.
+        self.overrideEnv('BRZ_HOME', None)
+        self.overrideEnv('BZR_HOME', None)
 
     def test_config_dir(self):
         self.assertEqual(bedding.config_dir(), self.bzr_home)
@@ -192,6 +206,15 @@
 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")

=== modified file 'breezy/tests/test_config.py'
--- breezy/tests/test_config.py	2020-06-23 01:02:30 +0000
+++ breezy/tests/test_config.py	2022-01-18 07:08:36 +0000
@@ -1012,12 +1012,12 @@
         """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_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_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

=== modified file 'breezy/tests/test_osutils.py'
--- breezy/tests/test_osutils.py	2022-01-16 17:48:26 +0000
+++ breezy/tests/test_osutils.py	2022-01-18 07:08:36 +0000
@@ -876,8 +876,10 @@
         self.requireFeature(features.win32_feature)
         self.assertEqual('C:/foo', osutils._win32_abspath('C:\\foo'))
         self.assertEqual('C:/foo', osutils._win32_abspath('C:/foo'))
-        self.assertEqual('//HOST/path', osutils._win32_abspath(r'\\HOST\path'))
-        self.assertEqual('//HOST/path', osutils._win32_abspath('//HOST/path'))
+        self.assertEqual('//HOST/SHARE/path',
+            osutils._win32_abspath(r'\\HOST\share\path'))
+        self.assertEqual('//HOST/SHARE/path',
+            osutils._win32_abspath('//host/SHARE/path'))
 
     def test_realpath(self):
         self.assertEqual('C:/foo', osutils._win32_realpath('C:\\foo'))
@@ -1068,10 +1070,30 @@
 
 class TestWalkDirs(tests.TestCaseInTempDir):
 
+    if sys.platform == 'win32':
+        @staticmethod
+        def _normalize_path_separators(path):
+            return path.replace("\\", "/")
+    else:
+        @staticmethod
+        def _normalize_path_separators(path):
+            return path
+
     def assertExpectedBlocks(self, expected, result):
-        self.assertEqual(expected,
-                         [(dirinfo, [line[0:3] for line in block])
-                          for dirinfo, block in result])
+        # This is randomly called with result obtained from _walkdirs_utf8
+        # which always uses forward slashes for path separators,
+        # and simple walkdirs, which relies on os.scandir,
+        # so yields paths with backslashes on Windows.
+        # We don't compare the "path-from-top" members here,
+        # but in deeper trees it appears in the "directory-path-from-top".
+        # We have to normalize the slashes in that element.
+        _normalize_walkdirs_result = lambda spec: [(
+            (relpath, self._normalize_path_separators(fromtop)),
+            [line[:3] for line in block],
+        ) for ((relpath, fromtop), block) in spec]
+        self.assertEqual(
+            _normalize_walkdirs_result(expected),
+            _normalize_walkdirs_result(result))
 
     def test_walkdirs(self):
         tree = [
@@ -1223,13 +1245,23 @@
         self.assertExpectedBlocks(expected_dirblocks[1:], result)
 
     def _filter_out_stat(self, result):
-        """Filter out the stat value from the walkdirs result"""
-        for dirdetail, dirblock in result:
+        """Filter out the stat value from the walkdirs result
+
+        If needed, also change the directory separators
+        in the path-from-top elements to be forward slashes.
+
+        """
+        new_result = []
+        for ((relpath, fromtop), dirblock) in result:
             new_dirblock = []
             for info in dirblock:
                 # Ignore info[3] which is the stat
-                new_dirblock.append((info[0], info[1], info[2], info[4]))
-            dirblock[:] = new_dirblock
+                new_dirblock.append((info[0], info[1], info[2],
+                    self._normalize_path_separators(info[4])))
+            new_result.append((
+                (relpath, self._normalize_path_separators(fromtop)),
+                new_dirblock))
+        return new_result
 
     def _save_platform_info(self):
         self.overrideAttr(osutils, '_fs_enc')
@@ -1257,6 +1289,8 @@
             UTF8DirReaderFeature.module.UTF8DirReader, b".")
 
     def test_force_walkdirs_utf8_fs_latin1(self):
+        if sys.platform == "win32":
+            self.skipTest("On Windows, FS functions use wide chars")
         self._save_platform_info()
         osutils._fs_enc = 'iso-8859-1'
         self.assertDirReaderIs(osutils.UnicodeDirReader, ".")
@@ -1301,11 +1335,9 @@
                 ]
              ),
             ]
-        result = list(osutils.walkdirs('.'))
-        self._filter_out_stat(result)
+        result = self._filter_out_stat(osutils.walkdirs('.'))
         self.assertEqual(expected_dirblocks, result)
-        result = list(osutils.walkdirs(u'./' + name1, name1))
-        self._filter_out_stat(result)
+        result = self._filter_out_stat(osutils.walkdirs(u'./' + name1, name1))
         self.assertEqual(expected_dirblocks[1:], result)
 
     def test_unicode__walkdirs_utf8(self):
@@ -1415,8 +1447,7 @@
                 ]
              ),
             ]
-        result = list(osutils._walkdirs_utf8('.'))
-        self._filter_out_stat(result)
+        result = self._filter_out_stat(osutils._walkdirs_utf8('.'))
         self.assertEqual(expected_dirblocks, result)
 
     def test__walkdirs_utf8_win32readdir(self):
@@ -1443,26 +1474,25 @@
         # All of the abspaths should be in unicode, all of the relative paths
         # should be in utf8
         expected_dirblocks = [
-            (('', '.'),
+            ((b'', '.'),
              [(name0, name0, 'file', './' + name0u),
               (name1, name1, 'directory', './' + name1u),
               (name2, name2, 'file', './' + name2u),
               ]
              ),
             ((name1, './' + name1u),
-             [(name1 + '/' + name0, name0, 'file', './' + name1u
+             [(name1 + b'/' + name0, name0, 'file', './' + name1u
                + '/' + name0u),
-              (name1 + '/' + name1, name1, 'directory', './' + name1u
+              (name1 + b'/' + name1, name1, 'directory', './' + name1u
                + '/' + name1u),
               ]
              ),
-            ((name1 + '/' + name1, './' + name1u + '/' + name1u),
+            ((name1 + b'/' + name1, './' + name1u + '/' + name1u),
              [
                 ]
              ),
             ]
-        result = list(osutils._walkdirs_utf8(u'.'))
-        self._filter_out_stat(result)
+        result = self._filter_out_stat(osutils._walkdirs_utf8(u'.'))
         self.assertEqual(expected_dirblocks, result)
 
     def assertStatIsCorrect(self, path, win32stat):
@@ -1471,8 +1501,10 @@
         self.assertAlmostEqual(os_stat.st_mtime, win32stat.st_mtime, places=4)
         self.assertAlmostEqual(os_stat.st_ctime, win32stat.st_ctime, places=4)
         self.assertAlmostEqual(os_stat.st_atime, win32stat.st_atime, places=4)
-        self.assertEqual(os_stat.st_dev, win32stat.st_dev)
-        self.assertEqual(os_stat.st_ino, win32stat.st_ino)
+        # win32stat is built from WIN32_FIND_DATA structure
+        # which contains neither dwVolumeSerialNumber nor nFileIndex
+        #self.assertEqual(os_stat.st_dev, win32stat.st_dev)
+        #self.assertEqual(os_stat.st_ino, win32stat.st_ino)
         self.assertEqual(os_stat.st_mode, win32stat.st_mode)
 
     def test__walkdirs_utf_win32_find_file_stat_file(self):
@@ -1489,7 +1521,7 @@
         with open(name0u, 'ab') as f:
             f.write(b'just a small update')
 
-        result = Win32ReadDir().read_dir('', u'.')
+        result = Win32ReadDir().read_dir(b'', u'.')
         entry = result[0]
         self.assertEqual((name0, name0, 'file'), entry[:3])
         self.assertEqual(u'./' + name0u, entry[4])
@@ -1617,13 +1649,17 @@
         processed_links = []
 
         def file_handler(from_path, to_path):
-            processed_files.append(('f', from_path, to_path))
+            processed_files.append(
+                ('f', osutils.normpath(from_path), osutils.normpath(to_path)))
 
         def dir_handler(from_path, to_path):
-            processed_files.append(('d', from_path, to_path))
+            processed_files.append(
+                ('d', osutils.normpath(from_path), osutils.normpath(to_path)))
 
         def link_handler(from_path, to_path):
-            processed_links.append((from_path, to_path))
+            processed_links.append(
+                (osutils.normpath(from_path), osutils.normpath(to_path)))
+
         handlers = {'file': file_handler,
                     'directory': dir_handler,
                     'symlink': link_handler,

=== modified file 'breezy/transport/local.py'
--- breezy/transport/local.py	2022-01-12 23:43:40 +0000
+++ breezy/transport/local.py	2022-01-18 07:08:36 +0000
@@ -30,13 +30,12 @@
 
 from breezy import (
     atomicfile,
-    osutils,
     urlutils,
     )
 from breezy.transport import LateReadError
 """)
 
-from .. import transport
+from .. import osutils, transport
 
 
 _append_flags = os.O_CREAT | os.O_APPEND | os.O_WRONLY | osutils.O_BINARY | osutils.O_NOINHERIT
@@ -154,7 +153,10 @@
             path = self._abspath(relpath)
             return open(path, 'rb')
         except (IOError, OSError) as e:
-            if e.errno == errno.EISDIR:
+            if e.errno == errno.EISDIR or (
+                (sys.platform == "win32") and (e.errno == errno.EACCES)
+                and os.path.isdir(path)
+            ):
                 return LateReadError(relpath)
             self._translate_error(e, path)