Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Clang][Darwin] Introduce SubFrameworks as a SDK default location #115048

Merged
merged 4 commits into from
Nov 15, 2024
Merged

[Clang][Darwin] Introduce SubFrameworks as a SDK default location #115048

merged 4 commits into from
Nov 15, 2024
+41 −10

Conversation

cyndyishida
Copy link
Member

  • Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths.
  • Teach clang-installapi to traverse there for framework input.
  • Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private.

resolves: rdar://137457006

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2024

@llvm/pr-subscribers-clang

Author: Cyndy Ishida (cyndyishida)

Changes
  • Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths.
  • Teach clang-installapi to traverse there for framework input.
  • Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private.

resolves: <rdar://137457006>

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

5 Files Affected:

  • (modified) clang/lib/InstallAPI/DirectoryScanner.cpp (+2-1)
  • (modified) clang/lib/Lex/InitHeaderSearch.cpp (+1)
  • (added) clang/test/Driver/darwin-subframeworks.c (+15)
  • (modified) llvm/lib/TextAPI/Utils.cpp (+3)
  • (modified) llvm/test/tools/llvm-readtapi/stubify-delete.test (+5-2)
diff --git a/clang/lib/InstallAPI/DirectoryScanner.cpp b/clang/lib/InstallAPI/DirectoryScanner.cpp
index 03a8208c7364e9..be43a96f3d97d1 100644
--- a/clang/lib/InstallAPI/DirectoryScanner.cpp
+++ b/clang/lib/InstallAPI/DirectoryScanner.cpp
@@ -277,7 +277,8 @@ llvm::Error DirectoryScanner::scanForFrameworks(StringRef Directory) {
   // Expect a certain directory structure and naming convention to find
   // frameworks.
   static const char *SubDirectories[] = {"System/Library/Frameworks/",
-                                         "System/Library/PrivateFrameworks/"};
+                                         "System/Library/PrivateFrameworks/",
+                                         "System/Library/SubFrameworks"};
 
   // Check if the directory is already a framework.
   if (isFramework(Directory)) {
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 86c2ecdf9e36eb..c1769c84887b5f 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -347,6 +347,7 @@ void InitHeaderSearch::AddDefaultIncludePaths(
       } else {
         AddPath("/System/Library/Frameworks", System, true);
         AddPath("/Library/Frameworks", System, true);
+        AddPath("/System/Library/SubFrameworks", System, true);
       }
     }
     return;
