-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: path/filepath: add Resolve, replacing EvalSymlinks #37113
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
Comments
I did not try it, but, I suspect, UNC paths wouldn't work in some situations. For example, can you pass UNC path to os.Chdir? Alex |
I don't know for certain, but judging by a quick Google search, it appears to be possible in Ruby, so I assume one can do that in C-based languages. I'm not a Windows developer, so I'm not a good person to ask about the capabilities of Windows. I'm just a Unix developer trying to make general-purpose software not be terrible on Windows. |
You are correct. I was wrong. os.Chdir does work with UNC paths. Alex |
It is also the case that This also works with C-based programs. |
Hey, Is there any interest in fixing this? Right now, there is no cross-platform way to canonicalize a path in Go. We keep running up against additional cases where the existing behavior doesn't canonicalize paths properly, leading to incompatibility with other programs on the system (notably Git). This necessarily limits the portability of using Go as a cross-platform language. |
By "function to canonicalize paths" do you mean a variation of EvalSymlinks that works on Windows? If so, note that EvalSymlinks is not recommended: #40180 (and probably can't be fixed). Go on Windows has a variety of long-standing filesystem bugs. I suggest using x/sys/windows to call the WinAPI if that solves your problem. |
I mean a function, when given a path, that returns a canonicalized version of that path. In other words, the equivalent to It isn't helpful to me to call the Windows API because (a) I'm not a Windows programmer and have no clue how to use it, (b) it isn't cross-platform, and (c) this is a function that is generally provided by the standard library. |
Go has gaps on Windows; I plug them in my code. You've seen the interest this issue evoked :-p What you need isn't hard. Create a file named yourpkg_windows.go, import "golang.org/x/sys/windows", define GetCanonicalPath() to call Create a file yourpkg_unix.go with a |
First of all, I appreciate that you're trying to help. However, I do feel firmly that this functionality should be in the standard library, since it is in almost every other language, and it is in POSIX. I don't want to carry a lot of platform-specific code in a program because it's difficult to maintain and test, especially when I don't typically develop on Windows. If Go is known to have known defects on Windows, those should be promptly fixed or clearly documented. For many purposes, it's fine if code doesn't run or run well on Windows, but there are some cases where it does. The documentation should clearly and prominently list any limitations with using Go on Windows so that folks can make an informed decision. Last I checked, the Normally, when I find a bug or missing feature, I would send a patch to implement that functionality. However, Go has a CLA, and I don't sign CLAs, so any patch I might submit wouldn't be accepted. If that changes, I'm happy to send a patch to implement this properly if nobody gets to it before me. |
On Unix systems I think the proposed function is the same as |
Yes, I believe that they are identical. |
This proposal should probably also deprecate EvalSymlinks, which is seriously broken on Windows, see #40180 (comment) |
What does "canonical" mean, precisely? |
If there are multiple ways to refer to a filename, the canonical path is the absolute filename which uses no indirections and uses the canonical case (that is, the path component as written to the file system) if the system permits case folding. On Unix, that's the one that contains no symlinks (and, on macOS, uses canonical case and composition). On Windows, there are many ways to have indirection in a path: symlinks, junctions, SUBST, etc. (I don't actually know all of the possible ways, since I almost never use Windows). The canonical form uses none of those indirections and uses the canonical case. Another way to say this is that assuming no hardlinks exist, a file on Unix should have exactly one canonical name whose components are either directories or non-symlink, non-directory (but possibly special) files. |
@networkimprov, you make assertions without being specific about them. I am confused about three of the things you've said related to this issue.
Thanks. |
I mentioned #40180 in #37113 (comment) to suggest that the issue author reconsider canonicalization of paths. I didn't link it again later, but it documents a long list of problems with EvalSymlinks on Windows (which I've now linked). Re path length bugs, other instances of those have been left alone, see #21782 & #36375. And here's a list of Windows bugs that mention "filepath" https://github.com/golang/go/issues?q=is%3Aopen+is%3Aissue+label%3AOS-Windows+filepath |
Russ, I disagree. I don't think we should add filepath.Resolve function that behaves like GetFinalPathNameByHandle Windows API. If someone wants to use this API, we can add golang.org/x/sys/windows.GetFinalPathNameByHandle and they can use it there. I still don't understand what proposed filepath.Resolve function will do. It will be even harder for others, who are just learning this package, to understand the difference between filepath.Resolve and filepath.EvaluateSymlinks. I would like to see documentation for that function before this proposal is even considered. filepath.Resolve is not a good name (like utils). It tells you nothing about what the function does. To me it is a good indication that you yourself do not know what this function does. And what problem will this new function solve? Is filepath.Resolve is supposed to be a replacement (good version) for filepath.EvaluateSymlinks? If yes, let's have someone actually try and replace filepath.EvaluateSymlinks in the current Go tree and see if it still works. If that does not work, then we need to think even harder why we need filepath.Resolve, and not just golang.org/x/sys/windows.GetFinalPathNameByHandle. Thank you for consideration. Alex PS: Sorry I did not comment earlier. But I have been busy with other staff, and missed this thread altogether. |
@alexbrainman all of the questions you raised have been addressed in the comments above. Re missing the thread, you last commented here 7 days ago. |
@ericwj is unable to comment on this issue. As far as we can tell, this is a GitHub bug. It is not due to any intentional action by the Go team. This comment is by @ericwj, sent via e-mail:
|
Yes, I agree that we cannot exactly reproduce this if we switch EvalSymlinks to GetFinalPathNameByHandle. Though I'm not fully sure that the clients of EvalSymlinks really do behave on this implementation detail (that is, unfortunately, documented). What if drop that sentence from the documentation, and just say that EvalSymlinks can either return a relative or an absolute path? Do you have an idea on whether clients really do rely on this specific behavior, even on Linux?
Notice that realpath on Linux doesn't follow mountpoints as well:
Is realpath(3) on Linux different than
So my understanding is that if this proposal is accepted, we end up with:
It looks like the biggest gain here is that we somehow exposes GetFinalPathNameByHandle which is a "common" function used in other frameworks / languages to canonicalize paths. Since we're missing it, we face interoperability problems. In fact, semantically speaking, we're going to have a Resolve function which behaves differently between Windows and Linux wrt mountpoints. And BTW we're also getting into another corner: if somebody puts in the effort of adding mountpoint resolution to Resolve on Linux, we end up with interoperability problems again, because the rest of the world might still just be calling realpath(3) on Linux. A slightly different proposal is to add |
Yes, because bind mounts on Linux are not identical. One directory may be read-only while the other may be read-write. Bind mounts also, by default, don't include submounts and are therefore not necessarily equivalent.
As far as I understand it, these mountpoints are functionally identical. They are more like opaque symlinks rather than the typical Unix mountpoints with different mount options.
This is exactly the behavior that's desired: canonicalizing the path in the way the operating system does that. Languages that use the system C library don't have this problem because they use the system C library functions for this purpose and implement those functions' behavior. I would literally define the expected behavior in terms of the system's C library, since that's what other languages (e.g., Rust) do.
That doesn't really assist in my use case of finding out whether one path is inside another. |
This should mostly be fixed by my CL (https://go-review.googlesource.com/c/go/+/263538). Please test it and let me know. It also rectifies some EvalSymlinks issues on Windows reported in your #40180 table. I have a followup CL (still not mailed) that fixes another set of issues in EvalSymlinks. NOTE: I'm not touching the semantic of the function as it is implemented now: |
There's clearly new, substantive discussion here. Moving back to Active. |
@networkimprov I cannot find them. Can you, please, provide references to me? Thank you. Alex |
To be honest, I disagree on this. The two underlying files are indeed the same and in fact Moreover, the same situation happens on Windows. You can easily have I think the only correct answer here is that the request is not to establish a cross-platform semantic, but just add a function that calls
Looking at git-lfs/git-lfs#4012, it looked like I have one question on this. Say that we have |
This is what Git does and what other projects do, and it's the semantics we need. It is the same semantics that other programming languages provide. I am interested in Go working with other software that is also on the system in a compatible way. You may argue that Windows and Unix should provide the same semantics for path canonicalization, and perhaps they should. However, Microsoft was fully aware of Unix path semantics and behavior, and clearly chose deliberately to do things in an incompatible and much more complicated way. I feel that was a mistake, but we're stuck with it now.
I need to change the directory to the root of the repository to make I also need to be able to determine if a path the user has provided is within the repository and fail if it is not. I can do that by removing the trailing file component, canonicalizing the directory, and appending the file path. That works to tell me if the component is in the repository, whether or not the file itself is a symlink. Repositories are not allowed to span file systems.
I don't need to canonicalize file names, only directory names. However, I do need the standard I want to be clear that I'm not asking for this just for Git LFS. I'm asking for this because these are the standard platform semantics and they should be available in a cross-platform way. I fully realize that they are inconsistent across platforms. I want the same behavior out of Go that I get out of languages that use the system C library. I'm not looking for special functionality to handle my special case, I'm looking for generic tools that make Go programs compatible with non-Go programs. It's fine that Go does not wrap the system C library, but if it chooses not to do that, it needs to be compatible with the semantics of the platform as if it did, and not provide behavior that is materially different. Trying to sell me on a behavior that is consistent across platforms when that is not practically achievable in a useful way isn't helpful. That's what we have now with EvalSymlinks, and everyone agrees that it's not the right decision. |
I want to be clear that while there may be other proposals, such as I also want to be clear that I'm an experienced developer who understands his problem space well and am confident I know what's best for it, so it's unhelpful (and frankly, insulting) to imply that some other solution would be better for my needs or that I should adopt a different design. If people disagree with the Git project's decisions to adopt this technique (with which I am implementing compatibility) and feel that they were unwise, those concerns should be addressed to the project's mailing list, although I suspect that in this case such a message will be about as welcome as I have found it. As I've mentioned, my goal here is to expose a cross-platform way to canonicalize paths as the operating system defines that. Whether the operating system designers made a provident design decision or whether the operating system has provided desirable semantics is outside the scope of this proposal. I assume for the purposes of this proposal that the fact that the operating system is successful means that its developers were competent and implemented desirable, useful, and well-thought-out features. The facts remain that other common, successful languages implement functionally equivalent designs, and that while similar concerns have been expressed during the design of those implementations, nobody has yet proposed a better or more universally desirable solution. Moreover, Go attempted to provide a solution for this problem in For these reasons, I believe that the proposal as it stands should be implemented. I take no position on proposals for other functions, as they are outside my scope. |
Brian, the Windows maintainer has objected to this proposal, raising Q's in #37113 (comment). It might help to point out where Russ or Ian have previously addressed those Q's. |
Looking at how this problem is solved elsewhere, if a new function is to be added, perhaps a better name would be
@ianlancetaylor The original author of this issue has blocked @ericwj because they felt insulted by the help they were providing. When you block a user on GitHub, they're unable to comment on issues/pull requests you're the owner of. |
I do believe people have the right to decide who they'd like to block, assuming they are not in a government position, and that asking people to unblock others is inappropriate. That's often an approach people use to engage in continued harassment of women and other folks who tend to be underrepresented in tech. That's not what's happening here, and I'm sure you had only good intent to move the discussion forward in a productive way, but since we often lack all the context to know people's reasons for blocking others, it's generally best not to ask. For the limited purpose of advancing this proposal, I have unblocked @ericwj for the moment. I'm fine with |
@bk2204 My sincere apologies. I was in the wrong, and I've done you harm. I'm sorry. @ericwj Your comment violates the Go Community Code of Conduct (https://golang.org/conduct), and I've deleted it. |
@networkimprov I assume you refer to me as Thank you. Alex |
Alex, sorry, I had gathered that you're recognized as the principle Windows maintainer. I didn't mean to invent a title. |
Well, I'm sorry for having been slightly edgy due to the total irrelevance of that statement with which he accompanies the news that he unblocked me. I care to repeat that Brian should argue his case from a position of hands on, mature knowledge rather than having us collect that for him and demanding his proposal be accepted unmodified and that none of us is in a better position than to test and evaluate the suitability of including the proposal of porting |
I'm going to close this issue and I'd like to ask the core team to lock it at their earliest convenience. I don't feel this discussion is going in a productive way and I don't wish to pursue this issue or proposal further. If other folks think this a similar proposal will be valuable, I'm happy for them to drive it without my involvement. |
No worries. You did not hurt my feelings or anything. I just did not want other people to get impression that they need to convince me more than others. Any proposal here should be judged purely on its merits, not on authority of proposal author or judges.
I am Go contributor, like many others. I gathered bunch of experience over the years. But that does not make me
No worries. No harm done. Alex |
I have written a new proposal, #42201, to replace this one. I will now lock this issue. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?Applies to all OSes
What did you do?
What did you expect to see?
The same results as calling GetFinalPathNameByHandle: a UNC path.
What did you see instead?
A path using the drive letter instead of the UNC path.
Notes
This affects any attempt to canonicalize paths using the output of Git in such a situation. Git produces some paths as absolute and some paths as relative, and uses GetFinalPathNameByHandle for canonicalizing absolute paths. However, Go lacks a function to canonicalize paths in a standard way, so it isn't possible to produce results equivalent to a C program and still write code that works portably across systems.
Go should add a function that is explicitly defined to canonicalize paths in a way equivalent to the underlying operating system, since using filepath.Abs and filepath.EvalSymlinks doesn't work correctly on Windows. It does work fine on Unix, but Unix paths are much simpler and easier to reason about.
It was determined in #17084 that filepath.Abs and filepath.EvalSymlinks were sufficient in this case, but that doesn't appear to be true. I expect there are other cases in which those don't work on Windows, but I am insufficiently versed in Windows paths to know what those are.
This was originally reported to the Git LFS project in git-lfs/git-lfs#4012.
The text was updated successfully, but these errors were encountered: