[Lldb-commits] [lldb] Fix the DEVELOPER_DIR computation (PR #70528)
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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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