-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
joinpath should not throw. period. #20912
Comments
More generally, no path manipulation functions that treat paths as strings detached from the file system should throw errors. |
it's a function for manipulating paths, that happens to operate on strings. if the inputs aren't valid paths, then shouldn't be calling joinpath. |
Yes, leaving surprising booby traps for programmers in seemingly innocuous functions – especially ones that only explode on some platforms – is exactly how we like to design our APIs. |
There are two very distinct classes of file/path manipulation functions:
The former should not throw errors – they are fundamentally just data manipulation functions. We already have the example why – an unexpected bug in a function that assumed that calling |
It can't give a correct answer in this corner case. The only correct answer is that your inputs are erroneous. |
It can return an incorrect path which will trigger an error when trying to open the file. That way you only need to catch exceptions in one place rather than two. Anyway a path is generally composed just before being used to access a file. |
It can probably just return the second object. Answering the question: "relative to a relative path B on drive A, what is the full path to the relative path C on drive D", is probably most sensibly "the relative path D on drive C" (with the first clause adding no useful information). So
also, ref #5123 |
Also, fwiw, this interpretation is valid generally on *nix systems (calling |
We should follow Python here since we've followed them on basically everything else path-related. As @nalimilan points out, if it's not a sane answer then opening the file or checking for it will give the appropriate result (error/does not exist). |
Resolved: in the case where |
Should anyone with a Windows machine want to do this, the functional change is this: diff --git a/base/path.jl b/base/path.jl
index c042bef833..bcb04129ee 100644
--- a/base/path.jl
+++ b/base/path.jl
@@ -208,7 +208,7 @@ function joinpath(a::String, b::String)
isabspath(b) && return b
A, a = splitdrive(a)
B, b = splitdrive(b)
- !isempty(B) && A != B && throw(ArgumentError("drive mismatch: $A$a $B$b"))
+ !isempty(B) && A != B && return B
C = isempty(B) ? A : B
isempty(a) ? string(C,b) :
ismatch(path_separator_re, a[end:end]) ? string(C,a,b) : In addition to that, however, it should have tests for different cases in |
As observed in #20695,
joinpath
causes an error on Windows in situations where a git URL looks like a relative path with a drive specification – since the drives of the two arguments don't match, an error is thrown. That specific problem can be worked around, but the deeper problem is thatjoinpath
throws in the first place. It's fundamentally just a string manipulation function and shouldn't throw errors. However, that leaves the question of what should happen when the second argument tojoinpath
is a relative with a drive that doesn't match that of the first argument.The text was updated successfully, but these errors were encountered: