Skip to content

Conversation

belamenso
Copy link
Contributor

Fixes #23646.
This pull introduces two changes in behavior of relpath. All changes and tests should apply only to platforms on which Sys.iswindows() is true.

  1. For relpath(a,b) where a and b are from different Windows drives, abspath(a) is returned.
  2. Paths are still case-sensitive, but not in the drive letter part preventing eg. relpath("C:\\a", "c:\\") == "..\\C:\\a".

@belamenso
Copy link
Contributor Author

Can somebody review this?

@musm
Copy link
Contributor

musm commented Nov 17, 2020

Thanks, this does improve the situation, but I think there are still issues we need to resolve with relpath, which might require bigger changes. See my comments in #23646

@musm
Copy link
Contributor

musm commented Nov 17, 2020

In particular

relpath("d:x","d:a/b/c")  returns "..\\..\\..\\d:x" but should return  "..\\..\\..\\x"      ("d:a/b/c" is a relative path in the current directory in the d drive)

@belamenso
Copy link
Contributor Author

The code should now correctly handle drive prefixes in relative paths, as verified by additional tests. I did not implement throwing ArgumentError for relpath between different drives, in such cases the absolute path of the requested destination is returned. Adding an exception to this method would be a breaking change.

relpath("d:x","d:a/b/c") == "..\\..\\..\\x"
relpath("c:/Users/vavasis/Documents", "E:/") == "c:\\Users\\vavasis\\Documents"
relpath("D","d") == "..\\D"
relpath("d:\\x", "D:\\x") == "."
relpath("c:/Users/vavasis/Documents", "C:/users/vavasis/Documents") == "..\\..\\..\\Users\\vavasis\\Documents"
relpath("c:/Users/vavasis/Documents", "c:/users/vavasis/Documents") == "..\\..\\..\\Users\\vavasis\\Documents"

@musm

@musm
Copy link
Contributor

musm commented Dec 22, 2020

Can you add to the doc string about case insensitive of drive letters on Windows and also that if there's a drive mismatch we return the first path?

@belamenso
Copy link
Contributor Author

Done.

@musm musm added backport 1.6 Change should be backported to release-1.6 system:windows Affects only Windows labels Dec 23, 2020
@musm musm merged commit 46487ae into JuliaLang:master Dec 23, 2020
KristofferC pushed a commit that referenced this pull request Jan 9, 2021
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Feb 1, 2021
KristofferC pushed a commit that referenced this pull request Feb 1, 2021
@musm musm added backport 1.6 Change should be backported to release-1.6 and removed backport 1.6 Change should be backported to release-1.6 labels Mar 25, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system:windows Affects only Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

relpath broken for windows
3 participants