diff --git a/clang/test/Driver/darwin-subframeworks.c b/clang/test/Driver/darwin-subframeworks.c
new file mode 100644
index 00000000000000..62d3ab50f44741
--- /dev/null
+++ b/clang/test/Driver/darwin-subframeworks.c
@@ -0,0 +1,15 @@
+// Add default directories before running clang to check default 
+// search paths. 
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: cp -R %S/Inputs/MacOSX15.1.sdk %t/
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/Frameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/SubFrameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/usr/include
+
+// RUN: %clang %s -target arm64-apple-darwin13.0 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck %s
+
+// CHECK:    -isysroot [[PATH:[^ ]*/MacOSX15.1.sdk]]
+// CHECK:    #include <...> search starts here:
+// CHECK:    [[PATH]]/usr/include
+// CHECK:    [[PATH]]/System/Library/Frameworks (framework directory)
+// CHECK:    [[PATH]]/System/Library/SubFrameworks (framework directory)
diff --git a/llvm/lib/TextAPI/Utils.cpp b/llvm/lib/TextAPI/Utils.cpp
index 8a06d53942a947..68d73eac86691d 100644
--- a/llvm/lib/TextAPI/Utils.cpp
+++ b/llvm/lib/TextAPI/Utils.cpp
@@ -120,6 +120,9 @@ bool llvm::MachO::isPrivateLibrary(StringRef Path, bool IsSymLink) {
   if (Path.starts_with("/System/Library/PrivateFrameworks"))
     return true;
 
+  if (Path.starts_with("/System/Library/SubFrameworks"))
+    return true;
+
   // Everything in /usr/lib/swift (including sub-directories) are considered
   // public.
   if (Path.consume_front("/usr/lib/swift/"))
diff --git a/llvm/test/tools/llvm-readtapi/stubify-delete.test b/llvm/test/tools/llvm-readtapi/stubify-delete.test
index 666d740560cbfe..d91b0df06d3d8f 100644
--- a/llvm/test/tools/llvm-readtapi/stubify-delete.test
+++ b/llvm/test/tools/llvm-readtapi/stubify-delete.test
@@ -2,17 +2,20 @@
 # Setup a mix of public and private libraries that resemble apple sdk.
 ; RUN: mkdir -p %t/sysroot/usr/local/lib/ %t/sysroot/usr/lib/
 ; RUN: mkdir -p %t/sysroot/System/Library/Frameworks/System.framework %t/sysroot/System/Library/PrivateFrameworks/Fat.framework
+; RUN: mkdir -p %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers
 ; RUN: yaml2obj %S/Inputs/libSystem.1.yaml -o %t/sysroot/System/Library/Frameworks/System.framework/System
 ; RUN: yaml2obj %S/Inputs/objc.yaml -o %t/sysroot/usr/lib/libobjc.dylib
 ; RUN: cp %t/sysroot/usr/lib/libobjc.dylib %t/sysroot/usr/local/lib/libobjc-unstable.dylib
 ; RUN: yaml2obj %S/Inputs/universal.yaml -o %t/sysroot/System/Library/PrivateFrameworks/Fat.framework/Fat
+; RUN: cp %t/sysroot/System/Library/PrivateFrameworks/Fat.framework/Fat %t/sysroot/System/Library/SubFrameworks/Fat.framework/Fat
+; RUN: touch %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers/Fat.h
 ; RUN: llvm-readtapi -stubify %t/sysroot --delete-input --delete-private-libraries 2>&1 | FileCheck %s --allow-empty  --implicit-check-not warning: --implicit-check-not error:
 # Validate expected files are removed.
 ; RUN: not test -f %t/sysroot/System/Library/PrivateFrameworks
 ; RUN: not test -f %t/sysroot/usr/local
 ; RUN: not test -f %t/sysroot/usr/lib/libobjc.dylib
 ; RUN: not test -f %t/sysroot/System/Library/Frameworks/System.framework/System
+; RUN: not test -f %t/sysroot/System/Library/SubFrameworks/Fat.framework/Fat
 ; RUN: test -f %t/sysroot/System/Library/Frameworks/System.framework/System.tbd
 ; RUN: test -f %t/sysroot/usr/lib/libobjc.tbd
-
-
+; RUN: test -f %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers/Fat.h

@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2024

@llvm/pr-subscribers-clang-driver

Author: Cyndy Ishida (cyndyishida)

Changes
  • Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths.
  • Teach clang-installapi to traverse there for framework input.
  • Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private.

resolves: <rdar://137457006>

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

5 Files Affected:

  • (modified) clang/lib/InstallAPI/DirectoryScanner.cpp (+2-1)
  • (modified) clang/lib/Lex/InitHeaderSearch.cpp (+1)
  • (added) clang/test/Driver/darwin-subframeworks.c (+15)
  • (modified) llvm/lib/TextAPI/Utils.cpp (+3)
  • (modified) llvm/test/tools/llvm-readtapi/stubify-delete.test (+5-2)
diff --git a/clang/lib/InstallAPI/DirectoryScanner.cpp b/clang/lib/InstallAPI/DirectoryScanner.cpp
index 03a8208c7364e9..be43a96f3d97d1 100644
--- a/clang/lib/InstallAPI/DirectoryScanner.cpp
+++ b/clang/lib/InstallAPI/DirectoryScanner.cpp
@@ -277,7 +277,8 @@ llvm::Error DirectoryScanner::scanForFrameworks(StringRef Directory) {
   // Expect a certain directory structure and naming convention to find
   // frameworks.
   static const char *SubDirectories[] = {"System/Library/Frameworks/",
-                                         "System/Library/PrivateFrameworks/"};
+                                         "System/Library/PrivateFrameworks/",
+                                         "System/Library/SubFrameworks"};
 
   // Check if the directory is already a framework.
   if (isFramework(Directory)) {
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 86c2ecdf9e36eb..c1769c84887b5f 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -347,6 +347,7 @@ void InitHeaderSearch::AddDefaultIncludePaths(
       } else {
         AddPath("/System/Library/Frameworks", System, true);
         AddPath("/Library/Frameworks", System, true);
+        AddPath("/System/Library/SubFrameworks", System, true);
       }
     }
     return;
diff --git a/clang/test/Driver/darwin-subframeworks.c b/clang/test/Driver/darwin-subframeworks.c
new file mode 100644
index 00000000000000..62d3ab50f44741
--- /dev/null
+++ b/clang/test/Driver/darwin-subframeworks.c
@@ -0,0 +1,15 @@
+// Add default directories before running clang to check default 
+// search paths. 
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: cp -R %S/Inputs/MacOSX15.1.sdk %t/
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/Frameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/SubFrameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/usr/include
+
+// RUN: %clang %s -target arm64-apple-darwin13.0 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck %s
+
+// CHECK:    -isysroot [[PATH:[^ ]*/MacOSX15.1.sdk]]
+// CHECK:    #include <...> search starts here:
+// CHECK:    [[PATH]]/usr/include
+// CHECK:    [[PATH]]/System/Library/Frameworks (framework directory)
+// CHECK:    [[PATH]]/System/Library/SubFrameworks (framework directory)
diff --git a/llvm/lib/TextAPI/Utils.cpp b/llvm/lib/TextAPI/Utils.cpp
index 8a06d53942a947..68d73eac86691d 100644
--- a/llvm/lib/TextAPI/Utils.cpp
+++ b/llvm/lib/TextAPI/Utils.cpp
@@ -120,6 +120,9 @@ bool llvm::MachO::isPrivateLibrary(StringRef Path, bool IsSymLink) {
   if (Path.starts_with("/System/Library/PrivateFrameworks"))
     return true;
 
+  if (Path.starts_with("/System/Library/SubFrameworks"))
+    return true;
+
   // Everything in /usr/lib/swift (including sub-directories) are considered
   // public.
   if (Path.consume_front("/usr/lib/swift/"))
diff --git a/llvm/test/tools/llvm-readtapi/stubify-delete.test b/llvm/test/tools/llvm-readtapi/stubify-delete.test
index 666d740560cbfe..d91b0df06d3d8f 100644
--- a/llvm/test/tools/llvm-readtapi/stubify-delete.test
+++ b/llvm/test/tools/llvm-readtapi/stubify-delete.test
@@ -2,17 +2,20 @@
 # Setup a mix of public and private libraries that resemble apple sdk.
 ; RUN: mkdir -p %t/sysroot/usr/local/lib/ %t/sysroot/usr/lib/
 ; RUN: mkdir -p %t/sysroot/System/Library/Frameworks/System.framework %t/sysroot/System/Library/PrivateFrameworks/Fat.framework
+; RUN: mkdir -p %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers
 ; RUN: yaml2obj %S/Inputs/libSystem.1.yaml -o %t/sysroot/System/Library/Frameworks/System.framework/System
 ; RUN: yaml2obj %S/Inputs/objc.yaml -o %t/sysroot/usr/lib/libobjc.dylib
 ; RUN: cp %t/sysroot/usr/lib/libobjc.dylib %t/sysroot/usr/local/lib/libobjc-unstable.dylib
 ; RUN: yaml2obj %S/Inputs/universal.yaml -o %t/sysroot/System/Library/PrivateFrameworks/Fat.framework/Fat
+; RUN: cp %t/sysroot/System/Library/PrivateFrameworks/Fat.framework/Fat %t/sysroot/System/Library/SubFrameworks/Fat.framework/Fat
+; RUN: touch %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers/Fat.h
 ; RUN: llvm-readtapi -stubify %t/sysroot --delete-input --delete-private-libraries 2>&1 | FileCheck %s --allow-empty  --implicit-check-not warning: --implicit-check-not error:
 # Validate expected files are removed.
 ; RUN: not test -f %t/sysroot/System/Library/PrivateFrameworks
 ; RUN: not test -f %t/sysroot/usr/local
 ; RUN: not test -f %t/sysroot/usr/lib/libobjc.dylib
 ; RUN: not test -f %t/sysroot/System/Library/Frameworks/System.framework/System
+; RUN: not test -f %t/sysroot/System/Library/SubFrameworks/Fat.framework/Fat
 ; RUN: test -f %t/sysroot/System/Library/Frameworks/System.framework/System.tbd
 ; RUN: test -f %t/sysroot/usr/lib/libobjc.tbd
-
-
+; RUN: test -f %t/sysroot/System/Library/SubFrameworks/Fat.framework/Headers/Fat.h

@ian-twilightcoder
Copy link
Contributor

Do we also need to update darwin::Linker::ConstructJob(...)?

@cyndyishida cyndyishida closed this Nov 5, 2024
@cyndyishida cyndyishida reopened this Nov 5, 2024
@cyndyishida
Copy link
Member Author

cyndyishida commented Nov 5, 2024 •
edited
Loading

Do we also need to update darwin::Linker::ConstructJob(...)?

I thought about it, but clang already doesn't for today's default paths so I decided against it. It seems only pass extra paths to the linker for platform-specific directories like DriverKit. Xcode's ld adds its own default path to S/L/F

@cyndyishida
[Clang][Darwin] Introduce SubFrameworks as a SDK default location
9359df8 
* Have clang always append & pass `System/Library/SubFrameworks` when
   determining default sdk search paths.
* Teach `clang-installapi` to traverse there for framework input.
* Teach `llvm-readtapi` that the library files (TBD or binary) in there should be considered
  private.

resolves: <rdar://137457006>
@cyndyishida cyndyishida force-pushed the eng/pr-newDefaultFrameworks branch from baecb5c to 25e30d6 Compare November 5, 2024 21:17
@cyndyishida
Copy link
Member Author 

> git checkout -f baecb5c50287efebf2482739bfbe320c8147b7ff
error: unable to unlink old 'clang/lib/AST/Decl.cpp': Invalid argument
error: unable to unlink old 'clang/lib/Serialization/ASTReaderDecl.cpp': Invalid argument

Seems like an unrelated issue is happening on the windows bot, noticed the same in a different PR. https://buildkite.com/llvm-project/github-pull-requests/builds/116688#0192fe1c-1484-47e8-b14c-7f3da7038b76

@cyndyishida
[llvm-readtapi] Make directory traversal more reslient to different path

Verified

This commit was signed with the committer’s verified signature. The key has expired.
seemethere Eli Uriegas
GPG key ID: EE2C3B4893010973
Expired
Verified
Learn about vigilant mode
Loading
Loading status checks…
4207ffe 
styles
@cyndyishida cyndyishida force-pushed the eng/pr-newDefaultFrameworks branch from 25e30d6 to 4207ffe Compare November 6, 2024 16:12
@cyndyishida
Copy link
Member Author

ping

@cyndyishida
Swap order of search paths.
Loading
Loading status checks…
3f45b42
@cyndyishida
Disable darwin-subframeworks.c test on windows like driverkit-path test

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
Loading
Loading status checks…
f7d617a
@cyndyishida
Copy link
Member Author

ping

ributzka
ributzka approved these changes Nov 14, 2024
View reviewed changes
Copy link
Collaborator

@ributzka ributzka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cyndyishida cyndyishida merged commit 2d48489 into llvm:main Nov 15, 2024
5 of 8 checks passed
cyndyishida added a commit to cyndyishida/llvm-project that referenced this pull request Dec 5, 2024
@cyndyishida
[Clang][Darwin] Introduce SubFrameworks as a SDK default location (l…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
9ad2355 
…lvm#115048)

* Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths.
* Teach clang-installapi to traverse there for framework input.
* Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private.

resolves: rdar://137457006
(cherry picked from commit 2d48489)
cyndyishida added a commit to swiftlang/llvm-project that referenced this pull request Dec 6, 2024
@cyndyishida
[Clang][Darwin] Introduce SubFrameworks as a SDK default location (l…
0b71161 
…lvm#115048)

* Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths.
* Teach clang-installapi to traverse there for framework input.
* Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private.

resolves: rdar://137457006
(cherry picked from commit 2d48489)
cyndyishida added a commit to cyndyishida/llvm-project that referenced this pull request Dec 6, 2024
@cyndyishida
[Clang][Darwin] Introduce SubFrameworks as a SDK default location (l…
e22f113 
…lvm#115048)

* Have clang always append & pass System/Library/SubFrameworks when determining default sdk search paths.
* Teach clang-installapi to traverse there for framework input.
* Teach llvm-readtapi that the library files (TBD or binary) in there should be considered private.

resolves: rdar://137457006
(cherry picked from commit 2d48489)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakepetroules jakepetroules

@ributzka ributzka

@zixu-w zixu-w

@ian-twilightcoder ian-twilightcoder

@cachemeifyoucan cachemeifyoucan

@ahatanak ahatanak

Assignees
No one assigned
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@cyndyishida @llvmbot @ian-twilightcoder @jakepetroules @ributzka