From bfabefd8c75e585d2a6a89c1c0d30d80ce0e30ab Mon Sep 17 00:00:00 2001 From: ikesyo Date: Fri, 4 Oct 2024 14:39:04 +0900 Subject: [PATCH] Fix collectPublicHeaders' notDuplicatedSymlinks logic - Flip contentsEqual condition - Resolve symbolic links before calling `FileManager.contentsEqual` because the method does not traverse symbolic links, but compares the links themselves --- .../PIF/FrameworkComponentsCollector.swift | 13 ++-- .../.gitignore | 9 +++ .../Package.swift | 25 ++++++++ .../README.md | 3 + .../external_headers/a.h | 1 + .../external_headers/b.h | 1 + .../src/some_lib/include/a.h | 1 + .../src/some_lib/include/b.h | 1 + .../src/some_lib/include/some_lib.h | 10 +++ .../src/some_lib/include/some_lib_dupe.h | 1 + .../src/some_lib/some_lib.c | 5 ++ Tests/ScipioKitTests/RunnerTests.swift | 64 +++++++++++++++++++ 12 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/.gitignore create mode 100644 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/Package.swift create mode 100644 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/README.md create mode 100644 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/external_headers/a.h create mode 100644 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/external_headers/b.h create mode 120000 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/a.h create mode 120000 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/b.h create mode 100644 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/some_lib.h create mode 120000 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/some_lib_dupe.h create mode 100644 Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/some_lib.c diff --git a/Sources/ScipioKit/Producer/PIF/FrameworkComponentsCollector.swift b/Sources/ScipioKit/Producer/PIF/FrameworkComponentsCollector.swift index c56fa9c4..f063b0b4 100644 --- a/Sources/ScipioKit/Producer/PIF/FrameworkComponentsCollector.swift +++ b/Sources/ScipioKit/Producer/PIF/FrameworkComponentsCollector.swift @@ -71,7 +71,7 @@ struct FrameworkComponentsCollector { in: generatedFrameworkPath ) - let publicHeaders = try collectPublicHeader() + let publicHeaders = try collectPublicHeaders() let resourceBundlePath = try collectResourceBundle( of: targetName, @@ -165,7 +165,7 @@ struct FrameworkComponentsCollector { } /// Collects public headers of clangTarget - private func collectPublicHeader() throws -> Set? { + private func collectPublicHeaders() throws -> Set? { guard let clangModule = buildProduct.target.underlying as? ScipioClangModule else { return nil } @@ -180,11 +180,14 @@ struct FrameworkComponentsCollector { // Sometimes, public headers include a file and its symlink both. // This situation raises a duplication error // So duplicated symlinks have to be omitted - let notDuplicatedSymlinks = symlinks.filter { path in - notSymlinks.allSatisfy { FileManager.default.contentsEqual(atPath: path.pathString, andPath: $0.pathString) } - } + let notDuplicatedSymlinks = symlinks + // `FileManager.contentsEqual` does not traverse symbolic links, but compares the links themselves. + // So we need to resolve the links beforehand. .map { $0.asURL.resolvingSymlinksInPath() } .map(\.absolutePath) + .filter { path in + notSymlinks.allSatisfy { !FileManager.default.contentsEqual(atPath: path.pathString, andPath: $0.pathString) } + } return Set(notSymlinks + notDuplicatedSymlinks) } diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/.gitignore b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/.gitignore new file mode 100644 index 00000000..3b298120 --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/.gitignore @@ -0,0 +1,9 @@ +.DS_Store +/.build +/Packages +/*.xcodeproj +xcuserdata/ +DerivedData/ +.swiftpm/config/registries.json +.swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata +.netrc diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/Package.swift b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/Package.swift new file mode 100644 index 00000000..9a99acc0 --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/Package.swift @@ -0,0 +1,25 @@ +// swift-tools-version: 5.7 +// The swift-tools-version declares the minimum version of Swift required to build this package. + +import PackageDescription + +let package = Package( + name: "ClangPackageWithSymbolicLinkHeaders", + platforms: [.iOS(.v11)], + products: [ + // Products define the executables and libraries a package produces, and make them visible to other packages. + .library( + name: "some_lib", + targets: ["some_lib"]), + ], + dependencies: [ + // Dependencies declare other packages that this package depends on. + // .package(url: /* package url */, from: "1.0.0"), + ], + targets: [ + .target( + name: "some_lib", + dependencies: [] + ), + ] +) diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/README.md b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/README.md new file mode 100644 index 00000000..75e48903 --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/README.md @@ -0,0 +1,3 @@ +# ClangPackageWithSymbolicLinkHeaders + +A description of this package. diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/external_headers/a.h b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/external_headers/a.h new file mode 100644 index 00000000..17ab46c8 --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/external_headers/a.h @@ -0,0 +1 @@ +// a.h diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/external_headers/b.h b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/external_headers/b.h new file mode 100644 index 00000000..12a6d54d --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/external_headers/b.h @@ -0,0 +1 @@ +// b.h diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/a.h b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/a.h new file mode 120000 index 00000000..54c939e4 --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/a.h @@ -0,0 +1 @@ +../../../external_headers/a.h \ No newline at end of file diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/b.h b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/b.h new file mode 120000 index 00000000..7dabb009 --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/b.h @@ -0,0 +1 @@ +../../../external_headers/b.h \ No newline at end of file diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/some_lib.h b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/some_lib.h new file mode 100644 index 00000000..c836c084 --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/some_lib.h @@ -0,0 +1,10 @@ +#ifndef some_lib_h +#define some_lib_h + +#include +#include "a.h" +#include "b.h" + +int add(int lhs, int rhs); + +#endif /* some_lib_h */ diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/some_lib_dupe.h b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/some_lib_dupe.h new file mode 120000 index 00000000..5d045376 --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/include/some_lib_dupe.h @@ -0,0 +1 @@ +some_lib.h \ No newline at end of file diff --git a/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/some_lib.c b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/some_lib.c new file mode 100644 index 00000000..e918ab35 --- /dev/null +++ b/Tests/ScipioKitTests/Resources/Fixtures/ClangPackageWithSymbolicLinkHeaders/src/some_lib/some_lib.c @@ -0,0 +1,5 @@ +#include "include/some_lib.h" + +int add(int lhs, int rhs) { + return lhs + rhs; +} diff --git a/Tests/ScipioKitTests/RunnerTests.swift b/Tests/ScipioKitTests/RunnerTests.swift index 4bf9b4b1..23c2a169 100644 --- a/Tests/ScipioKitTests/RunnerTests.swift +++ b/Tests/ScipioKitTests/RunnerTests.swift @@ -12,6 +12,7 @@ private let binaryPackagePath = fixturePath.appendingPathComponent("BinaryPackag private let resourcePackagePath = fixturePath.appendingPathComponent("ResourcePackage") private let usingBinaryPackagePath = fixturePath.appendingPathComponent("UsingBinaryPackage") private let clangPackagePath = fixturePath.appendingPathComponent("ClangPackage") +private let clangPackageWithSymbolicLinkHeadersPath = fixturePath.appendingPathComponent("ClangPackageWithSymbolicLinkHeaders") private let clangPackageWithCustomModuleMapPath = fixturePath.appendingPathComponent("ClangPackageWithCustomModuleMap") private let clangPackageWithUmbrellaDirectoryPath = fixturePath.appendingPathComponent("ClangPackageWithUmbrellaDirectory") @@ -155,6 +156,69 @@ final class RunnerTests: XCTestCase { } } + func testBuildClangPackageWithSymbolicLinkHeaders() async throws { + let runner = Runner( + mode: .createPackage, + options: .init( + baseBuildOptions: .init(isSimulatorSupported: false), + shouldOnlyUseVersionsFromResolvedFile: true + ) + ) + do { + try await runner.run(packageDirectory: clangPackageWithSymbolicLinkHeadersPath, + frameworkOutputDir: .custom(frameworkOutputDir)) + } catch { + XCTFail("Build should be succeeded. \(error.localizedDescription)") + } + + for library in ["some_lib"] { + print(frameworkOutputDir) + let xcFramework = frameworkOutputDir.appendingPathComponent("\(library).xcframework") + let versionFile = frameworkOutputDir.appendingPathComponent(".\(library).version") + let framework = xcFramework.appendingPathComponent("ios-arm64") + .appendingPathComponent("\(library).framework") + + XCTAssertTrue( + fileManager.fileExists(atPath: framework.appendingPathComponent("Headers/some_lib.h").path), + "Should exist an umbrella header" + ) + XCTAssertTrue( + fileManager.fileExists(atPath: framework.appendingPathComponent("Headers/a.h").path), + "Should exist a header from symbolic link" + ) + XCTAssertTrue( + fileManager.fileExists(atPath: framework.appendingPathComponent("Headers/b.h").path), + "Should exist another header from symbolic link" + ) + XCTAssertFalse( + fileManager.fileExists(atPath: framework.appendingPathComponent("Headers/some_lib_dupe.h").path), + "Should not exist a header from symbolic link which is duplicated to non-symbolic link one" + ) + + let moduleMapPath = framework.appendingPathComponent("Modules/module.modulemap").path + XCTAssertTrue( + fileManager.fileExists(atPath: moduleMapPath), + "Should exist a modulemap" + ) + let moduleMapContents = try XCTUnwrap(fileManager.contents(atPath: moduleMapPath).flatMap { String(decoding: $0, as: UTF8.self) }) + XCTAssertEqual( + moduleMapContents, + """ + framework module some_lib { + umbrella header "some_lib.h" + export * + } + """, + "modulemap should be generated" + ) + + XCTAssertTrue(fileManager.fileExists(atPath: xcFramework.path), + "Should create \(library).xcframework") + XCTAssertFalse(fileManager.fileExists(atPath: versionFile.path), + "Should not create .\(library).version in create mode") + } + } + func testBuildClangPackageWithCustomModuleMap() async throws { let runner = Runner( mode: .createPackage,