Teach GetXcodeSDK to look in the Xcode that contains LLDB

[aprantl]

Summary

Teach GetXcodeSDK to look in the Xcode that contains LLDB instead of preferring the one chosen with xcode-select. Because we're using xcrun to find matching SDK's you can now get into a situation where LLDB, when run from a non-xcode-selected Xcode will find a matching SDK in the xcode-selected Xcode, which can cause anything from mild performance degradation to really confusing Clang compile errors, if the other Xcode is, for example, older, or missing an SDK.

rdar://problem/64000666

[friss]

This LGTM, with the caveat that it doesn't handle command line tools. But we didn't handle it correctly before, so not something we need to fix here.

[JDevlieghere]

lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

363

Above we use getenv, here we use Host::GetEnvironment. We should pick one and be consistent.

366

It looks like this is basically dealing with one case (shlib) of the logic in GetXcodeContentsDirectory. I would prefer to make this a separate function GetXcodeDeveloperDirectory or something that does just that.

Before I refactored these functions there were several methods where we tried to share code like this. While I'm generally a strong opponent of sharing code, this lead to the pattern we see here where we need to add complexity to the "generic" function to make things work. This in turn makes things more complex and harder to understand, up till the point that we inevitable implemented something "similar but different" elsewhere.

[JDevlieghere]

lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

371

Not something you should fix for this patch, but RunShellCommand should be able to take environment variables.

[aprantl]

Address feedback from Jonas!

[JDevlieghere]

lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

365

Any reason you choose to leave this inline and not making it a separate function? If we did we could also cache the result like we do for some of the other paths.

[aprantl]

lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

363

I will create a follow-up patch for above.

365

This function is going to be called at most ~7*2 times, once for each XcodeSDK kind * Internal. So caching isn't really relevant. i also would like to refactor the other function after this patch, which would make factoring this out first really awkward.

[JDevlieghere]

LGTM

lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

365

Sounds good!

lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

lldb_private::FileSpec HostInfoMacOSX::GetXcodeDeveloperDirectory() {
   });
   return g_developer_directory;
 }
 
 static std::string GetXcodeSDK(XcodeSDK sdk) {
   XcodeSDK::Info info = sdk.Parse();
   std::string sdk_name = XcodeSDK::GetCanonicalName(info);
   auto find_sdk = [](std::string sdk_name) -> std::string {
-    std::string xcrun_cmd = "xcrun --show-sdk-path --sdk " + sdk_name;
+    std::string xcrun_cmd;
+    Environment env = Host::GetEnvironment();
+    std::string developer_dir = env.lookup("DEVELOPER_DIR");
+    if (developer_dir.empty())
+      if (FileSpec fspec = HostInfo::GetShlibDir())
+        if (FileSystem::Instance().Exists(fspec)) {
+          FileSpec path(
+              XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath()));
+          if (path.RemoveLastPathComponent())
+            developer_dir = path.GetPath();
+        }
+    if (!developer_dir.empty())
+      xcrun_cmd = "/usr/bin/env DEVELOPER_DIR=\"" + developer_dir + "\" ";
+    xcrun_cmd += "xcrun --show-sdk-path --sdk " + sdk_name;
+
     int status = 0;
     int signo = 0;
     std::string output_str;
     lldb_private::Status error =
         Host::RunShellCommand(xcrun_cmd.c_str(), FileSpec(), &status, &signo,
                               &output_str, std::chrono::seconds(15));
 
     // Check that xcrun return something useful.