Hi Sean, I was digging around in FileSystem.h and I noticed we already have `FileSystem::Readlink`. Reading the documentation of `readlink` and `realpath` there seem to be a few minor differences, but they are just that - minor. I'm wondering if you actually *need* any semantics of `realpath` that `readlink` don't provide. If not, maybe we can delete `FileSystem::ResolveSymbolicLink` and use `FileSystem::Readlink` instead. What do you think? On Fri, Sep 18, 2015 at 2:42 PM Zachary Turner <ztur...@google.com> wrote:
> Any time there's something involving windows, even if you're just > #ifdef'ing out a code path, I would prefer if you could wait until I or > someone else who works on Windows has a chance to comment before committing. > > On Fri, Sep 18, 2015 at 2:40 PM Sean Callanan via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > >> Author: spyffe >> Date: Fri Sep 18 16:39:31 2015 >> New Revision: 248048 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=248048&view=rev >> Log: >> Added support for resolving symbolic links to FileSpec. >> >> We use the symbolic link to resolver to find the target of the LLDB shlib >> symlink if there is a symlink. This allows us to find shlib-relative >> resources >> even when running under the testsuite, where _lldb.so is a symlink in the >> Python >> resource directory. >> >> Also changed a comment to be slightly more clear about what resolve_path >> in the >> constructor for FileSpec means, since if we were actually using >> realpath() this >> code wouldn't have been necessary. >> >> http://reviews.llvm.org/D12984 >> >> Modified: >> lldb/trunk/include/lldb/Host/FileSpec.h >> lldb/trunk/source/Host/common/FileSpec.cpp >> lldb/trunk/source/Host/common/HostInfoBase.cpp >> >> Modified: lldb/trunk/include/lldb/Host/FileSpec.h >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=248048&r1=248047&r2=248048&view=diff >> >> ============================================================================== >> --- lldb/trunk/include/lldb/Host/FileSpec.h (original) >> +++ lldb/trunk/include/lldb/Host/FileSpec.h Fri Sep 18 16:39:31 2015 >> @@ -73,7 +73,7 @@ public: >> /// The full or partial path to a file. >> /// >> /// @param[in] resolve_path >> - /// If \b true, then we resolve the path with realpath, >> + /// If \b true, then we resolve the path, removing stray ../.. >> and so forth, >> /// if \b false we trust the path is in canonical form already. >> /// >> /// @see FileSpec::SetFile (const char *path, bool resolve) >> @@ -511,6 +511,9 @@ public: >> >> bool >> IsSymbolicLink () const; >> + >> + FileSpec >> + ResolveSymbolicLink () const; >> >> //------------------------------------------------------------------ >> /// Get the memory cost of this object. >> >> Modified: lldb/trunk/source/Host/common/FileSpec.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=248048&r1=248047&r2=248048&view=diff >> >> ============================================================================== >> --- lldb/trunk/source/Host/common/FileSpec.cpp (original) >> +++ lldb/trunk/source/Host/common/FileSpec.cpp Fri Sep 18 16:39:31 2015 >> @@ -811,6 +811,32 @@ FileSpec::IsSymbolicLink () const >> #endif >> } >> >> +FileSpec >> +FileSpec::ResolveSymbolicLink () const { >> + if (!IsSymbolicLink()) >> + { >> + return *this; >> + } >> + >> + char resolved_path[PATH_MAX]; >> + if (!GetPath (resolved_path, sizeof (resolved_path))) >> + { >> + return *this; >> + } >> + >> +#ifdef _WIN32 >> + return *this; // TODO make this work on win32 >> +#else >> + char real_path[PATH_MAX + 1]; >> + if (realpath(resolved_path, real_path) == nullptr) >> + { >> + return *this; >> + } >> + >> + return FileSpec(real_path, false); >> +#endif >> +} >> + >> uint32_t >> FileSpec::GetPermissions () const >> { >> >> Modified: lldb/trunk/source/Host/common/HostInfoBase.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/HostInfoBase.cpp?rev=248048&r1=248047&r2=248048&view=diff >> >> ============================================================================== >> --- lldb/trunk/source/Host/common/HostInfoBase.cpp (original) >> +++ lldb/trunk/source/Host/common/HostInfoBase.cpp Fri Sep 18 16:39:31 >> 2015 >> @@ -306,7 +306,10 @@ HostInfoBase::ComputeSharedLibraryDirect >> >> FileSpec lldb_file_spec( >> Host::GetModuleFileSpecForHostAddress(reinterpret_cast<void >> *>(reinterpret_cast<intptr_t>(HostInfoBase::GetLLDBPath)))); >> - >> + >> + // This is necessary because when running the testsuite the shlib >> might be a symbolic link inside the Python resource dir. >> + lldb_file_spec = lldb_file_spec.ResolveSymbolicLink(); >> + >> // Remove the filename so that this FileSpec only represents the >> directory. >> file_spec.GetDirectory() = lldb_file_spec.GetDirectory(); >> >> >> >> _______________________________________________ >> lldb-commits mailing list >> lldb-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >> >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
