Skip to content

Handle leading // for posixpath.realpath #117338

Description

@nineteendo

Bug report

Bug description:

>>> import posixpath
>>> posixpath.realpath("//foo")
'/foo'

Expected: //foo with precisely two leading slashes, see: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13

This can be quite easily solved using posixpath.splitroot, see the PR.

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

Activity

  1. barneygale commented on Mar 28, 2024

    @barneygale
    Contributor

    os.path.realpath() resolves the path using the current OS's rules, which might be more specific than POSIX. Both my Linux and macOS machines resolve //foo to /foo. Are there any supported Python platforms where // isn't resolved to /?

  2. nineteendo commented on Mar 28, 2024

    @nineteendo
    ContributorAuthor

    I'm currently on my Windows laptop, so I can't check.
    In any case, the old implementation can never return //foo because path is set to os.sep.
    Is there a "needs investigation" label?

  3. barneygale commented on Mar 28, 2024

    @barneygale
    Contributor

    There doesn't seem to be anything like a "needs investigation" label.

    Happy to leave this issue open for a bit while you further research a repro case, but if we can't find one, then I think this issue should be closed as "can't repro".

  4. nineteendo commented on Mar 28, 2024

    @nineteendo
    ContributorAuthor

    Also, the current behaviour is undocumented:

    Join two paths, normalizing and eliminating any symbolic links encountered in the second path.

    Normalizing doesn't touch the double slash & // is not a symlink.

  5. barneygale commented on Mar 28, 2024

    @barneygale
    Contributor

    You're quoting a comment above the internal _joinrealpath() function.

    Both the posixpath.realpath() docstring, and the online docs, describe it as returning a canonical path. The online docs add:

    This function emulates the operating system’s procedure for making a path canonical

  6. nineteendo commented on Mar 28, 2024

    @nineteendo
    ContributorAuthor

    OK, but that comment doesn't document this feature, which it probably should, in case this works correctly.

  7. bluss commented on Mar 29, 2024

    @bluss

    Why do you want to fix this, and could you describe in more detail how the changes relate to the specification you are pointing to? I don't see what in the specification requires this.

  8. nineteendo commented on Mar 29, 2024

    @nineteendo
    ContributorAuthor

    Yep, the old behaviour seems to be correct (cd doesn't resolve symlinks), in that case the feature should just be documented and tested.

    • macOS:
      Screenshot 2024-03-29 at 09 04 52
    • Pydroid 3:
      image
  9. changed the title [-]Handle leading `//` for `realpath.commonpath`[/-] [+]Handle leading `//` for `posixpath.realpath`[/+] on Mar 29, 2024
  10. barneygale commented on Mar 29, 2024

    @barneygale
    Contributor

    We don't need to document every detail of what realpath() does. Tests would be welcome, though

  11. nineteendo commented on Mar 29, 2024

    @nineteendo
    ContributorAuthor

    I've added just one sentence in this pull request: #117350.

    Join two paths, normalizing and eliminating any symbolic links encountered in the second path. Two leading slashes are replaced by a single slash.

    Where should I add the test though? It doesn't really fit in with the rest of the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions