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

ntpath.realpath() should use GetFinalPathNameByHandle() #58302

Closed
vstinner opened this issue Feb 22, 2012 · 10 comments
Closed

ntpath.realpath() should use GetFinalPathNameByHandle() #58302

vstinner opened this issue Feb 22, 2012 · 10 comments
Labels
3.8 OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

vstinner commented Feb 22, 2012

BPO 14094
Nosy @pfmoore, @amauryfa, @pitrou, @vstinner, @tjguk, @briancurtin, @akheron, @zware, @eryksun, @zooba, @vladima
PRs
  • bpo-14094: Use _getfinalpathname to implement realpath #11248
  • 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 2019-09-11.13:13:11.798>
    created_at = <Date 2012-02-22.23:47:49.504>
    labels = ['3.8', 'type-feature', 'library', 'OS-windows']
    title = 'ntpath.realpath() should use GetFinalPathNameByHandle()'
    updated_at = <Date 2019-09-11.13:13:11.797>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2019-09-11.13:13:11.797>
    actor = 'steve.dower'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-09-11.13:13:11.798>
    closer = 'steve.dower'
    components = ['Library (Lib)', 'Windows']
    creation = <Date 2012-02-22.23:47:49.504>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 14094
    keywords = ['patch']
    message_count = 10.0
    messages = ['154018', '222954', '297133', '297139', '297188', '332094', '332152', '335040', '335042', '351857']
    nosy_count = 11.0
    nosy_names = ['paul.moore', 'amaury.forgeotdarc', 'pitrou', 'vstinner', 'tim.golden', 'brian.curtin', 'petri.lehtinen', 'zach.ware', 'eryksun', 'steve.dower', 'v2m']
    pr_nums = ['11248']
    priority = 'normal'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue14094'
    versions = ['Python 3.8']

    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 22, 2012

    nt.realpath() should use GetFinalPathNameByHandleW() when available, instead of GetFullPathNameW(), to resolve symlinks.

    By the way, it's strange that Py_GetFinalPathNameByHandleW() is called with VOLUME_NAME_NT to get the buffer size, and then with VOLUME_NAME_DOS. Is it a bug?

    /* We have a good handle to the target, use it to determine the
       target path name. */
    buf_size = Py_GetFinalPathNameByHandleW(hFile, 0, 0, VOLUME_NAME_NT);
    
        if(!buf_size)
            return win32_error_object("GetFinalPathNameByHandle", po);
    
        target_path = (wchar_t *)malloc((buf_size+1)*sizeof(wchar_t));
        if(!target_path)
            return PyErr_NoMemory();
    
        result_length = Py_GetFinalPathNameByHandleW(hFile, target_path,
                                                     buf_size, VOLUME_NAME_DOS);

    See also issue bpo-9333 (issue which added os.symlink() on Windows).

    @vstinner vstinner added the stdlib Python modules in the Lib dir label Feb 22, 2012
    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 13, 2014

    I'm assuming that this should be treated as an enhancement request.

    @BreamoreBoy BreamoreBoy mannequin added the type-feature A feature request or enhancement label Jul 13, 2014
    @vstinner
    Copy link
    Member Author

    vstinner commented Jun 28, 2017

    Python 3 now uses GetFinalPathNameByHandle()!

    @eryksun
    Copy link
    Contributor

    eryksun commented Jun 28, 2017

    I assume by nt.realpath we're talking about ntpath.realpath. In that case nothing was done to fix this. It's still an alias for ntpath.abspath, which calls GetFullPathNameW.

    @eryksun eryksun added the 3.7 label Jun 28, 2017
    @eryksun eryksun reopened this Jun 28, 2017
    @eryksun eryksun changed the title nt.realpath() should use GetFinalPathNameByHandle() when available ntpath.realpath() should use GetFinalPathNameByHandle() Jun 28, 2017
    @vstinner
    Copy link
    Member Author

    vstinner commented Jun 28, 2017

    Oh, only os.stat() was patched to use GetFinalPathNameByHandle()? It seems like most of the code was already written, so it shouldn't be hard to add a nt.GetFinalPathName() function using the win32_xstat_impl() code (and get_target_path()), to implement a real ntpath.realpath(), no?

    @eryk: Are you interested to work on a patch?

    @vstinner
    Copy link
    Member Author

    vstinner commented Dec 18, 2018

    Any update on this issue?

    @vstinner vstinner added 3.8 and removed 3.7 labels Dec 18, 2018
    @vladima
    Copy link
    Mannequin

    vladima mannequin commented Dec 19, 2018

    I can give it a try.

    @eryksun
    Copy link
    Contributor

    eryksun commented Feb 7, 2019

    Ping on PR 11248. It would be nice to get this into 3.8.

    @zooba
    Copy link
    Member

    zooba commented Feb 7, 2019

    Posted a review. I suggest a few changes for the sake of tidying up, but I agree that I'd like to see more tests added.

    @zooba
    Copy link
    Member

    zooba commented Sep 11, 2019

    As of 3.8, it (sort of) does.

    @zooba zooba closed this as completed Sep 11, 2019
    @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
    3.8 OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants