[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)

2023-10-27 Thread Adrian Prantl via lldb-commits 

https://github.com/adrian-prantl created 
https://github.com/llvm/llvm-project/pull/70528

The code was incorrectly going into the wrong direction by removing one 
component instead of appendeing /Developer to it. Due to fallback mechanisms in 
xcrun this never seemed to have caused any issues.

>From d5e0dee978e75f5d623f6a96121ba277a25c40e9 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Fri, 27 Oct 2023 17:32:43 -0700
Subject: [PATCH] Fix the DEVELOPER_DIR computation

The code was incorrectly going into the wrong direction by removing
one component instead of appendeing /Developer to it. Due to fallback
mechanisms in xcrun this never seemed to have caused any issues.
---
 lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index e3506a01c606b78..33d94504fe70f8c 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -461,13 +461,11 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 // Invoke xcrun with the shlib dir.
 if (FileSpec fspec = HostInfo::GetShlibDir()) {
   if (FileSystem::Instance().Exists(fspec)) {
-std::string contents_dir =
-XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
-llvm::StringRef shlib_developer_dir =
-llvm::sys::path::parent_path(contents_dir);
-if (!shlib_developer_dir.empty()) {
-  auto sdk =
-  xcrun(sdk_name, show_sdk_path, std::move(shlib_developer_dir));
+llvm::SmallString<0> shlib_developer_dir(
+XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath()));
+llvm::sys::path::append(shlib_developer_dir, "Developer");
+if (FileSystem::Instance().Exists(shlib_developer_dir)) {
+  auto sdk = xcrun(sdk_name, show_sdk_path, shlib_developer_dir);
   if (!sdk)
 return sdk.takeError();
   if (!sdk->empty())

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

[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)

2023-10-27 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)


Changes

The code was incorrectly going into the wrong direction by removing one 
component instead of appendeing /Developer to it. Due to fallback mechanisms in 
xcrun this never seemed to have caused any issues.

---
Full diff: https://github.com/llvm/llvm-project/pull/70528.diff


1 Files Affected:

- (modified) lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm (+5-7) 


``diff
diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index e3506a01c606b78..33d94504fe70f8c 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -461,13 +461,11 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 // Invoke xcrun with the shlib dir.
 if (FileSpec fspec = HostInfo::GetShlibDir()) {
   if (FileSystem::Instance().Exists(fspec)) {
-std::string contents_dir =
-XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
-llvm::StringRef shlib_developer_dir =
-llvm::sys::path::parent_path(contents_dir);
-if (!shlib_developer_dir.empty()) {
-  auto sdk =
-  xcrun(sdk_name, show_sdk_path, std::move(shlib_developer_dir));
+llvm::SmallString<0> shlib_developer_dir(
+XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath()));
+llvm::sys::path::append(shlib_developer_dir, "Developer");
+if (FileSystem::Instance().Exists(shlib_developer_dir)) {
+  auto sdk = xcrun(sdk_name, show_sdk_path, shlib_developer_dir);
   if (!sdk)
 return sdk.takeError();
   if (!sdk->empty())

``




https://github.com/llvm/llvm-project/pull/70528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)

2023-10-27 Thread Alex Langford via lldb-commits 

https://github.com/bulbazord approved this pull request.

Makes sense to me

https://github.com/llvm/llvm-project/pull/70528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)

2023-10-27 Thread Alex Langford via lldb-commits 

https://github.com/bulbazord edited 
https://github.com/llvm/llvm-project/pull/70528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)

2023-10-27 Thread Alex Langford via lldb-commits 


@@ -461,13 +461,11 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 // Invoke xcrun with the shlib dir.
 if (FileSpec fspec = HostInfo::GetShlibDir()) {
   if (FileSystem::Instance().Exists(fspec)) {
-std::string contents_dir =
-XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
-llvm::StringRef shlib_developer_dir =
-llvm::sys::path::parent_path(contents_dir);
-if (!shlib_developer_dir.empty()) {
-  auto sdk =
-  xcrun(sdk_name, show_sdk_path, std::move(shlib_developer_dir));
+llvm::SmallString<0> shlib_developer_dir(

bulbazord wrote:

Why 0?
I assume the switch to SmallString has to do with the 
`llvm::sys::path::append`, that part makes sense to me.

https://github.com/llvm/llvm-project/pull/70528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)

2023-10-27 Thread Alex Langford via lldb-commits 


@@ -461,13 +461,11 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 // Invoke xcrun with the shlib dir.
 if (FileSpec fspec = HostInfo::GetShlibDir()) {
   if (FileSystem::Instance().Exists(fspec)) {
-std::string contents_dir =
-XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
-llvm::StringRef shlib_developer_dir =
-llvm::sys::path::parent_path(contents_dir);
-if (!shlib_developer_dir.empty()) {
-  auto sdk =
-  xcrun(sdk_name, show_sdk_path, std::move(shlib_developer_dir));
+llvm::SmallString<0> shlib_developer_dir(
+XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath()));
+llvm::sys::path::append(shlib_developer_dir, "Developer");
+if (FileSystem::Instance().Exists(shlib_developer_dir)) {

bulbazord wrote:

Do we actually need to check its existence or can `xcrun` take care of that for 
us?

https://github.com/llvm/llvm-project/pull/70528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)

2023-10-27 Thread Jonas Devlieghere via lldb-commits 


@@ -461,13 +461,11 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 // Invoke xcrun with the shlib dir.
 if (FileSpec fspec = HostInfo::GetShlibDir()) {
   if (FileSystem::Instance().Exists(fspec)) {
-std::string contents_dir =
-XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
-llvm::StringRef shlib_developer_dir =
-llvm::sys::path::parent_path(contents_dir);
-if (!shlib_developer_dir.empty()) {
-  auto sdk =
-  xcrun(sdk_name, show_sdk_path, std::move(shlib_developer_dir));
+llvm::SmallString<0> shlib_developer_dir(

JDevlieghere wrote:

The way I read that is that there's no reasonably small default so this avoids 
the stack allocation and goes directly to the heap. 

https://github.com/llvm/llvm-project/pull/70528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)

2023-10-30 Thread Adrian Prantl via lldb-commits 


@@ -461,13 +461,11 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 // Invoke xcrun with the shlib dir.
 if (FileSpec fspec = HostInfo::GetShlibDir()) {
   if (FileSystem::Instance().Exists(fspec)) {
-std::string contents_dir =
-XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
-llvm::StringRef shlib_developer_dir =
-llvm::sys::path::parent_path(contents_dir);
-if (!shlib_developer_dir.empty()) {
-  auto sdk =
-  xcrun(sdk_name, show_sdk_path, std::move(shlib_developer_dir));
+llvm::SmallString<0> shlib_developer_dir(
+XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath()));
+llvm::sys::path::append(shlib_developer_dir, "Developer");
+if (FileSystem::Instance().Exists(shlib_developer_dir)) {

adrian-prantl wrote:

There is already a code path to try without DEVELOPER_DIR, and this avoids 
making the same query twice if it doesn't exist.

https://github.com/llvm/llvm-project/pull/70528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)

2023-10-30 Thread Adrian Prantl via lldb-commits 

https://github.com/adrian-prantl closed 
https://github.com/llvm/llvm-project/pull/70528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits