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.

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

Add SwiftPM support #515

Merged
merged 2 commits into from
May 10, 2024
Merged

Conversation

Kyle-Ye
Copy link
Contributor

@Kyle-Ye Kyle-Ye commented May 7, 2024

No description provided.

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented May 7, 2024

添加了 SwiftPM 支持,大佬能抽空帮忙看下吗? @Richard-Cao

合入后可能还得发布一个新版本才行

@Richard-Cao Richard-Cao requested a review from wsxyeah May 8, 2024 02:00
@Richard-Cao
Copy link
Member

@wsxyeah 一起看看

@wsxyeah
Copy link
Collaborator

wsxyeah commented May 8, 2024

这几个头文件是用的软链接?GitHub diff 上看不太出来

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented May 8, 2024

是的,不太想破坏原目录结构,常见的做法是把对应 header 软链到 include 里来满足 Package 要求的 Layout

https://github.com/apple/swift-package-manager/blob/main/Documentation/Usage.md#creating-c-language-targets

cd include
ln -s .../**/*.h ./

@wsxyeah
Copy link
Collaborator

wsxyeah commented May 8, 2024

是的,不太想破坏原目录结构,常见的做法是把对应 header 软链到 include 里来满足 Package 要求的 Layout

https://github.com/apple/swift-package-manager/blob/main/Documentation/Usage.md#creating-c-language-targets


cd include

ln -s .../**/*.h ./

配置 publicHeadersPath 应该也可以?

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented May 8, 2024

是的,不太想破坏原目录结构,常见的做法是把对应 header 软链到 include 里来满足 Package 要求的 Layout
https://github.com/apple/swift-package-manager/blob/main/Documentation/Usage.md#creating-c-language-targets


cd include

ln -s .../**/*.h ./

配置 publicHeadersPath 应该也可以?

publicHeadersPath 是额外的for search用的,这里讨论的是 include / modulemap layout

我的经验告诉我是不行的,或者您可以尝试下,直接 comment 提供新的 patch

publicHeadersPath 不传就是默认 "include",用 "." 试过是不行的,因为布局不满足要求

@wsxyeah
Copy link
Collaborator

wsxyeah commented May 8, 2024

你说的 for search 应该是指 cSettings.headerSearchPath

我看这个库是用的publicHeadersPath

参考文档

@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented May 8, 2024

你说的 for search 应该是指 cSettings.headerSearchPath

参考文档

Nope.

Sorry for the miswording/confustion I cause.

You can have a look at the SwiftPM repo code here

// Compute the path to public headers directory.
let publicHeaderComponent = manifestTarget.publicHeadersPath ?? ClangTarget.defaultPublicHeadersComponent 

public final class ClangTarget: Target {
    /// The default public include directory component.
    public static let defaultPublicHeadersComponent = "include"

publicHeadersPath will be "include" if we do not specific it for ClangTarget.

We still need such layout here.

If you do not provide include folder and try set it to "." things will not work here because the layout is not valid.

我看这个库是用的publicHeadersPath

If you like, I can add publicHeadersPath: "include" here but that's not the point and unnecessary for most of the case.

Again, if you have any better suggestion or comment, I believe the best way is to communicate via code/patch.

@wsxyeah
Copy link
Collaborator

wsxyeah commented May 9, 2024

试了下这样写是可以的

@@ -38,12 +38,14 @@ let package = Package(
             name: "CLogan",
             dependencies: ["mbedtls"],
             path: "Logan/Clogan",
-            exclude: ["main.c"]
+            exclude: ["main.c"],
+            publicHeadersPath: "./"
         ),
         .target(
             name: "Logan",
             dependencies: ["CLogan"],
-            path: "Logan/iOS"
+            path: "Logan/iOS",
+            publicHeadersPath: "./"
         ),
     ]
 )

另外 .gitignore 需要添加下 .build 目录

Co-authored-by: wsxyeah <[email protected]>
@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented May 9, 2024

试了下这样写是可以的

Thanks, it works. Maybe there is some strange cache issue for my previous try.

另外 .gitignore 需要添加下 .build 目录

Done

@Richard-Cao
Copy link
Member

@wsxyeah 那我合了?

@wsxyeah
Copy link
Collaborator

wsxyeah commented May 9, 2024

@wsxyeah 那我合了?

ok

@Richard-Cao Richard-Cao merged commit 41bc5d0 into Meituan-Dianping:master May 10, 2024
@Richard-Cao
Copy link
Member

@wsxyeah 那我合了?

ok

合了,应该要重新发个版

@Kyle-Ye Kyle-Ye deleted the feature/spm branch May 10, 2024 03:04
@Kyle-Ye
Copy link
Contributor Author

Kyle-Ye commented May 14, 2024

合了,应该要重新发个版

cc @wsxyeah

@wsxyeah
Copy link
Collaborator

wsxyeah commented May 16, 2024

合了,应该要重新发个版

cc @wsxyeah

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants