> In order to detect loops I need an unique identifier for a file.Isn't os.SameFile useful here?
--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
A program that follows symlinks and recursively goes down
directories can potentially run forever if it can't determine
whether a directory has been visited before. Consider:
ln -s .. foo
On Wed, Jan 14, 2015 at 7:57 AM, Manlio Perillo
<manlio....@gmail.com> wrote:
>
> Is it possible to change the Stat function for Windows, in order to store
> the
> syscall.ByHandleFileInformation instance in the sys field?
To answer this specific question, we can not change the sys field in
this way, because it would break the Go 1 guarantee.
> [...]
Looking at the FindNextFile Win32 API function, it seems that this was necessary since WIN32_FIND_DATA struct does not contain the same data asBY_HANDLE_FILE_INFORMATION struct, and Win32FileAttributeData just happens to contain common data.The only solution is to call lstat, in readdir windows implementation, as it is done for all other systems. It is not possible (IMHO) that Windows gets special treatment resulting in a less usable/portable API.
On Wed, Jan 14, 2015 at 7:57 AM, Manlio Perillo
<manlio....@gmail.com> wrote:
>
> Is it possible to change the Stat function for Windows, in order to store
> the
> syscall.ByHandleFileInformation instance in the sys field?
That said, I'm not sure it makes sense to add WalkFollowingSymlinks to
path/filepath. That seems special purpose to me and I suspect
different programs will have different requirements for unusual
situations. Of course it could be a useful package outside of the
standard library.
On Wed Jan 14 2015 at 4:53:12 PM Bakul Shah <ba...@bitblocks.com> wrote:
A program that follows symlinks and recursively goes down
directories can potentially run forever if it can't determine
whether a directory has been visited before. Consider:
ln -s .. fooIf that program doesn't do the simple thing the kernel does, yes. But if that program does do the simple thing the kernel does, namely, refuse to go more than 'n' hops down a chain of symlinks, problem solved.
Further, I'm advocating that the program do an even simpler thing: stat the path, and if it gets EMLOOP, then the kernel has told you there's an issue with the path.
A better solution (IMHO, of course) is to make the FileInfo returned by Readdir return WIN32_FIND_DATA, and the FileInfo returned by Stat and Lstat return BY_HANDLE_FILE_INFORMATION.This avoids the need to create an intermediate Win32FileAttributeData and, most important, avoids to throw away precious information returned by the operating system.
Hmmm, you're right. I guess that we can change the dynamic type that
the Sys() method returns on Windows without necessarily breaking the
Go 1 guarantee.Why? Changing the dynamic type will break all the code that is using .Sysfield.
> I see. I don't see that the go tool needs a general facility for
> this. It can check for symlinks in the existing calls to Walk
> (fi.Mode()&ModeSymlink != 0) and see if os.Stat on the file returns a
> directory. Then use os.Readlink and carry on with appropriate
> bookkeeping. Keep a map of the names returned by os.Readlink and fail
> if you see the same one more than once.
In particular the go tool doesn't care about loops as such. It cares
about the same directory appearing more than once in GOPATH--that
would make little sense. If we catch that case, we'll catch loops.
WalkFollowingSymlinks can be implemented so that it works as required by the go tools.
What problem are you trying to solve in the go tools?The cmd/go tool itself doesn't handle symlinks in GOPATH, so nor should thetools in the tools sub-repo.
So it should be ok to:
1) Change the dynamic type.
2) Return a different dynamic type based on how the FileInfowas obtained.
This might be OK, but I read your proposal correctly, you want to change the Sys()from a FileInfo obtained through os.Stat.That is problematic. Imagine how people will try to use that Sys() method. If wechanges what Sys() returns in Go 1.5, they will have to handle another type.
Your API proposal already needs to be platform specific, why not just use windowssyscalls and keep the os package as is?
On Sat, Jan 17, 2015 at 1:27 AM, minux <mi...@golang.org> wrote:> [...]
WalkFollowingSymlinks can be implemented so that it works as required by the go tools.
What problem are you trying to solve in the go tools?The cmd/go tool itself doesn't handle symlinks in GOPATH, so nor should thetools in the tools sub-repo.
Since I find symlinks in GOPATH useful I have patched the cmd/go tool to support symlinks on my system. They allow a local version of vanity imports.Of course, as I wrote, I can live without them.
On Sat, Jan 17, 2015 at 1:31 AM, minux <mi...@golang.org> wrote:> [...]So it should be ok to:1) Change the dynamic type.2) Return a different dynamic type based on how the FileInfowas obtained.This might be OK, but I read your proposal correctly, you want to change the Sys()from a FileInfo obtained through os.Stat.That is problematic. Imagine how people will try to use that Sys() method. If wechanges what Sys() returns in Go 1.5, they will have to handle another type.Yes, it is problematic.However go already changed some implementation specific API.
Your API proposal already needs to be platform specific, why not just use windowssyscalls and keep the os package as is?It can be done, of course.But do we really want os.Stat to return incomplete/truncated informations from the OS? Since Go is a system language I find this is not acceptable.
>
> Then it should be easy and efficient to guard against this kind of loop
> using symlinks, in a WalkFollowingSymlinks function.
I'm not yet persuaded that a general WalkFollowingSymlinks function is
meaningful. I think it will have to adopt a specific strategy for
loops that may not be appropriate for all users. Of course I could be
wrong.
Besides, there is no fundamental problems with the current Win32FileAttributeData result.For volume serial number and file index, just use GetFileInformationByHandle.
Actually, as os.SameFile just uses two FileInfo, and do not use any syscalls, so O(N^2)behavior is not that bad. If that is truly inefficient, we might be able to introduce os.File.UniqueKeymethod, but that's another discussion (I don't want it to return interface{}, I think it should returnstring or fill in []byte, or return a Stringer). I imagine net/http might also need it to generateETag for ServeFile, so it's generally useful. Besides, portable interfaces is always betterthan non-portable OS-specific interfaces.
Your API proposal already needs to be platform specific, why not just use windowssyscalls and keep the os package as is?