This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author db3l
Recipients SilentGhost, benhoyt, db3l, eryksun, ideasman42, mont29, paul.moore, python-dev, serhiy.storchaka, steve.dower, tim.golden, vstinner, zach.ware
Date 2016-03-29.10:36:32
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1459247794.31.0.574279942344.issue25911@psf.upfronthosting.co.za>
In-reply-to
Content
I'm including some comments here from an email thread I had with Victor about this issue on the Win8 buildbot, which led to his recent changeset 2b25fa7e3b7a.

The Win8 3.x failure (the 5 != 4 length error) was due to the revision of os._DummyDirEntry (introduced I believe in eb91d0387d59) using stat() rather than lstat() to check for a symbolic link, which couldn't work, which in turn failed to exclude the symbolic directory link from the walk in the test.

The 3.5 branch failure about the mis-matched tuple values from the walk is a separate issue, where the broken directory link is still counted as a directory (rather than a file).  It seems due to os.path.isdir() - in the older os._DummyDirEntry implementation - returning True for the broken link in the test, rather than False as on Unix.  That's because on Unix the underlying os.stat call fails, but on Windows isdir is replaced with nt._isdir which avoids os.stat but is just a shim for.  The success on Windows sort of makes sense since the target_is_directory parameter to os.symlink() was used to create the broken link in the test, so Windows knows it's a directory, even if the link target is missing.

If the bytes walk implementation needs to treat broken links like files on Windows (matching the non-bytes version), then it can't depend on isdir() failing.  The non-bytes version (using scandir()) works even on Windows since internally scandir appears to build up mode bits out of os.stat/os.lstat calls and never uses isdir.

Back-porting the 3.x implementation of DummyDirEntry would be one quick way to fix the 3.5 issue as it also avoids isdir().

BTW, with respect to changeset 2b25fa7e3b7a, I'm not sure it has quite the right semantics for is_dir, at least not if it's supposed to parallel os.path.isdir().  I believe that should return True for a symbolic link to a directory, which it doesn't look like this change would, if is_symlink happened to have been called first.  It's possible the semantics of how _DummyDirEntry is used precludes that scenario, but it seems a little fragile.  I'd probably just use lstat() for is_symlink, but otherwise not touch is_dir.

Finally, as to why this only showed up on the Win8 buildbot - it turns out that's the only machine where the tests could create a symbolic link and thus encounter the scenario - they were prevented on Win7 and Win10 under the normal buildbot user due to lack of privileges (which was a surprise to me).  That's also what was happening on Victor's local Win8 VM.  So the failing conditions were just being skipped.  The Win8 buildbot was itself sort of pure luck, as early on I must have set it up to run under an elevated (administrative) command prompt, probably due to trying to handle some other issue.  I've now done the same thing for Win10 so it began seeing the same failures in the last few tests.
History
Date User Action Args
2016-03-29 10:36:34db3lsetrecipients: + db3l, paul.moore, vstinner, tim.golden, ideasman42, SilentGhost, benhoyt, python-dev, zach.ware, serhiy.storchaka, eryksun, steve.dower, mont29
2016-03-29 10:36:34db3lsetmessageid: <1459247794.31.0.574279942344.issue25911@psf.upfronthosting.co.za>
2016-03-29 10:36:34db3llinkissue25911 messages
2016-03-29 10:36:32db3lcreate