[FileSystem] Remove ResolveExecutableLocation() from FileSpec

[JDevlieghere]

Summary

This patch removes the ResolveExecutableLocation method from FileSpec and updates its uses with calls to the FileSystem.

[labath]

Why did you change the function name? I think it would be nice to keep "executable" or something to that effect in the name (e.g. the llvm function has "program"), given that the underlying function checks for the executable bit, and so it cannot be used for general searches.

[JDevlieghere]

In D53834#1281822, @labath wrote:

Why did you change the function name? I think it would be nice to keep "executable" or something to that effect in the name (e.g. the llvm function has "program"), given that the underlying function checks for the executable bit, and so it cannot be used for general searches.

Yep you're right, this wasn't clear from the doxygen but I should've checked the llvm implementation.

[davide]

LGTM, thanks.

include/lldb/Host/FileSystem.h

public:
   /// @}
 
   /// Resolve path to make it canonical.
   /// @{
   void Resolve(llvm::SmallVectorImpl<char> &path);
   void Resolve(FileSpec &file_spec);
   /// @}
 
+  /// Call into the Host to see if it can help find the file.
+  bool ResolveExecutableLocation(FileSpec &file_spec);
+
   enum EnumerateDirectoryResult {
     /// Enumerate next entry in the current directory.
     eEnumerateDirectoryResultNext,
     /// Recurse into the current entry if it is a directory or symlink, or next
     /// if not.
     eEnumerateDirectoryResultEnter,
     /// Stop directory enumerations at any level.
     eEnumerateDirectoryResultQuit

include/lldb/Utility/FileSpec.h

public:
   /// Existence test.
   ///
   /// @return
   ///     \b true if the file exists on disk, \b false otherwise.
   //------------------------------------------------------------------
   bool Exists() const;
 
   //------------------------------------------------------------------
-  /// Expanded existence test.
-  ///
-  /// Call into the Host to see if it can help find the file (e.g. by
-  /// searching paths set in the environment, etc.).
-  ///
-  /// If found, sets the value of m_directory to the directory where the file
-  /// was found.
-  ///
-  /// @return
-  ///     \b true if was able to find the file using expanded search
-  ///     methods, \b false otherwise.
-  //------------------------------------------------------------------
-  bool ResolveExecutableLocation();
-
-  //------------------------------------------------------------------
   /// Canonicalize this file path (basically running the static
   /// FileSpec::Resolve method on it). Useful if you asked us not to resolve
   /// the file path when you set the file.
   //------------------------------------------------------------------
   bool ResolvePath();
 
   Style GetPathStyle() const;
 

source/API/SBFileSpec.cpp

 //===-- SBFileSpec.cpp ------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include <inttypes.h> // PRIu64
 #include <limits.h>
 
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/PosixApi.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/SmallString.h"
 
 using namespace lldb;
if (log)
     log->Printf("SBFileSpec(%p)::Exists () => %s",
                 static_cast<void *>(m_opaque_ap.get()),
                 (result ? "true" : "false"));
 
   return result;
 }
 
 bool SBFileSpec::ResolveExecutableLocation() {
-  return m_opaque_ap->ResolveExecutableLocation();
+  return FileSystem::Instance().ResolveExecutableLocation(*m_opaque_ap);
 }
 
 int SBFileSpec::ResolvePath(const char *src_path, char *dst_path,
                             size_t dst_len) {
   llvm::SmallString<64> result(src_path);
   lldb_private::FileSpec::Resolve(result);
   ::snprintf(dst_path, dst_len, "%s", result.c_str());
   return std::min(dst_len - 1, result.size());

source/Host/common/FileSystem.cpp

 //===-- FileSystem.cpp ------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/FileSystem.h"
 
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/TildeExpressionResolver.h"
 
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/Threading.h"
 
 #include <algorithm>
 #include <fstream>
 #include <vector>
 
 using namespace lldb;
 using namespace lldb_private;
void FileSystem::Resolve(FileSpec &file_spec) {
   file_spec.GetPath(path);
 
   // Resolve the path.
   Resolve(path);
 
   // Update the FileSpec with the resolved path.
   file_spec.SetPath(path);
 }
+
+bool FileSystem::ResolveExecutableLocation(FileSpec &file_spec) {
+  // If the directory is set there's nothing to do.
+  const ConstString &directory = file_spec.GetDirectory();
+  if (directory)
+    return false;
+
+  // We cannot look for a file if there's no file name.
+  const ConstString &filename = file_spec.GetFilename();
+  if (!filename)
+    return false;
+
+  // Search for the file on the host.
+  const std::string filename_str(filename.GetCString());
+  llvm::ErrorOr<std::string> error_or_path =
+      llvm::sys::findProgramByName(filename_str);
+  if (!error_or_path)
+    return false;
+
+  // findProgramByName returns "." if it can't find the file.
+  llvm::StringRef path = *error_or_path;
+  llvm::StringRef parent = llvm::sys::path::parent_path(path);
+  if (parent.empty() || parent == ".")
+    return false;
+
+  // Make sure that the result exists.
+  FileSpec result(*error_or_path, false);
+  if (!Exists(result))
+    return false;
+
+  file_spec = result;
+  return true;
+}

source/Host/common/MonitoringProcessLauncher.cpp

 //===-- MonitoringProcessLauncher.cpp ---------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/MonitoringProcessLauncher.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Status.h"
 
 #include "llvm/Support/FileSystem.h"
 
 using namespace lldb;
MonitoringProcessLauncher::LaunchProcess(const ProcessLaunchInfo &launch_info,
 
   llvm::sys::fs::file_status stats;
   status(exe_spec.GetPath(), stats);
   if (!exists(stats)) {
     exe_spec.ResolvePath();
     status(exe_spec.GetPath(), stats);
   }
   if (!exists(stats)) {
-    exe_spec.ResolveExecutableLocation();
+    FileSystem::Instance().ResolveExecutableLocation(exe_spec);
     status(exe_spec.GetPath(), stats);
   }
 
   if (!exists(stats)) {
     error.SetErrorStringWithFormatv("executable doesn't exist: '{0}'",
                                     exe_spec);
     return HostProcess();
   }

source/Host/macosx/objcxx/Host.mm

 #include <stdlib.h>
 #include <sys/proc.h>
 #include <sys/stat.h>
 #include <sys/sysctl.h>
 #include <sys/types.h>
 #include <unistd.h>
 
 #include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/ThreadLauncher.h"
-#include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/NameMatches.h"
Status Host::LaunchProcess(ProcessLaunchInfo &launch_info) {
 
   llvm::sys::fs::file_status stats;
   status(exe_spec.GetPath(), stats);
   if (!exists(stats)) {
     exe_spec.ResolvePath();
     status(exe_spec.GetPath(), stats);
   }
   if (!exists(stats)) {
-    exe_spec.ResolveExecutableLocation();
+    FileSystem::Instance().ResolveExecutableLocation(exe_spec);
     status(exe_spec.GetPath(), stats);
   }
   if (!exists(stats)) {
     error.SetErrorStringWithFormatv("executable doesn't exist: '{0}'",
                                     launch_info.GetExecutableFile());
     return error;
   }
 

source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

if (IsHost()) {
     // on the current path variables
     if (!resolved_module_spec.GetFileSpec().Exists()) {
       resolved_module_spec.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
       resolved_module_spec.GetFileSpec().SetFile(exe_path, true,
                                                  FileSpec::Style::native);
     }
 
     if (!resolved_module_spec.GetFileSpec().Exists())
-      resolved_module_spec.GetFileSpec().ResolveExecutableLocation();
+      FileSystem::Instance().ResolveExecutableLocation(
+          resolved_module_spec.GetFileSpec());
 
     // Resolve any executable within a bundle on MacOSX
     Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
 
     if (resolved_module_spec.GetFileSpec().Exists())
       error.Clear();
     else {
       const uint32_t permissions = FileSystem::Instance().GetPermissions(

source/Plugins/Platform/Windows/PlatformWindows.cpp

if (IsHost()) {
     // variables
     if (!resolved_module_spec.GetFileSpec().Exists()) {
       resolved_module_spec.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
       resolved_module_spec.GetFileSpec().SetFile(exe_path, true,
                                                  FileSpec::Style::native);
     }
 
     if (!resolved_module_spec.GetFileSpec().Exists())
-      resolved_module_spec.GetFileSpec().ResolveExecutableLocation();
+      FileSystem::Instance().ResolveExecutableLocation(
+          resolved_module_spec.GetFileSpec());
 
     if (resolved_module_spec.GetFileSpec().Exists())
       error.Clear();
     else {
       ms.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
       error.SetErrorStringWithFormat("unable to find executable for '%s'",
                                      exe_path);
     }

source/Target/ProcessLaunchInfo.cpp

void ProcessLaunchInfo::SetProcessPluginName(llvm::StringRef plugin) {
   m_plugin_name = plugin;
 }
 
 const FileSpec &ProcessLaunchInfo::GetShell() const { return m_shell; }
 
 void ProcessLaunchInfo::SetShell(const FileSpec &shell) {
   m_shell = shell;
   if (m_shell) {
-    m_shell.ResolveExecutableLocation();
+    FileSystem::Instance().ResolveExecutableLocation(m_shell);
     m_flags.Set(lldb::eLaunchFlagLaunchInShell);
   } else
     m_flags.Clear(lldb::eLaunchFlagLaunchInShell);
 }
 
 void ProcessLaunchInfo::SetLaunchInSeparateProcessGroup(bool separate) {
   if (separate)
     m_flags.Set(lldb::eLaunchFlagLaunchInSeparateProcessGroup);

source/Utility/FileSpec.cpp

void FileSpec::Dump(Stream *s) const {
   }
 }
 
 //------------------------------------------------------------------
 // Returns true if the file exists.
 //------------------------------------------------------------------
 bool FileSpec::Exists() const { return llvm::sys::fs::exists(GetPath()); }
 
-bool FileSpec::ResolveExecutableLocation() {
-  // CLEANUP: Use StringRef for string handling.
-  if (!m_directory) {
-    const char *file_cstr = m_filename.GetCString();
-    if (file_cstr) {
-      const std::string file_str(file_cstr);
-      llvm::ErrorOr<std::string> error_or_path =
-          llvm::sys::findProgramByName(file_str);
-      if (!error_or_path)
-        return false;
-      std::string path = error_or_path.get();
-      llvm::StringRef dir_ref = llvm::sys::path::parent_path(path);
-      if (!dir_ref.empty()) {
-        // FindProgramByName returns "." if it can't find the file.
-        if (strcmp(".", dir_ref.data()) == 0)
-          return false;
-
-        m_directory.SetCString(dir_ref.data());
-        if (Exists())
-          return true;
-        else {
-          // If FindProgramByName found the file, it returns the directory +
-          // filename in its return results. We need to separate them.
-          FileSpec tmp_file(dir_ref.data(), false);
-          if (tmp_file.Exists()) {
-            m_directory = tmp_file.m_directory;
-            return true;
-          }
-        }
-      }
-    }
-  }
-
-  return false;
-}
-
 bool FileSpec::ResolvePath() {
   if (m_is_resolved)
     return true; // We have already resolved this path
 
   // SetFile(...) will set m_is_resolved correctly if it can resolve the path
   SetFile(GetPath(false), true);
   return m_is_resolved;
 }