aprantl updated this revision to Diff 475857.
aprantl added a comment.
Address feedback
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138060/new/
https://reviews.llvm.org/D138060
Files:
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
lldb/unittests/Host/CMakeLists.txt
lldb/unittests/Host/HostTest.cpp
Index: lldb/unittests/Host/HostTest.cpp
===================================================================
--- lldb/unittests/Host/HostTest.cpp
+++ lldb/unittests/Host/HostTest.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "lldb/Host/Host.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfoBase.h"
#include "gtest/gtest.h"
using namespace lldb_private;
@@ -25,3 +27,25 @@
ASSERT_EQ("Host::GetEnvironment",
Host::GetEnvironment().lookup("LLDB_TEST_ENVIRONMENT_VAR"));
}
+
+#if defined(__APPLE__)
+TEST(Host, RunShellCommand) {
+ HostInfoBase::Initialize();
+ FileSystem::Initialize();
+ std::string shell = std::string("SHELL=") + getenv("SHELL");
+ putenv(const_cast<char *>("SHELL=/bin/LLDB_TEST_this-file-does-not-exist"));
+ int status, signo;
+ std::string out;
+ auto timeout = std::chrono::seconds(60);
+ Status error = Host::RunShellCommand("/usr/bin/true", FileSpec(), &status,
+ &signo, &out, timeout);
+ ASSERT_TRUE(error.Fail());
+ putenv(const_cast<char *>(shell.c_str()));
+
+ error = Host::RunShellCommand("/usr/bin/false", FileSpec(), &status, &signo,
+ &out, timeout);
+ ASSERT_FALSE(error.Fail());
+ HostInfoBase::Terminate();
+ FileSystem::Terminate();
+}
+#endif
Index: lldb/unittests/Host/CMakeLists.txt
===================================================================
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -32,6 +32,7 @@
${FILES}
LINK_LIBS
lldbHost
+ lldbCore
lldbUtilityHelpers
lldbHostHelpers
LLVMTestingSupport
Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===================================================================
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -6,15 +6,17 @@
//
//===----------------------------------------------------------------------===//
+#include "lldb/Host/macosx/HostInfoMacOSX.h"
+#include "Utility/UuidCompatibility.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Progress.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
-#include "lldb/Host/macosx/HostInfoMacOSX.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Timer.h"
-#include "Utility/UuidCompatibility.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
@@ -365,12 +367,16 @@
return g_developer_directory;
}
-static std::string GetXcodeSDK(XcodeSDK sdk) {
+llvm::Expected<std::string> GetXcodeSDK(XcodeSDK sdk) {
XcodeSDK::Info info = sdk.Parse();
std::string sdk_name = XcodeSDK::GetCanonicalName(info);
+ Progress progress(
+ llvm::formatv("Invoking xcrun to search for SDK {0}", sdk_name));
+ Log *log = GetLog(LLDBLog::Host);
auto xcrun = [](const std::string &sdk,
- llvm::StringRef developer_dir = "") -> std::string {
+ llvm::StringRef developer_dir =
+ "") -> llvm::Expected<std::string> {
Args args;
if (!developer_dir.empty()) {
args.AppendArgument("/usr/bin/env");
@@ -391,13 +397,29 @@
int status = 0;
int signo = 0;
std::string output_str;
- lldb_private::Status error =
- Host::RunShellCommand(args, FileSpec(), &status, &signo, &output_str,
- std::chrono::seconds(15));
-
- // Check that xcrun return something useful.
- if (status != 0 || output_str.empty())
- return {};
+ // The first time after Xcode was updated or freshly installed,
+ // xcrun can take surprisingly long to build up its database.
+ auto timeout = std::chrono::seconds(60);
+ bool run_in_shell = false;
+ lldb_private::Status error = Host::RunShellCommand(
+ args, FileSpec(), &status, &signo, &output_str, timeout, run_in_shell);
+
+ // Check that xcrun returned something useful.
+ if (error.Fail()) {
+ // Catastrophic error.
+ LLDB_LOG(log, "xcrun failed to execute: %s", error.AsCString());
+ return error.ToError();
+ }
+ if (status != 0) {
+ // xcrun didn't find a matching SDK. Not an error, we'll try
+ // different spellings.
+ LLDB_LOG(log, "xcrun returned exit code %d", status);
+ return "";
+ }
+ if (output_str.empty()) {
+ LLDB_LOG(log, "xcrun returned no results");
+ return "";
+ }
// Convert to a StringRef so we can manipulate the string without modifying
// the underlying data.
@@ -414,7 +436,8 @@
return output.str();
};
- auto find_sdk = [&xcrun](const std::string &sdk_name) -> std::string {
+ auto find_sdk =
+ [&xcrun](const std::string &sdk_name) -> llvm::Expected<std::string> {
// Invoke xcrun with the developer dir specified in the environment.
std::string developer_dir = GetEnvDeveloperDir();
if (!developer_dir.empty()) {
@@ -430,8 +453,10 @@
llvm::StringRef shlib_developer_dir =
llvm::sys::path::parent_path(contents_dir);
if (!shlib_developer_dir.empty()) {
- std::string sdk = xcrun(sdk_name, std::move(shlib_developer_dir));
- if (!sdk.empty())
+ auto sdk = xcrun(sdk_name, std::move(shlib_developer_dir));
+ if (!sdk)
+ return sdk.takeError();
+ if (!sdk->empty())
return sdk;
}
}
@@ -441,7 +466,10 @@
return xcrun(sdk_name);
};
- std::string path = find_sdk(sdk_name);
+ auto path_or_err = find_sdk(sdk_name);
+ if (!path_or_err)
+ return path_or_err.takeError();
+ std::string path = *path_or_err;
while (path.empty()) {
// Try an alternate spelling of the name ("macosx10.9internal").
if (info.type == XcodeSDK::Type::MacOSX && !info.version.empty() &&
@@ -449,29 +477,37 @@
llvm::StringRef fixed(sdk_name);
if (fixed.consume_back(".internal"))
sdk_name = fixed.str() + "internal";
- path = find_sdk(sdk_name);
+ path_or_err = find_sdk(sdk_name);
+ if (!path_or_err)
+ return path_or_err.takeError();
+ path = *path_or_err;
if (!path.empty())
break;
}
- Log *log = GetLog(LLDBLog::Host);
LLDB_LOGF(log, "Couldn't find SDK %s on host", sdk_name.c_str());
// Try without the version.
if (!info.version.empty()) {
info.version = {};
sdk_name = XcodeSDK::GetCanonicalName(info);
- path = find_sdk(sdk_name);
+ path_or_err = find_sdk(sdk_name);
+ if (!path_or_err)
+ return path_or_err.takeError();
+ path = *path_or_err;
if (!path.empty())
break;
}
LLDB_LOGF(log, "Couldn't find any matching SDK on host");
- return {};
+ return "";
}
// Whatever is left in output should be a valid path.
- if (!FileSystem::Instance().Exists(path))
- return {};
+ if (!FileSystem::Instance().Exists(path)) {
+ LLDB_LOGF(log, "SDK returned by xcrun doesn't exist");
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "SDK returned by xcrun doesn't exist");
+ }
return path;
}
@@ -485,7 +521,18 @@
auto it = g_sdk_path.find(sdk.GetString());
if (it != g_sdk_path.end())
return it->second;
- auto it_new = g_sdk_path.insert({sdk.GetString(), GetXcodeSDK(sdk)});
+ auto path_or_err = GetXcodeSDK(sdk);
+ std::string path;
+ if (!path_or_err)
+ Debugger::ReportError("Error while searching for Xcode SDK: " +
+ toString(path_or_err.takeError()));
+ else
+ path = *path_or_err;
+ if (path.empty())
+ Debugger::ReportWarning("Could not find a matching Xcode SDK for " +
+ sdk.GetString().str());
+
+ auto it_new = g_sdk_path.insert({sdk.GetString(), path});
return it_new.first->second;
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
