Skip to content

Commit

Permalink
Only write module map files if the contents change. (#6212)
Browse files Browse the repository at this point in the history 
* Only write module map files if the contents change.

Fixes #6211

* Linting

* Update Sources/TuistLoader/SwiftPackageManager/SwiftPackageManagerModuleMapGenerator.swift

Co-authored-by: Daniele Formichelli <df@bendingspoons.com>

* Update callsites

---------

Co-authored-by: Daniele Formichelli <df@bendingspoons.com>
  • Loading branch information
@waltflanagan @danieleformichelli
waltflanagan and danieleformichelli committed Apr 22, 2024
1 parent 2573944 commit 01eeb8f
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 44 deletions.
43 changes: 35 additions & 8 deletions Sources/TuistLoader/SwiftPackageManager/SwiftPackageManagerModuleMapGenerator.swift
View file
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
94 changes: 58 additions & 36 deletions Tests/TuistLoaderTests/SwiftPackageManager/SwiftPackageManagerModuleMapGeneratorTests.swift
View file
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
}
}

0 comments on commit 01eeb8f

Please sign in to comment.