Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os.path.ismount on windows doesn't support windows mount points #53281

Closed
OrenHeld mannequin opened this issue Jun 20, 2010 · 21 comments
Closed

os.path.ismount on windows doesn't support windows mount points #53281

OrenHeld mannequin opened this issue Jun 20, 2010 · 21 comments
Assignees
Labels
OS-windows type-bug An unexpected behavior, bug, or error

Comments

@OrenHeld
Copy link
Mannequin

OrenHeld mannequin commented Jun 20, 2010

BPO 9035
Nosy @atsuoishimoto, @orsenthil, @giampaolo, @tjguk, @briancurtin, @ctismer
Files
  • issue9035.patch
  • issue9035.4.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/tjguk'
    closed_at = <Date 2013-08-01.15:02:40.641>
    created_at = <Date 2010-06-20.08:49:10.058>
    labels = ['type-bug', 'OS-windows']
    title = "os.path.ismount on windows doesn't support windows mount\tpoints"
    updated_at = <Date 2013-08-01.15:02:40.639>
    user = 'https://bugs.python.org/OrenHeld'

    bugs.python.org fields:

    activity = <Date 2013-08-01.15:02:40.639>
    actor = 'tim.golden'
    assignee = 'tim.golden'
    closed = True
    closed_date = <Date 2013-08-01.15:02:40.641>
    closer = 'tim.golden'
    components = ['Windows']
    creation = <Date 2010-06-20.08:49:10.058>
    creator = 'Oren_Held'
    dependencies = []
    files = ['26558', '31098']
    hgrepos = []
    issue_num = 9035
    keywords = ['patch']
    message_count = 21.0
    messages = ['108225', '108230', '108231', '108232', '108361', '108363', '134434', '134670', '138197', '144895', '144896', '166703', '171467', '193562', '193932', '193939', '193958', '193989', '194049', '194065', '194077']
    nosy_count = 11.0
    nosy_names = ['ishimoto', 'orsenthil', 'giampaolo.rodola', 'tim.golden', 'brian.curtin', 'sijinjoseph', 'Oren_Held', 'santoso.wijaya', 'markm', 'python-dev', 'Christian.Tismer']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue9035'
    versions = ['Python 3.4']

    @OrenHeld
    Copy link
    Mannequin Author

    OrenHeld mannequin commented Jun 20, 2010

    On unices, ismount checks whether the given path is a mount point.
    On windows, it only checks whether it's a drive letter.

    Long story short, Python simply returns False when doing ismount(r"c:\mount1"), while c:\mount1 is a real mount point.

    This is relevant for all modern windows versions.

    --

    I'm using win32file.GetVolumePathName() for overcoming this, but I'm not sure if the os python package should be importing win32file, maybe there is a better way to check whether a path is a mount point..

    @OrenHeld OrenHeld mannequin added the OS-windows label Jun 20, 2010
    @tjguk
    Copy link
    Member

    tjguk commented Jun 20, 2010

    Switching to Python 3.2 as this essentially constitutes a behaviour change and 2.6 is in bugfix mode and 2.7 is about to enter rc2. It would certainly be possible to use one of the volume APIs under the covers. Would you be willing to offer a patch to, say, posixmodule.c?

    @tjguk tjguk self-assigned this Jun 20, 2010
    @tjguk tjguk added the type-bug An unexpected behavior, bug, or error label Jun 20, 2010
    @tjguk
    Copy link
    Member

    tjguk commented Jun 20, 2010

    All we need to do is check the FILE_ATTRIBUTE_REPARSE_POINT
    in the file attributes. Frustratingly, we grab file attributes
    a dozen times in posixpath.c only to throw most of it away.

    Is there a case for adding an "attributes" function to os.path
    which exposes the full file attributes on Windows, and its
    posix equivalent if there is one? This could then be used
    in the ismount function currently implemented in ntpath.py.

    @tjguk tjguk changed the title os.path.ismount on windows doesn't support windows mount points os.path.ismount on windows doesn't support windows mount points Jun 20, 2010
    @tjguk
    Copy link
    Member

    tjguk commented Jun 20, 2010

    ... of course you still need to get the reparse tag to determine whether this is a mount point so the file attributes alone in this case are not enough.

    @orsenthil
    Copy link
    Member

    I see that ismount like function on windows is provide by the various
    Win32 extensions.

    If Windows supported is added to ismount function itself, then it might be
    a good idea to have attributes or list_attributes function as well.

    But for posix, how will it be different from details provided by stat?
    Would not it add redundancy?

    Or would it be better to provide file attributes as part of stat
    itself (if some are missing in Windows).

    @tjguk
    Copy link
    Member

    tjguk commented Jun 22, 2010

    I think we're saying the same thing :)

    The simplest thing to do here is to create a win_ismount function
    in posixmodule.c which does the attributes / reparse tag dance and
    returns True/False and use that wherever it's needed to support this
    concept under Windows. The current solution is correct for a subset
    of cases. Arguably a bug, although I doubt I'd get that past the
    release manager!

    The wider issue of exposing GetFileAttributesW, eg under one of the
    unused stat fields, should be explored elsewhere.

    On 22/06/2010 11:46, Senthil Kumaran wrote:

    Senthil Kumaran<orsenthil@gmail.com> added the comment:

    I see that ismount like function on windows is provide by the various
    Win32 extensions.

    If Windows supported is added to ismount function itself, then it might be
    a good idea to have attributes or list_attributes function as well.

    But for posix, how will it be different from details provided by stat?
    Would not it add redundancy?

    Or would it be better to provide file attributes as part of stat
    itself (if some are missing in Windows).

    ----------
    nosy: +orsenthil


    Python tracker<report@bugs.python.org>
    <http://bugs.python.org/issue9035\>



    Python-bugs-list mailing list
    Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/mail%40timgolden.me.uk

    @sijinjoseph
    Copy link
    Mannequin

    sijinjoseph mannequin commented Apr 26, 2011

    I'd like to add the win_ismount function mentioned by Tim. Is anyone else working on this presently?

    @orsenthil
    Copy link
    Member

    Sijin, please go ahead and submit a patch. No one is working on this at the moment.

    @markm
    Copy link
    Mannequin

    markm mannequin commented Jun 12, 2011

    I was looking at this - and see that (at least as far as GetFileAttributes is concerned) that a mount and a linked directory are seen the same...

    Here are some tests using ctypes

    # mounted drive
    >>> hex(windll.kernel32.GetFileAttributesW(ur"c:\temp\test_c_mount"))
    '0x410'
    
    # normal directory
    >>> hex(windll.kernel32.GetFileAttributesW(ur"c:\temp\orig"))
    '0x10'
    
    # link (created via mklink /d c:\temp\orig c:\temp\here2
    >>> hex(windll.kernel32.GetFileAttributesW(ur"c:\temp\here2"))
    '0x410'

    On futher searching - I found the following link:
    http://msdn.microsoft.com/en-us/library/aa363940%28v=vs.85%29.aspx

    So the function ismount will need to do the following
    a) Get the file attributes
    b) check that it's a directory and is a reparse point
    c) Use FindFirstFile (and FindNextFile? - I need to test more) to fill in WIN32_FIND_DATA.dwResearved0
    d) Check that against IO_REPARSE_TAG_MOUNT_POINT (0xA0000003)

    @OrenHeld
    Copy link
    Mannequin Author

    OrenHeld mannequin commented Oct 4, 2011

    Anything wrong with the following simple approach? (e.g. is it bad to depend on win32file?)

    def win_ismount(path):
      import win32file
      volume_path = win32file.GetVolumePathName(path)
      return volume_path == path # May have to ignore a trailing backslash

    @briancurtin
    Copy link
    Member

    We can't depend on stuff from pywin32, but we could expose GetVolumePathName ourselves.

    @atsuoishimoto
    Copy link
    Mannequin

    atsuoishimoto mannequin commented Jul 29, 2012

    Patch to expose GetVolumePathName() and implementation of ismount().
    Tested on Windows7/XP.

    @tjguk
    Copy link
    Member

    tjguk commented Sep 28, 2012

    Unfortunately this missed the boat for 3.3; I'll target 3.4 when we've got a branch to commit to.

    @ctismer
    Copy link
    Contributor

    ctismer commented Jul 22, 2013

    Hi Tim,

    Yes, this would be great to get sorted out.
    Then we could make watchdog.py automatically
    configure itself for network mounts.

    Right now this makes no nense because of windows.

    cheers - chris

    @tjguk
    Copy link
    Member

    tjguk commented Jul 30, 2013

    I put a bit of work in on this this morning, following Mark's suggestion (msg138197) since that's the "canonical" approach. Unfortunately, it completely fails to work for the most common case: the root folder of a drive! The documentation for FindFirstFile explicitly precludes that possibility.

    It looks as though GetVolumePathName is the way to go. I thought I'd previously found some instance where that failed but, ad hoc, I can't make it fail now. I'll try to rework Atsuo's patch against the current posixmodule.c.

    @tjguk
    Copy link
    Member

    tjguk commented Jul 30, 2013

    bpo-9035.2.patch is an updated version of Atsuo's patch.

    Known issues:

    • I haven't reworked it for the new memory-management API
    • There's no test for a non-root mount point (which is really the basis for this issue). It's difficult to see how to do that in a robust way on an arbitrary machine without quite a bit of support machinery. I've done ad hoc tests which succeed.

    @tjguk
    Copy link
    Member

    tjguk commented Jul 30, 2013

    bpo-9035.3.patch has switched to the new memory management API and has
    tweaked the tests slightly for robustness.

    This approach does introduce a behavioural change: the root of a SUBSTed
    drive (essentially a symlink into the Dos namespace) will raise an
    OSError because GetVolumePathName returns error 87: invalid parameter.
    So os.path.ismount("F:\\") will fail where F: is the result of running,
    eg, "SUBST F: C:\temp".

    I think the simplest thing is to special-case drive roots (which are
    always mount points) and then to apply the new GetVolumePathName logic.

    @tjguk
    Copy link
    Member

    tjguk commented Jul 31, 2013

    4th and hopefully final patch. Added tests for byte paths. Reworked the ismount so it uses the original detection approach first (which is wholly lexical) and drops back to the volume path technique only if the path doesn't appear to be a drive or a share root. This should minimise backwards-incompatibility while still solving the original problem.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 1, 2013

    New changeset f283589cb71e by Tim Golden in branch 'default':
    Issue bpo-9035: os.path.ismount now recognises volumes mounted below
    http://hg.python.org/cpython/rev/f283589cb71e

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 1, 2013

    New changeset 5258c4399f2e by Tim Golden in branch 'default':
    bpo-9035: Prevent Windows-specific tests from running on non-Windows platforms
    http://hg.python.org/cpython/rev/5258c4399f2e

    @tjguk
    Copy link
    Member

    tjguk commented Aug 1, 2013

    Fixed. Thanks for the patch

    @tjguk tjguk closed this as completed Aug 1, 2013
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    OS-windows type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants