[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

[Douglas Yung via Phabricator via lldb-commits]

dyung added a comment.

I’m also seeing this fail on 2 buildbots:

https://lab.llvm.org/buildbot/#/builders/217/builds/20568
https://lab.llvm.org/buildbot/#/builders/243/builds/5576


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149397/new/

https://reviews.llvm.org/D149397

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

I'm seeing the same build failures reported by mgorny. Would you consider 
reverting this patch until you can fix it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149397/new/

https://reviews.llvm.org/D149397

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

This change broke building with GCC:

  samu: job failed: /usr/lib/ccache/bin/x86_64-pc-linux-gnu-g++ -DHAVE_ROUND 
-D_DEBUG -D_GLIBCXX_ASSERTIONS -D_LIBCPP_ENABLE_ASSERTIONS 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-I/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb_build/source/Host 
-I/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/source/Host 
-I/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/include 
-I/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb_build/include 
-I/usr/lib/llvm/17/include -I/usr/include/python3.11 
-I/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/tools/clang/include
 
-I/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb_build/../clang/include
 -I/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/source 
-I/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb_build/source 
-isystem /usr/include/libxml2  -march=znver2 --param=l1-cache-size=32 
--param=l1-cache-line-size=64 -O2 -pipe -frecord-gcc-switches -fPIC 
-fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall 
-Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
-Wno-missing-field-initializers -Wimplicit-fallthrough -Wno-class-memaccess 
-Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment 
-Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color 
-ffunction-sections -fdata-sections -Wno-deprecated-declarations 
-Wno-unknown-pragmas -Wno-strict-aliasing -Wno-stringop-truncation -std=c++17 
-UNDEBUG -MD -MT source/Host/CMakeFiles/lldbHost.dir/posix/PipePosix.cpp.o -MF 
source/Host/CMakeFiles/lldbHost.dir/posix/PipePosix.cpp.o.d -o 
source/Host/CMakeFiles/lldbHost.dir/posix/PipePosix.cpp.o -c 
/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/source/Host/posix/PipePosix.cpp
  In file included from 
/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/include/lldb/Host/posix/HostInfoPosix.h:12,
   from 
/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/include/lldb/Host/linux/HostInfoLinux.h:12,
   from 
/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/include/lldb/Host/HostInfo.h:43,
   from 
/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/source/Host/posix/PipePosix.cpp:10:
  
/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/include/lldb/Host/HostInfoBase.h:130:29:
 error: declaration of ‘std::optional 
lldb_private::HostInfoBase::SDKOptions::XcodeSDK’ changes meaning of ‘XcodeSDK’ 
[-Wchanges-meaning]
130 | std::optional XcodeSDK;
| ^~~~
  
/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/include/lldb/Host/HostInfoBase.h:130:19:
 note: used here to mean ‘class lldb_private::XcodeSDK’
130 | std::optional XcodeSDK;
|   ^~~~
  In file included from 
/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/include/lldb/Host/HostInfoBase.h:16:
  
/var/tmp/portage/dev-util/lldb-17.0.0_pre20230429/work/lldb/include/lldb/Utility/XcodeSDK.h:24:7:
 note: declared here
 24 | class XcodeSDK {
|   ^~~~
  samu: subcommands failed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149397/new/

https://reviews.llvm.org/D149397

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

[Saleem Abdulrasool via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGade3c6a6a88e: Host: generalise `GetXcodeSDKPath` (authored 
by compnerd).

Changed prior to commit:
  https://reviews.llvm.org/D149397?vs=517764=517959#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149397/new/

https://reviews.llvm.org/D149397

Files:
  lldb/include/lldb/Host/HostInfoBase.h
  lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
  lldb/source/Core/Module.cpp
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/unittests/Host/HostInfoTest.cpp

Index: lldb/unittests/Host/HostInfoTest.cpp
===
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -57,7 +57,8 @@
 #if defined(__APPLE__)
 TEST_F(HostInfoTest, GetXcodeSDK) {
   auto get_sdk = [](std::string sdk, bool error = false) -> llvm::StringRef {
-auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(sdk)));
+auto sdk_path_or_err =
+HostInfo::GetSDKRoot(HostInfo::SDKOptions{XcodeSDK(std::move(sdk))});
 if (!error) {
   EXPECT_TRUE((bool)sdk_path_or_err);
   return *sdk_path_or_err;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -124,7 +124,8 @@
   }
 
   // Use the default SDK as a fallback.
-  auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS());
+  auto sdk_path_or_err =
+  HostInfo::GetSDKRoot(HostInfo::SDKOptions{XcodeSDK::GetAnyMacOS()});
   if (!sdk_path_or_err) {
 Debugger::ReportError("Error while searching for Xcode SDK: " +
   toString(sdk_path_or_err.takeError()));
Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -284,7 +284,8 @@
   std::string secondary) {
   llvm::StringRef sdk;
   auto get_sdk = [&](std::string sdk) -> llvm::StringRef {
-auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(sdk)));
+auto sdk_path_or_err =
+HostInfo::GetSDKRoot(HostInfo::SDKOptions{XcodeSDK(std::move(sdk))});
 if (!sdk_path_or_err) {
   Debugger::ReportError("Error while searching for Xcode SDK: " +
 toString(sdk_path_or_err.takeError()));
Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -338,7 +338,8 @@
   }
 }
 
-auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS());
+auto sdk_path_or_err =
+HostInfo::GetSDKRoot(SDKOptions{XcodeSDK::GetAnyMacOS()});
 if (!sdk_path_or_err) {
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOGF(log, "Error while searching for Xcode SDK: %s",
@@ -519,7 +520,7 @@
   return path;
 }
 
-llvm::Expected HostInfoMacOSX::GetXcodeSDKPath(XcodeSDK sdk) {
+llvm::Expected HostInfoMacOSX::GetSDKRoot(SDKOptions options) {
   struct ErrorOrPath {
 std::string str;
 bool is_error;
@@ -530,6 +531,11 @@
   std::lock_guard guard(g_sdk_path_mutex);
   LLDB_SCOPED_TIMER();
 
+  if (!options.XcodeSDK)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "XCodeSDK not specified");
+  XcodeSDK sdk = *options.XcodeSDK;
+
   auto key = sdk.GetString();
   auto it = g_sdk_path.find(key);
   if (it != g_sdk_path.end()) {
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1607,8 +1607,8 @@
 
 void Module::RegisterXcodeSDK(llvm::StringRef sdk_name,
   llvm::StringRef sysroot) {
-  XcodeSDK sdk(sdk_name.str());
-  auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(sdk);
+  auto sdk_path_or_err =
+  HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk_name.str()});
 
   if (!sdk_path_or_err) {
 Debugger::ReportError("Error while searching for Xcode SDK: " +
Index: lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
===
--- lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
+++ lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
@@ -31,7 +31,7 @@
   static FileSpec GetXcodeDeveloperDirectory();
 
   /// Query xcrun to find an Xcode SDK directory.
-  static llvm::Expected 

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks Saleem!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149397/new/

https://reviews.llvm.org/D149397

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd created this revision.
compnerd added reviewers: JDevlieghere, kastiglione, aprantl.
compnerd added a project: LLDB.
Herald added a project: All.
compnerd requested review of this revision.

This generalises the `GetXcodeSDKPath` hook to a `GetSDKRoot` path which
will be re-used for the Windows support to compute a language specific
SDK path on the platform.  Because there may be other options that we
wish to use to compute the SDK path, sink the `XcodeSDK` parameter into
a structure which can pass a disaggregated set of options.  Furthermore,
optionalise the parameter as Xcode is not available for all platforms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149397

Files:
  lldb/include/lldb/Host/HostInfoBase.h
  lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
  lldb/source/Core/Module.cpp
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/unittests/Host/HostInfoTest.cpp

Index: lldb/unittests/Host/HostInfoTest.cpp
===
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -57,7 +57,7 @@
 #if defined(__APPLE__)
 TEST_F(HostInfoTest, GetXcodeSDK) {
   auto get_sdk = [](std::string sdk, bool error = false) -> llvm::StringRef {
-auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(sdk)));
+auto sdk_path_or_err = HostInfo::GetSDKRoot(SDKOptions{XcodeSDK(std::move(sdk))});
 if (!error) {
   EXPECT_TRUE((bool)sdk_path_or_err);
   return *sdk_path_or_err;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -124,7 +124,7 @@
   }
 
   // Use the default SDK as a fallback.
-  auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS());
+  auto sdk_path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{XcodeSDK::GetAnyMacOS()});
   if (!sdk_path_or_err) {
 Debugger::ReportError("Error while searching for Xcode SDK: " +
   toString(sdk_path_or_err.takeError()));
Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -284,7 +284,7 @@
   std::string secondary) {
   llvm::StringRef sdk;
   auto get_sdk = [&](std::string sdk) -> llvm::StringRef {
-auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK(std::move(sdk)));
+auto sdk_path_or_err = HostInfo::GetSDKRoot(SDKOptions{XcodeSDK(std::move(sdk))});
 if (!sdk_path_or_err) {
   Debugger::ReportError("Error while searching for Xcode SDK: " +
 toString(sdk_path_or_err.takeError()));
Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -338,7 +338,7 @@
   }
 }
 
-auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS());
+auto sdk_path_or_err = HostInfo::GetSDKRoot(SDKOptions{XcodeSDK::GetAnyMacOS()});
 if (!sdk_path_or_err) {
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOGF(log, "Error while searching for Xcode SDK: %s",
@@ -519,7 +519,7 @@
   return path;
 }
 
-llvm::Expected HostInfoMacOSX::GetXcodeSDKPath(XcodeSDK sdk) {
+llvm::Expected HostInfoMacOSX::GetSDKRoot(SDKOptions options) {
   struct ErrorOrPath {
 std::string str;
 bool is_error;
@@ -539,7 +539,7 @@
 else
   return it->second.str;
   }
-  auto path_or_err = GetXcodeSDK(sdk);
+  auto path_or_err = GetXcodeSDK(options.XcodeSDK);
   if (!path_or_err) {
 std::string error = toString(path_or_err.takeError());
 g_sdk_path.insert({key, {error, true}});
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1607,8 +1607,7 @@
 
 void Module::RegisterXcodeSDK(llvm::StringRef sdk_name,
   llvm::StringRef sysroot) {
-  XcodeSDK sdk(sdk_name.str());
-  auto sdk_path_or_err = HostInfo::GetXcodeSDKPath(sdk);
+  auto sdk_path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk_name.str()});
 
   if (!sdk_path_or_err) {
 Debugger::ReportError("Error while searching for Xcode SDK: " +
Index: lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
===
--- lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
+++ lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
@@ -31,7