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

Skip rewriting modulemaps if not changed to fix issues with pch #6212

Merged
merged 5 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import TSCBasic
import TuistCore
import TuistSupport

/// The type of modulemap file
Expand Down Expand Up @@ -43,7 +44,11 @@ public protocol SwiftPackageManagerModuleMapGenerating {
}

public final class SwiftPackageManagerModuleMapGenerator: SwiftPackageManagerModuleMapGenerating {
public init() {}
private let contentHasher: ContentHashing

public init(contentHasher: ContentHashing = ContentHasher()) {
self.contentHasher = contentHasher
}

// swiftlint:disable:next function_body_length
public func generate(
Expand Down Expand Up @@ -81,9 +86,12 @@ public final class SwiftPackageManagerModuleMapGenerator: SwiftPackageManagerMod
if let customModuleMapPath {
return .custom(customModuleMapPath, umbrellaHeaderPath: umbrellaHeaderPath)
}
try FileHandler.shared.write(
umbrellaHeaderModuleMap(umbrellaHeaderPath: umbrellaHeaderPath, sanitizedModuleName: sanitizedModuleName),
path: generatedModuleMapPath,
try writeIfDifferent(
moduleMapContent: umbrellaHeaderModuleMap(
umbrellaHeaderPath: umbrellaHeaderPath,
sanitizedModuleName: sanitizedModuleName
),
to: generatedModuleMapPath,
atomically: true
)
// If 'PublicHeadersDir/ModuleName.h' exists, then use it as the umbrella header.
Expand All @@ -92,9 +100,12 @@ public final class SwiftPackageManagerModuleMapGenerator: SwiftPackageManagerMod
if let customModuleMapPath {
return .custom(customModuleMapPath, umbrellaHeaderPath: nestedUmbrellaHeaderPath)
}
try FileHandler.shared.write(
umbrellaHeaderModuleMap(umbrellaHeaderPath: nestedUmbrellaHeaderPath, sanitizedModuleName: sanitizedModuleName),
path: generatedModuleMapPath,
try writeIfDifferent(
moduleMapContent: umbrellaHeaderModuleMap(
umbrellaHeaderPath: nestedUmbrellaHeaderPath,
sanitizedModuleName: sanitizedModuleName
),
to: generatedModuleMapPath,
atomically: true
)
// If 'PublicHeadersDir/ModuleName/ModuleName.h' exists, then use it as the umbrella header.
Expand All @@ -114,13 +125,29 @@ public final class SwiftPackageManagerModuleMapGenerator: SwiftPackageManagerMod
export *
}
"""
try FileHandler.shared.write(generatedModuleMapContent, path: generatedModuleMapPath, atomically: true)
try writeIfDifferent(moduleMapContent: generatedModuleMapContent, to: generatedModuleMapPath, atomically: true)

return .directory(moduleMapPath: generatedModuleMapPath, umbrellaDirectory: publicHeadersPath)
} else {
return .none
}
}

/// Write our modulemap to disk if it is distinct from what already exists.
/// This addresses an issue with dependencies that are included in a precompiled header.
/// https://github.com/tuist/tuist/issues/6211
/// - Parameters:
/// - moduleMapContent: contents of the moduleMap file to write
/// - path: destination to write file contents to
/// - atomically: whether to write atomically
func writeIfDifferent(moduleMapContent: String, to path: AbsolutePath, atomically: Bool) throws {
let newContentHash = try contentHasher.hash(moduleMapContent)
let currentContentHash = try? contentHasher.hash(path: path)
if currentContentHash != newContentHash {
try FileHandler.shared.write(moduleMapContent, path: path, atomically: true)
}
}

static func customModuleMapPath(publicHeadersPath: AbsolutePath) throws -> AbsolutePath? {
guard FileHandler.shared.exists(publicHeadersPath) else { return nil }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import TSCBasic
import TuistCoreTesting
import TuistSupportTesting
import XCTest

@testable import TuistLoader

class SwiftPackageManagerModuleMapGeneratorTests: TuistTestCase {
private var subject: SwiftPackageManagerModuleMapGenerator!
private var hasher: MockContentHasher!

override func setUp() {
super.setUp()
subject = SwiftPackageManagerModuleMapGenerator()
hasher = MockContentHasher()
subject = SwiftPackageManagerModuleMapGenerator(contentHasher: hasher)
}

override func tearDown() {
Expand Down Expand Up @@ -40,7 +43,7 @@ class SwiftPackageManagerModuleMapGeneratorTests: TuistTestCase {
}

private func test_generate(for moduleMap: ModuleMap) throws {
var writeCalled = false
var writeCount = 0
fileHandler.stubContentsOfDirectory = { _ in
switch moduleMap {
case .none:
Expand Down Expand Up @@ -81,41 +84,12 @@ class SwiftPackageManagerModuleMapGeneratorTests: TuistTestCase {
}
}
fileHandler.stubWrite = { content, path, atomically in
writeCalled = true
let expectedContent: String
switch moduleMap {
case .none, .custom:
writeCount += 1
guard let expectedContent = self.expectedContent(for: moduleMap) else {
XCTFail("FileHandler.write should not be called")
return
case let .header(umbrellaHeaderPath, moduleMapPath: _):
if umbrellaHeaderPath.parentDirectory.basename == "Module" {
expectedContent = """
framework module Module {
umbrella header "/Absolute/Public/Headers/Path/Module/Module.h"

export *
module * { export * }
}
"""
} else {
expectedContent = """
framework module Module {
umbrella header "/Absolute/Public/Headers/Path/Module.h"

export *
module * { export * }
}
"""
}
case .directory:
expectedContent = """
module Module {
umbrella "/Absolute/Public/Headers/Path"
export *
}

"""
}

XCTAssertEqual(content, expectedContent)
XCTAssertEqual(path, "/Absolute/PackageDir/Derived/Module.modulemap")
XCTAssertTrue(atomically)
Expand All @@ -125,12 +99,60 @@ class SwiftPackageManagerModuleMapGeneratorTests: TuistTestCase {
moduleName: "Module",
publicHeadersPath: "/Absolute/Public/Headers/Path"
)

// Set hasher for path on disk
hasher.stubHashForPath["/Absolute/PackageDir/Derived/Module.modulemap"] = try hasher
.hash(expectedContent(for: moduleMap) ?? "")
// generate a 2nd time to validate that we dont write content that is already on disk
let _ = try subject.generate(
packageDirectory: "/Absolute/PackageDir",
moduleName: "Module",
publicHeadersPath: "/Absolute/Public/Headers/Path"
)

XCTAssertEqual(got, moduleMap)
switch moduleMap {
case .none, .custom:
XCTAssertFalse(writeCalled)
XCTAssertEqual(writeCount, 0)
case .directory, .header:
XCTAssertTrue(writeCalled)
XCTAssertEqual(writeCount, 1)
}
}

private func expectedContent(for moduleMap: ModuleMap) -> String? {
let expectedContent: String
switch moduleMap {
case .none, .custom:
return nil
case let .header(umbrellaHeaderPath, moduleMapPath: _):
if umbrellaHeaderPath.parentDirectory.basename == "Module" {
expectedContent = """
framework module Module {
umbrella header "/Absolute/Public/Headers/Path/Module/Module.h"

export *
module * { export * }
}
"""
} else {
expectedContent = """
framework module Module {
umbrella header "/Absolute/Public/Headers/Path/Module.h"

export *
module * { export * }
}
"""
}
case .directory:
expectedContent = """
module Module {
umbrella "/Absolute/Public/Headers/Path"
export *
}

"""
}
return expectedContent
}
}