[Support][Program] Add findProgramByName(Name, OptionalPaths)

[Bigcheese]

Summary

This adds a replacement for FindProgramByName that accepts an optional list of paths to search. The main intent of this change is to move the code in Clang that searches for tools over to this API so that we correctly find .exe files on Windows.

Followup patches move all users of FindProgramByName over and remove it.

[• rafael]

Seems a reasonable API. I just have a few small questions.

include/llvm/Support/Program.h

69

Why do we need the Optional? Can't we just use an empty Paths as a special value that means "read $PATH"?

lib/Support/Unix/Program.inc

107

Can't we just say that name should not be empty and assert? It looks like a misuse of the API, not an error.

114

Why do you need the extra storage for all cases? Can't this be

if (using $PATH) {

build the array
return  recurse passing the array

}

use the arrayref argument directly

[shankarke]

Testcase ?

lib/Support/Unix/Program.inc

111

StringRef::npos ?

[Bigcheese]

include/llvm/Support/Program.h

69

K.

lib/Support/Unix/Program.inc

107

I was duplicating the behavior of FindProgramByName. I'm not sure if anything relies on the behavior.

111

K.

114

K.

[• rafael]

rafael wrote:

Can't we just say that name should not be empty and assert? It looks like a misuse of the API, not an error.

I was duplicating the behavior of FindProgramByName. I'm not sure if anything relies on the behavior.

Lets assert and find out.

[Bigcheese]

Address review comments.

[• rafael]

lib/Support/Unix/Program.inc

107

assert

116

on windows it is ; instead of :, no.

120

Redundant if. Just let the for iterate 0 times.

130

.c_str() since we have a SmallString anyway.

[Bigcheese]

lib/Support/Unix/Program.inc

107

K.

116

This is UNIX only code.

120

K.

130

K.

[Bigcheese]

Addressed review comments.

[• rafael]

LGTM.

[Eugene.Zelenko]

Committed in r221220.

include/llvm/Support/Program.h

 // This file declares the llvm::sys::Program class.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_PROGRAM_H
 #define LLVM_SUPPORT_PROGRAM_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/Path.h"
 #include <system_error>
 
 namespace llvm {
 namespace sys {
 
   /// This is the OS-specific separator for PATH like environment variables:
   // a colon on Unix or a semicolon on Windows.
#endif
   /// It does not perform hashing as a shell would but instead stats each PATH
   /// entry individually so should generally be avoided. Core LLVM library
   /// functions and options should instead require fully specified paths.
   ///
   /// @returns A string containing the path of the program or an empty string if
   /// the program could not be found.
   std::string FindProgramByName(const std::string& name);
 
+  /// \brief Find the first executable file \p Name in \p Paths.
+  ///
+  /// This does not perform hashing as a shell would but instead stats each PATH
+  /// entry individually so should generally be avoided. Core LLVM library
+  /// functions and options should instead require fully specified paths.
+  ///
+  /// \param Name name of the executable to find. If it contains any system
+  ///   slashes, it will be returned as is.
+  /// \param Paths optional list of paths to search for \p Name. If empty it
+  ///   will use the system PATH environment instead.
+  ///
+  /// \returns The fully qualified path to the first \p Name in \p Paths if it
+  ///   exists. \p Name if \p Name has slashes in it. Otherwise an error.
+  ErrorOr<std::string>
+  findProgramByName(StringRef Name,
+                    ArrayRef<StringRef> Paths = ArrayRef<StringRef>());
+
   // These functions change the specified standard stream (stdin or stdout) to
   // binary mode. They return errc::success if the specified stream
   // was changed. Otherwise a platform dependent error is returned.
   std::error_code ChangeStdinToBinary();
   std::error_code ChangeStdoutToBinary();
 
   /// This function executes the program using the arguments provided.  The
   /// invoked program will inherit the stdin, stdout, and stderr file

lib/Support/Unix/Program.inc

 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only generic UNIX code that
 //===          is guaranteed to work on *all* UNIX variants.
 //===----------------------------------------------------------------------===//
 
 #include "Unix.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include <llvm/Config/config.h>
 #if HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif
 #if HAVE_SYS_RESOURCE_H
while (PathLen) {
     while (*PathStr == ':') {
       PathStr++;
       PathLen--;
     }
   }
   return "";
 }
 
+ErrorOr<std::string> sys::findProgramByName(StringRef Name,
+                                            ArrayRef<StringRef> Paths) {
+  assert(!Name.empty() && "Must have a name!");
+  // Use the given path verbatim if it contains any slashes; this matches
+  // the behavior of sh(1) and friends.
+  if (Name.find('/') != StringRef::npos)
+    return std::string(Name);
+
+  if (Paths.empty()) {
+    SmallVector<StringRef, 16> SearchPaths;
+    SplitString(std::getenv("PATH"), SearchPaths, ":");
+    return findProgramByName(Name, SearchPaths);
+  }
+
+  for (auto Path : Paths) {
+    if (Path.empty())
+      continue;
+
+    // Check to see if this first directory contains the executable...
+    SmallString<128> FilePath(Path);
+    sys::path::append(FilePath, Name);
+    if (sys::fs::can_execute(FilePath.c_str()))
+      return std::string(FilePath.str()); // Found the executable!
+  }
+  return std::errc::no_such_file_or_directory;
+}
+
 static bool RedirectIO(const StringRef *Path, int FD, std::string* ErrMsg) {
   if (!Path) // Noop
     return false;
   std::string File;
   if (Path->empty())
     // Redirect empty paths to /dev/null
     File = "/dev/null";
   else

lib/Support/Windows/Program.inc

 //===- Win32/Program.cpp - Win32 Program Implementation ------- -*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file provides the Win32 specific implementation of the Program class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "WindowsSupport.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/WindowsError.h"
 #include <cstdio>
 #include <fcntl.h>
 #include <io.h>
 #include <malloc.h>
 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only Win32 specific code
 //===          and must not be UNIX code
std::string sys::FindProgramByName(const std::string &progName) {
   buffer.set_size(len);
   SmallVector<char, MAX_PATH> result;
   if (windows::UTF16ToUTF8(buffer.begin(), buffer.size(), result))
     return "";
 
   return std::string(result.data(), result.size());
 }
 
+ErrorOr<std::string> sys::findProgramByName(StringRef Name,
+                                            ArrayRef<StringRef> Paths) {
+  assert(!Name.empty() && "Must have a name!");
+
+  if (Name.find_first_of("/\\") != StringRef::npos)
+    return std::string(Name);
+
+  const char16_t *Path = nullptr;
+  std::u16string PathStorage;
+  if (!Paths.empty()) {
+    PathStorage.reserve(Paths.size() * MAX_PATH);
+    for (int i = 0; i < Paths.size(); ++i) {
+      if (i)
+        PathStorage.push_back(';');
+      StringRef P = Paths[i];
+      SmallVector<wchar_t, MAX_PATH> TmpPath;
+      if (std::error_code EC = windows::UTF8ToUTF16(P, TmpPath))
+        return EC;
+      PathStorage.append(TmpPath.begin(), TmpPath.end());
+    }
+    Path = PathStorage.c_str();
+  }
+
+  SmallVector<wchar_t, MAX_PATH> U16Name;
+  if (std::error_code EC = windows::UTF8ToUTF16(Name, U16Name))
+    return EC;
+
+  SmallVector<StringRef, 12> PathExts;
+  PathExts.push_back("");
+  SplitString(std::getenv("PATHEXT"), PathExts, ";");
+
+  SmallVector<wchar_t, MAX_PATH> U16Result;
+  DWORD Len = MAX_PATH;
+  for (StringRef Ext : PathExts) {
+    SmallVector<wchar_t, MAX_PATH> U16Ext;
+    if (std::error_code EC = windows::UTF8ToUTF16(Ext, U16Ext))
+      return EC;
+
+    do {
+      U16Result.reserve(Len);
+      Len = ::SearchPathW((const wchar_t *)Path, c_str(U16Name),
+                          U16Ext.empty() ? nullptr : c_str(U16Ext),
+                          U16Result.capacity(), U16Result.data(), nullptr);
+    } while (Len > U16Result.capacity());
+
+    if (Len != 0)
+      break; // Found it.
+  }
+
+  if (Len == 0)
+    return mapWindowsError(::GetLastError());
+
+  U16Result.set_size(Len);
+
+  SmallVector<char, MAX_PATH> U8Result;
+  if (std::error_code EC =
+          windows::UTF16ToUTF8(U16Result.data(), U16Result.size(), U8Result))
+    return EC;
+
+  return std::string(U8Result.begin(), U8Result.end());
+}
+
 static HANDLE RedirectIO(const StringRef *path, int fd, std::string* ErrMsg) {
   HANDLE h;
   if (path == 0) {
     if (!DuplicateHandle(GetCurrentProcess(), (HANDLE)_get_osfhandle(fd),
                          GetCurrentProcess(), &h,
                          0, TRUE, DUPLICATE_SAME_ACCESS))
       return INVALID_HANDLE_VALUE;
     return h;