Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Zachary Turner via lldb-commits
zturner accepted this revision. zturner added a comment. Thanks! http://reviews.llvm.org/D12984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Sean Callanan via lldb-commits
spyffe updated this revision to Diff 35144. spyffe added a comment. Restored the old name based on zturner's suggestion that Realpath() is too specific and has semantics that Windows wouldn't honor. http://reviews.llvm.org/D12984 Files: include/lldb/Host/FileSpec.h include/lldb/Host/FileSy

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Zachary Turner via lldb-commits
zturner added a comment. If you change the name back to `ResolveSymbolicLink` or `GetSymbolicLinkTarget`, then this looks fine. Comment at: include/lldb/Host/FileSystem.h:43 @@ -42,1 +42,3 @@ + +static Error Realpath(const FileSpec &src, FileSpec &dst); --

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Sean Callanan via lldb-commits
spyffe added a reviewer: zturner. spyffe removed a subscriber: zturner. spyffe updated this revision to Diff 35143. spyffe added a comment. At zturner's suggestion, moved the function to FileSystem and renamed it to be more consistent with what other FileSystem functions are called. http://revi

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Zachary Turner via lldb-commits
Thanks! Feel free to leave the method in Host/windows/FileSystem.cpp empty, i'll fill it out. On Fri, Sep 18, 2015 at 3:08 PM Sean Callanan wrote: > spyffe added a subscriber: spyffe. > spyffe added a comment. > > Sure. On it. > > Sean > > > http://reviews.llvm.org/D12984 > > > > _

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Sean Callanan via lldb-commits
Sure. On it. Sean > On Sep 18, 2015, at 2:44 PM, Zachary Turner wrote: > > zturner added a comment. > > Furthermore, FileSpec can refer to a remote path, so you can't even > guarantee that the OS you're on is the same OS as that which the path > refers to. So another reason why putting this

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Sean Callanan via lldb-commits
spyffe added a subscriber: spyffe. spyffe added a comment. Sure. On it. Sean http://reviews.llvm.org/D12984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Zachary Turner via lldb-commits
zturner added a comment. Furthermore, FileSpec can refer to a remote path, so you can't even guarantee that the OS you're on is the same OS as that which the path refers to. So another reason why putting this in FileSpec doesn't make sense in my opinion. http://reviews.llvm.org/D12984 __

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Zachary Turner via lldb-commits
Can you move this to FileSystem.h? I don't think we should be adding more things that hit the file system to FileSpec. That's exactly the reason FileSystem.h exists, because many of the operations will be implemented differently across platforms, so we should be using the Host layer. On Fri, Sep

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Greg Clayton via lldb-commits
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D12984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Sean Callanan via lldb-commits
spyffe updated this revision to Diff 35138. spyffe added a comment. Updated to reflect Greg's comments. http://reviews.llvm.org/D12984 Files: include/lldb/Host/FileSpec.h source/Host/common/FileSpec.cpp source/Host/common/HostInfoBase.cpp Index: source/Host/common/HostInfoBase.cpp ==

Re: [Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Greg Clayton via lldb-commits
clayborg requested changes to this revision. This revision now requires changes to proceed. Comment at: include/lldb/Host/FileSpec.h:516 @@ -514,1 +515,3 @@ +FileSpec +GetSymbolicLinkTarget () const; rename to ResolveSymbolicLink? Comme

[Lldb-commits] [PATCH] D12984: Resolve symlinks when looking for the LLDB shlib

2015-09-18 Thread Sean Callanan via lldb-commits
spyffe created this revision. spyffe added a reviewer: clayborg. spyffe added a subscriber: lldb-commits. When running the testsuite, the LLDB shlib is a symlink to its actual location, so finding shlib-relative resources can be problematic. This patch fixes that by resolving symlinks. http://