Add llvm::sys::fs::real_path

[zturner]

Summary

LLVM currently has a mechanism for resolving symlinks and collapsing . and .. items through the openFileForRead method, but it has a couple of limitations:

1. It's not convenient to call, since it requires calling an intermediate function first and interpreting the results, then cleaning up after the function (by closing the file descriptor). It would be nice to just be able to say real_path(Path, Real);
2. It requires an open file descriptor, which means that particularly on Windows, there's currently no way to get the real path of a directory ("opening" a directory and opening a file require different code, and we don't expose a method to do the former)
3. On non-Windows platforms, we can't currently handle tilde expressions.

This patch addresses all of this by providing a simple method with the signature std::error_code real_path(const Twine &, SmallVectorImpl<char> &). It works even with directories on Windows, and on Posix platforms it is smart enough to translate ~ into the appropriate home directory.

As a result, real_path(X) works like you would expect it to from a bash shell on Linux/Mac/etc, and real_path(X) works with directories on Windows, providing maximum portability.

[amccarth]

I can see why it's probably not feasible to test realPathFromHandle and directoryRealPath directly, but is there a way to test real_path to ensure that it covers a lot of the important cases (like MAX_PATH on Windows)?

llvm/lib/Support/Unix/Path.inc

483

Isn't the style expandTildeExpr?

843

realPath?

844

I would move the definition of fd down near the definition of EC so that it's not live before it needs to be.

853

s/then/them/

llvm/lib/Support/Windows/Path.inc

785

Maybe this should be larger than MAX_PATH because it will have the \\?\ prefix.

786

Reading MSDN, it looks like the length of the buffer should be reported as MAX_PATH - 1, otherwise a path that's exactly the maximum length might result in a one character overrun.

787

GetFinalPathNameByHandle is Vista+, no XP. I assume that's OK.

797

Looks like this silently fails for paths longer than MAX_PATH. Is that a limitation we're already facing in the rest of path support in LLVM? Should this return an error in that case?

807

Would PathUTF16.data() be more appropriate than .begin()?

812

If realPathFromHandle could return an error in the long name case, then you could propagate the error out here.

[zturner]

In D30668#693823, @amccarth wrote:

I can see why it's probably not feasible to test realPathFromHandle and directoryRealPath directly, but is there a way to test real_path to ensure that it covers a lot of the important cases (like MAX_PATH on Windows)?

I actually tried adding some tests for real_path, but it's not that easy. You need a way to generate a symbolic link, and creating a symbolic link requires SeCreateSymbolicLink privilege. Because of this, llvm::sys::fs::create_link actually creates a hard link instead of a symbolic link, and when you make a file B which is a hardlink to A, can call real_path(B) it will just return B, not A (as expected).

I could probably add a test for . and .. syntax, but I don't see a way to write one for symbolic link resolution.

[labath]

I am not sure whether having things work "exactly like from a shell" is a feature (*), as there are other utilities doing similar things which don't do tilde expansion. Maybe I am being too pedantic, but I think it's a point worth bringing up.

(*) The rules for tilde expansion in posix shells are also quite tricky. E.g. ~/foo expands ~, but "~/foo" does not (but "$HOME/foo" expands $HOME, ...).

llvm/lib/Support/Unix/Path.inc

851

I am wondering whether the tilde expansion should not be controlled by an argument. As the comment says, tilde expansion is done at a completely different level than path canonicalization. ~ is a perfectly valid path component and it's not clear to me that every user of this function would want the magic tilde expansion behavior as well. I think that can create some confusion, as this is not the behavior of any of the standard utilities and functions elsewhere (realpath unix utility, realpath POSIX function, llvm's openFileForRead, ...). For that reason it may be safer for the caller to explicitly opt-in into this behavior.

[zturner]

Makes tilde expansion opt-in, and adds a test for . and .. collapsing. shell_expand does nothing currently on Windows, but we could make it expand environment variables in theory.

[rnk]

llvm/lib/Support/Unix/Path.inc

851

Does the tilde expansion work on Windows? If not, let's leave it out like Pavel suggests.

[zturner]

llvm/lib/Support/Unix/Path.inc

851

No, but I thought he just suggested to make it opt-in rather than leave it out entirely. The "equivalent" thing on Windows would be to expand environment variables. That said, maybe there's a case to be made for allowing tilde expansion on Windows too. I mean, Windows users do have a home directory, so it's not like it doesn't make sense at all.

[labath]

llvm/include/llvm/Support/FileSystem.h

347

I think this description is very vague. After reading this, I would expect we will handle all kinds of fancy shell expansions (${HOME//foo} anyone ?). I suggest name the argument tilde_expand, document it as performing home-directory expansion using ~ and nothing more, until we have a very good idea of what other kinds of expansions we may want to perform.

llvm/lib/Support/Unix/Path.inc

851

Yes, my suggestion was to make this opt-in. I can see a case for making ~-expansion a separate function, but that may then be clunky to use for cases when you want both, and we will probably end up with a wrapper function that does both sooner or later.

PS: I like the idea of ~ working on windows as well (we just shouldn't be pretending to emulate any specific shell then).

[jhenderson]

llvm/lib/Support/Windows/Path.inc

787

I had a similar question earlier, but I noticed that we don't use the XP toolset in the MSVC build, so I'm guessing the LLVM tools are not supported on XP. Haven't investigated the official policy though.

[zturner]

Support tilde expressions on Windows.

[zturner]

Add a better comment to the function's declarations.

[mgorny]

I'm sorry but what is the exact use case for this? I'm not exactly against having a function like this but 'realpath' logic is the kind of thing people frequently abuse, so I'd rather make sure we don't end up accidentally replacing 'canonical' paths provided by the user with 'real' paths.

[labath]

looks good to me, thank you for the patience. BTW I accidentally stumbled onto this https://docs.python.org/2/library/os.path.html#os.path.expanduser yesterday, so it looks like supporting tilde expressions on windows is not without precedent.

@mgorny: The immediate motivation is lowering similar code for this in lldb. I am not sure what are all the reasons lldb needs it in the first place, but I can totally believe that it is overusing this (OTOH, lldb only consumes paths, it does not produce them, so it's probably not that critical). Still, maybe this is another reason to have ~ logic separate from real_path stuff -- then one could still support home directory expansion where it makes sense, without forcing full path resolution.

[zturner]

Fixed a few edge cases and removed a few copies, making things slightly more efficient.

Also, I responded yesterday but for some reason I don't see it showing up on the review. So just to make sure everyone got it: @mgorny LLVM already has realpath-like functionality built in, it's just cumbersome to use. You can do it by calling openFileForRead and passing an optional output parameter. A search of the codebase shows that a few clients are already using this (mostly in clang). Unfortunately though, it doesn't work for directories on Windows. And as mentioned, it's a clunky interface if the *only* thing you want is the path.

In any case, I'll leave this up for another day in case anyone has further comments.

[amccarth]

Most of my comments on llvm/lib/Support/Windows/Path.inc still seem unaddressed. Are you not seeing them? The rest looks good.

[zturner]

I think this should address all of Adrian's comments. I've left real_path as is instead of realPath since I didn't want to break consistency with all of the other path and file system functions like home_directory etc, but I've made the internal private functions using camel case notations.

[amccarth]

Thanks for the changes. LGTM.

llvm/include/llvm/Support/FileSystem.h

 /// Create a hard link from \a from to \a to, or return an error.
 ///
 /// @param to The path to hard link to.
 /// @param from The path to hard link from. This is created.
 /// @returns errc::success if the link was created, otherwise a platform
 /// specific error_code.
 std::error_code create_hard_link(const Twine &to, const Twine &from);
 
+/// @brief Collapse all . and .. patterns, resolve all symlinks, and optionally
+///        expand ~ expressions to the user's home directory.
+///
+/// @param path The path to resolve.
+/// @param output The location to store the resolved path.
+/// @param expand_tilde If true, resolves ~ expressions to the user's home
+///                     directory.
+std::error_code real_path(const Twine &path, SmallVectorImpl<char> &output,
+                          bool expand_tilde = false);
+
 /// @brief Get the current path.
 ///
 /// @param result Holds the current path on return.
 /// @returns errc::success if the current path has been stored in result,
 ///          otherwise a platform-specific error_code.
 std::error_code current_path(SmallVectorImpl<char> &result);
 
 /// @brief Set the current path.

llvm/lib/Support/Unix/Path.inc

 # if HAVE_SYS_DIR_H
 #  include <sys/dir.h>
 # endif
 # if HAVE_NDIR_H
 #  include <ndir.h>
 # endif
 #endif
 
+#include <pwd.h>
+
 #ifdef __APPLE__
 #include <mach-o/dyld.h>
 #include <sys/attr.h>
 #endif
 
 // Both stdio.h and cstdio are included via different paths and
 // stdcxx's cstdio doesn't include stdio.h, so it doesn't #undef the macros
 // either.
std::error_code equivalent(const Twine &A, const Twine &B, bool &result) {
   if (std::error_code ec = status(A, fsA))
     return ec;
   if (std::error_code ec = status(B, fsB))
     return ec;
   result = equivalent(fsA, fsB);
   return std::error_code();
 }
 
+static void expandTildeExpr(SmallVectorImpl<char> &Path) {
+  StringRef PathStr(Path.begin(), Path.size());
+  if (PathStr.empty() || !PathStr.startswith("~"))
+    return;
+
+  PathStr = PathStr.drop_front();
+  StringRef Expr = PathStr.take_until(path::is_separator);
+  StringRef Remainder = PathStr.substr(Expr.size() + 1);
+  SmallString<128> Storage;
+  if (Expr.empty()) {
+    // This is just ~/..., resolve it to the current user's home dir.
+    if (!path::home_directory(Storage)) {
+      // For some reason we couldn't get the home directory.  Just exit.
+      return;
+    }
+
+    // Overwrite the first character and insert the rest.
+    Path[0] = Storage[0];
+    Path.insert(Path.begin() + 1, Storage.begin() + 1, Storage.end());
+    return;
+  }
+
+  // This is a string of the form ~username/, look up this user's entry in the
+  // password database.
+  struct passwd *Entry = nullptr;
+  std::string User = Expr.str();
+  Entry = ::getpwnam(User.c_str());
+
+  if (!Entry) {
+    // Unable to look up the entry, just return back the original path.
+    return;
+  }
+
+  Storage = Remainder;
+  Path.clear();
+  Path.append(Entry->pw_dir, Entry->pw_dir + strlen(Entry->pw_dir));
+  llvm::sys::path::append(Path, Storage);
+}
+
 static std::error_code fillStatus(int StatRet, const struct stat &Status,
                              file_status &Result) {
   if (StatRet != 0) {
     std::error_code ec(errno, std::generic_category());
     if (ec == errc::no_such_file_or_directory)
       Result = file_status(file_type::file_not_found);
     else
       Result = file_status(file_type::status_error);
if (static_cast<size_t>(CharCount) == ResultPath.capacity()) {
     if (CharCount > sb.st_size)
       return std::error_code(ENAMETOOLONG, std::generic_category());
   }
   ResultPath.set_size(static_cast<size_t>(CharCount));
 #endif
   return std::error_code();
 }
 
 template <typename T>
 static std::error_code remove_directories_impl(const T &Entry,
                                                bool IgnoreErrors) {
   std::error_code EC;
   directory_iterator Begin(Entry, EC, false);
   directory_iterator End;
   while (Begin != End) {
     auto &Item = *Begin;
     file_status st;
     EC = Item.status(st);
     if (EC && !IgnoreErrors)
       return EC;
 
     if (is_directory(st)) {
       EC = remove_directories_impl(Item, IgnoreErrors);
       if (EC && !IgnoreErrors)
         return EC;
     }
 
std::error_code remove_directories(const Twine &path, bool IgnoreErrors) {
   if (EC && !IgnoreErrors)
     return EC;
   EC = fs::remove(path, true);
   if (EC && !IgnoreErrors)
     return EC;
   return std::error_code();
 }
 
+std::error_code real_path(const Twine &path, SmallVectorImpl<char> &dest,
+                          bool expand_tilde) {
+  dest.clear();
+  if (path.isTriviallyEmpty())
+    return std::error_code();
+
+  if (expand_tilde) {
+    SmallString<128> Storage;
+    path.toVector(Storage);
+    expandTildeExpr(Storage);
+    return real_path(Storage, dest, false);
+  }
+
+  int fd;
+  std::error_code EC = openFileForRead(path, fd, &dest);
+
+  if (EC)
+    return EC;
+  ::close(fd);
+  return std::error_code();
+}
+
 } // end namespace fs
 
 namespace path {
 
 bool home_directory(SmallVectorImpl<char> &result) {
   if (char *RequestedDir = getenv("HOME")) {
     result.clear();
     result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
     return true;
   }
-
   return false;
 }
 
 static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char> &Result) {
   #if defined(_CS_DARWIN_USER_TEMP_DIR) && defined(_CS_DARWIN_USER_CACHE_DIR)
   // On Darwin, use DARWIN_USER_TEMP_DIR or DARWIN_USER_CACHE_DIR.
   // macros defined in <unistd.h> on darwin >= 9
   int ConfName = TempDir ? _CS_DARWIN_USER_TEMP_DIR

llvm/lib/Support/Windows/Path.inc

if (std::error_code ec =
           UTF16ToUTF8(FindData.cFileName, ::wcslen(FindData.cFileName),
                       directory_entry_path_utf8))
     return ec;
 
   it.CurrentEntry.replace_filename(Twine(directory_entry_path_utf8));
   return std::error_code();
 }
 
+static std::error_code realPathFromHandle(HANDLE H,
+                                          SmallVectorImpl<char> &RealPath) {
+  RealPath.clear();
+  llvm::SmallVector<wchar_t, MAX_PATH> Buffer;
+  DWORD CountChars = ::GetFinalPathNameByHandleW(
+      H, Buffer.begin(), Buffer.capacity() - 1, FILE_NAME_NORMALIZED);
+  if (CountChars > Buffer.capacity()) {
+    // The buffer wasn't big enough, try again.  In this case the return value
+    // *does* indicate the size of the null terminator.
+    Buffer.reserve(CountChars);
+    CountChars = ::GetFinalPathNameByHandleW(
+        H, Buffer.data(), Buffer.capacity() - 1, FILE_NAME_NORMALIZED);
+  }
+  if (CountChars == 0)
+    return mapWindowsError(GetLastError());
+
+  const wchar_t *Data = Buffer.data();
+  if (CountChars >= 4) {
+    if (0 == ::memcmp(Data, L"\\\\?\\", 8)) {
+      CountChars -= 4;
+      Data += 4;
+    }
+  }
+
+  // Convert the result from UTF-16 to UTF-8.
+  return UTF16ToUTF8(Data, CountChars, RealPath);
+}
+
+static std::error_code directoryRealPath(const Twine &Name,
+                                         SmallVectorImpl<char> &RealPath) {
+  SmallVector<wchar_t, 128> PathUTF16;
+
+  if (std::error_code EC = widenPath(Name, PathUTF16))
+    return EC;
+
+  HANDLE H =
+      ::CreateFileW(PathUTF16.begin(), GENERIC_READ,
+                    FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+                    NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
+  if (H == INVALID_HANDLE_VALUE)
+    return mapWindowsError(GetLastError());
+  std::error_code EC = realPathFromHandle(H, RealPath);
+  ::CloseHandle(H);
+  return EC;
+}
+
 std::error_code openFileForRead(const Twine &Name, int &ResultFD,
                                 SmallVectorImpl<char> *RealPath) {
   SmallVector<wchar_t, 128> PathUTF16;
 
   if (std::error_code EC = widenPath(Name, PathUTF16))
     return EC;
 
   HANDLE H =
std::error_code openFileForRead(const Twine &Name, int &ResultFD,
 
   int FD = ::_open_osfhandle(intptr_t(H), 0);
   if (FD == -1) {
     ::CloseHandle(H);
     return mapWindowsError(ERROR_INVALID_HANDLE);
   }
 
   // Fetch the real name of the file, if the user asked
-  if (RealPath) {
-    RealPath->clear();
-    wchar_t RealPathUTF16[MAX_PATH];
-    DWORD CountChars =
-      ::GetFinalPathNameByHandleW(H, RealPathUTF16, MAX_PATH,
-                                  FILE_NAME_NORMALIZED);
-    if (CountChars > 0 && CountChars < MAX_PATH) {
-      // Convert the result from UTF-16 to UTF-8.
-      SmallString<MAX_PATH> RealPathUTF8;
-      if (!UTF16ToUTF8(RealPathUTF16, CountChars, RealPathUTF8))
-        RealPath->append(RealPathUTF8.data(),
-                         RealPathUTF8.data() + strlen(RealPathUTF8.data()));
-    }
-  }
+  std::error_code EC;
+  if (RealPath)
+    EC = realPathFromHandle(H, *RealPath);
 
   ResultFD = FD;
-  return std::error_code();
+  return EC;
 }
 
 std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
                             sys::fs::OpenFlags Flags, unsigned Mode) {
   // Verify that we don't have both "append" and "excl".
   assert((!(Flags & sys::fs::F_Excl) || !(Flags & sys::fs::F_Append)) &&
          "Cannot specify both 'excl' and 'append' file creation flags!");
 
std::error_code remove_directories(const Twine &path, bool IgnoreErrors) {
   shfos.fFlags = FOF_NO_UI;
 
   int result = ::SHFileOperationW(&shfos);
   if (result != 0 && !IgnoreErrors)
     return mapWindowsError(result);
   return std::error_code();
 }
 
+static void expandTildeExpr(SmallVectorImpl<char> &Path) {
+  // Path does not begin with a tilde expression.
+  if (Path.empty() || Path[0] != '~')
+    return;
+
+  StringRef PathStr(Path.begin(), Path.size());
+  PathStr = PathStr.drop_front();
+  StringRef Expr = PathStr.take_until(path::is_separator);
+
+  if (!Expr.empty()) {
+    // This is probably a ~username/ expression.  Don't support this on Windows.
+    return;
+  }
+
+  SmallString<128> HomeDir;
+  if (!path::home_directory(HomeDir)) {
+    // For some reason we couldn't get the home directory.  Just exit.
+    return;
+  }
+
+  // Overwrite the first character and insert the rest.
+  Path[0] = HomeDir[0];
+  Path.insert(Path.begin() + 1, HomeDir.begin() + 1, HomeDir.end());
+}
+
+std::error_code real_path(const Twine &path, SmallVectorImpl<char> &dest,
+                          bool expand_tilde) {
+  dest.clear();
+  if (path.isTriviallyEmpty())
+    return std::error_code();
+
+  if (expand_tilde) {
+    SmallString<128> Storage;
+    path.toVector(Storage);
+    expandTildeExpr(Storage);
+    return real_path(Storage, dest, false);
+  }
+
+  if (is_directory(path))
+    return directoryRealPath(path, dest);
+
+  int fd;
+  if (std::error_code EC = llvm::sys::fs::openFileForRead(path, fd, &dest))
+    return EC;
+  ::close(fd);
+  return std::error_code();
+}
+
 } // end namespace fs
 
 namespace path {
 static bool getKnownFolderPath(KNOWNFOLDERID folderId,
                                SmallVectorImpl<char> &result) {
   wchar_t *path = nullptr;
   if (::SHGetKnownFolderPath(folderId, KF_FLAG_CREATE, nullptr, &path) != S_OK)
     return false;

llvm/unittests/Support/Path.cpp

TEST_F(FileSystemTest, Unique) {
   ASSERT_NO_ERROR(fs::getUniqueID(Dir2.c_str(), F2));
   ASSERT_NE(F1, F2);
   ASSERT_NO_ERROR(fs::remove(Dir1));
   ASSERT_NO_ERROR(fs::remove(Dir2));
   ASSERT_NO_ERROR(fs::remove(TempPath2));
   ASSERT_NO_ERROR(fs::remove(TempPath));
 }
 
+TEST_F(FileSystemTest, RealPath) {
+  ASSERT_NO_ERROR(
+      fs::create_directories(Twine(TestDirectory) + "/test1/test2/test3"));
+  ASSERT_TRUE(fs::exists(Twine(TestDirectory) + "/test1/test2/test3"));
+
+  SmallString<64> RealBase;
+  SmallString<64> Expected;
+  SmallString<64> Actual;
+
+  // TestDirectory itself might be under a symlink or have been specified with
+  // a different case than the existing temp directory.  In such cases real_path
+  // on the concatenated path will differ in the TestDirectory portion from
+  // how we specified it.  Make sure to compare against the real_path of the
+  // TestDirectory, and not just the value of TestDirectory.
+  ASSERT_NO_ERROR(fs::real_path(TestDirectory, RealBase));
+  path::native(Twine(RealBase) + "/test1/test2", Expected);
+
+  ASSERT_NO_ERROR(fs::real_path(
+      Twine(TestDirectory) + "/././test1/../test1/test2/./test3/..", Actual));
+
+  EXPECT_EQ(Expected, Actual);
+
+  SmallString<64> HomeDir;
+  ASSERT_TRUE(llvm::sys::path::home_directory(HomeDir));
+  ASSERT_NO_ERROR(fs::real_path(HomeDir, Expected));
+  ASSERT_NO_ERROR(fs::real_path("~", Actual, true));
+  EXPECT_EQ(Expected, Actual);
+  ASSERT_NO_ERROR(fs::real_path("~/", Actual, true));
+  EXPECT_EQ(Expected, Actual);
+
+  fs::real_path(Twine(TestDirectory) + "/does_not_exist", Actual);
+  EXPECT_EQ("", Actual);
+
+  ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory) + "/test1"));
+}
+
 TEST_F(FileSystemTest, TempFiles) {
   // Create a temp file.
   int FileDescriptor;
   SmallString<64> TempPath;
   ASSERT_NO_ERROR(
       fs::createTemporaryFile("prefix", "temp", FileDescriptor, TempPath));
 
   // Make sure it exists.