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

Implement os.path.samefile and os.path.sameopenfile on Windows #50235

Closed
sandberg mannequin opened this issue May 10, 2009 · 10 comments
Closed

Implement os.path.samefile and os.path.sameopenfile on Windows #50235

sandberg mannequin opened this issue May 10, 2009 · 10 comments
Labels
type-feature A feature request or enhancement

Comments

@sandberg
Copy link
Mannequin

sandberg mannequin commented May 10, 2009

BPO 5985
Nosy @asvetlov
Files
  • samefile.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 = None
    closed_at = <Date 2010-09-09.20:39:36.417>
    created_at = <Date 2009-05-10.10:02:47.191>
    labels = ['type-feature']
    title = 'Implement os.path.samefile and os.path.sameopenfile on Windows'
    updated_at = <Date 2010-09-09.20:39:36.415>
    user = 'https://bugs.python.org/sandberg'

    bugs.python.org fields:

    activity = <Date 2010-09-09.20:39:36.415>
    actor = 'ocean-city'
    assignee = 'none'
    closed = True
    closed_date = <Date 2010-09-09.20:39:36.417>
    closer = 'ocean-city'
    components = []
    creation = <Date 2009-05-10.10:02:47.191>
    creator = 'sandberg'
    dependencies = []
    files = ['15122']
    hgrepos = []
    issue_num = 5985
    keywords = ['patch']
    message_count = 10.0
    messages = ['87518', '87840', '93970', '93973', '93979', '93986', '93993', '95931', '96016', '115973']
    nosy_count = 3.0
    nosy_names = ['ocean-city', 'sandberg', 'asvetlov']
    pr_nums = []
    priority = 'normal'
    resolution = 'out of date'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue5985'
    versions = ['Python 2.7', 'Python 3.2']

    @sandberg
    Copy link
    Mannequin Author

    sandberg mannequin commented May 10, 2009

    It would be nice if samefile / sameopenfile was present on Windows.
    Right now I usually work around this by keeping a platform-specific hack
    for Windows that approximates samefile by comparing normalized paths;
    this is ugly and doesn't handle junctions correctly.

    In one of my projects I have written a C implementation of samefile,
    which I manually monkey-patched os.path with. It would probably be
    rather easy to adapt it to become a native part of ntpath.

    My code relies on GetFileInformationByHandle, which is only available in
    Windows 2000 professional and newer
    (http://msdn.microsoft.com/en-us/library/aa364952(VS.85).aspx); if I
    understood it correctly this should not be a problem as Python 2.6 and
    newer doesn't support older versions of Windows.

    Unfortunately I don't use Windows myself, but I have rdesktop access to
    an XP machine with cygwin and Visual Studio 2005 installed (which seems
    insufficient to build python 2.6, at least), so it will be difficult for
    me to test my code. But I'll post some code soon.

    @sandberg sandberg mannequin added the type-feature A feature request or enhancement label May 10, 2009
    @terryjreedy
    Copy link
    Member

    At this point, 2.7/3.2 are the only targets for new features.

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Oct 14, 2009

    Here is my experimental patch.

    @sandberg
    Copy link
    Mannequin Author

    sandberg mannequin commented Oct 14, 2009

    An alternative solution which I would have considered, is to extend
    stat/fstat on Windows to set st_dev and st_ino to sensible values (based
    on dwVolumeSerialNumber and nFileIndexLow/High), and then use the POSIX
    implementations of samefile/sameopenfile.

    There may be one potential problem with this solution though: It would
    require stat to perform a CreateFile in addition to GetFileAttributesEx,
    and I don't know if there are situations where one is allowed to call
    the latter but not the former on a file. There would be no such problems
    with fstat, though.

    In your patch, I think the dwShareMode parameter to CreateFile* should
    be changed to FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE so
    make the operation less intrusive.

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Oct 14, 2009

    extend stat/fstat on Windows to set st_dev and st_ino to sensible
    values (based on dwVolumeSerialNumber and nFileIndexLow/High)

    Once I considered this approach, but problems was that
    nFileIndexLow/High can change every time file handle is opened, so it's
    not unique. See remarks in following page.
    http://msdn.microsoft.com/en-us/library/aa363788%28VS.85%29.aspx

    I think the dwShareMode parameter to CreateFile* should
    be changed to FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE
    so make the operation less intrusive.

    Probably you are right. I must admit I'm not familiar with this *shared*
    flag, and I was careless about its usage.

    @sandberg
    Copy link
    Mannequin Author

    sandberg mannequin commented Oct 14, 2009

    Once I considered this approach, but problems was that
    nFileIndexLow/High can change every time file handle is opened, so
    it's not unique.

    Ah, I see, then your approach makes sense.

    There's another part of your patch that I don't understand:

    +except ImportError: # not running on Windows - mock up something sensible
    + from posixpath import samefile # XXX

    In what situations will this happen, and are we guaranteed in those
    cases that samefile will be found in posixpath?

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Oct 14, 2009

    I'm not sure about this neither. So, XXX is in comment. ;-)

    On abspath() above, it also tries to import native method
    _getfullpathname(), and when it fails alternative implementation runs.
    (probably when someone calls ntpath.abspath from linux or somewhere.....
    Does it really happen? I don't know)

    I was not sure what kind of alternative implementation is appropriate,
    so I borrowed posixmodule's one. Probably more appropriate
    implementation may exist.

    @sandberg
    Copy link
    Mannequin Author

    sandberg mannequin commented Dec 3, 2009

    Once I considered this approach, but problems was that
    nFileIndexLow/High can change every time file handle is opened, so it's
    not unique. See remarks in following page.
    http://msdn.microsoft.com/en-us/library/aa363788%28VS.85%29.aspx

    Actually, that page only says that file identities may vary over time,
    and that it may happen during operations such as defragmentation.
    However, I have been in contact with a customer who observed a file
    system where the file index actually changed every time a file was
    closed and re-opened, given that nobody else kept the file open
    inbetween. This was on some kind of network mount, I wasn't told which
    kind (and I couldn't reproduce it with samba).

    This means that it's actually essential that you don't close the file
    between the CreateFile calls (and yes, your patch does this correctly).

    @asvetlov
    Copy link
    Contributor

    asvetlov commented Dec 6, 2009

    Please note: patch for http://bugs.python.org/issue1578269 is already has
    implementation for samefile. It's easer to add sameopenfile is same way
    than maintain two different approaches.

    Unfortunately solution will work only starting from Windows Vista and
    Windows Server 2008.

    @ocean-city
    Copy link
    Mannequin

    ocean-city mannequin commented Sep 9, 2010

    Both os.path.samefile and os.path.sameopenfile are now in py3k.
    And release27-maint is in feature freeze, so I think this issue
    should be closed.

    # Implemented on
    os.path.samefile: bpo-1578269
    os.path.sameopenfile: bpo-7566

    @ocean-city ocean-city mannequin closed this as completed Sep 9, 2010
    @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
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants