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

Task / Enable root package target caching #146

Merged
merged 14 commits into from
Oct 9, 2024
49 changes: 41 additions & 8 deletions Sources/ScipioKit/Producer/Cache/CacheSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import Foundation
import ScipioStorage
import TSCBasic
import struct TSCUtility.Version
@preconcurrency import class PackageGraph.PinsStore
import Algorithms
@preconcurrency import PackageGraph
import PackageModel
import SourceControl

private let jsonEncoder = {
let encoder = JSONEncoder()
Expand Down Expand Up @@ -251,7 +253,7 @@ struct CacheSystem: Sendable {

func calculateCacheKey(of target: CacheTarget) async throws -> SwiftPMCacheKey {
let targetName = target.buildProduct.target.name
let pin = try retrievePin(product: target.buildProduct)
let pin = try retrievePin(package: target.buildProduct.package)
let buildOptions = target.buildOptions
guard let clangVersion = try await ClangChecker().fetchClangVersion() else {
throw Error.compilerVersionNotDetected
Expand All @@ -269,14 +271,18 @@ struct CacheSystem: Sendable {
)
}

private func retrievePin(product: BuildProduct) throws -> PinsStore.Pin {
#if swift(>=5.10)
guard let pin = pinsStore.pins[product.package.identity] else {
throw Error.revisionNotDetected(product.package.manifest.displayName)
private func retrievePin(package: ResolvedPackage) throws -> PinsStore.Pin {
#if compiler(>=6.0)
guard let pin = pinsStore.pins[package.identity] ?? package.makePinFromRevision() else {
Comment on lines +275 to +276
Copy link
Collaborator

@ikesyo ikesyo Oct 7, 2024

Choose a reason for hiding this comment

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

[Question] I'm not sure why the addition is gated by #if compiler(>=6.0). Is that change not compatible with Swift 5.10 (Xcode 15.3)?

Copy link
Collaborator

@ikesyo ikesyo Oct 7, 2024

Choose a reason for hiding this comment

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

I understand that GitRepository.getCurrentTag is added in Swift 6.0 👍

https://github.com/swiftlang/swift-package-manager/blob/release/6.0.2/CHANGELOG.md#swift-60

Copy link
Contributor Author

@dmhts dmhts Oct 7, 2024

Choose a reason for hiding this comment

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

Thanks for the question. I should've mentioned that in the PR description. There are two reasons for this: GitRepository.getCurrentTag is only available in SwiftPM's release/6.0 branch. Reimplementing the function (with the respective tests) would, of course, be trivial. And here comes the second reason, as I understand it, support for Xcode 15 is going to be dropped soon, so I thought it wouldn't be worth the effort. Let me know if it makes sense.

Update: I just saw your previous post where you figured it out yourself :)

Copy link
Owner

Choose a reason for hiding this comment

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

Good point. it is correct it will cause the incompatibility.

But I told to @dmhts that we don't need to maintain the backward compatibility.
#140 (comment)

So, I think it's no problem if the compatibility is broken because Swift 5 support will be dropped soon. We don't need to make an effort to keep backward compatibility.

I'll drop the Swift 5 support.

throw Error.revisionNotDetected(package.manifest.displayName)
}
#elseif swift(>=5.10)
guard let pin = pinsStore.pins[package.identity] else {
throw Error.revisionNotDetected(package.manifest.displayName)
}
#else
guard let pin = pinsStore.pinsMap[product.package.identity] else {
throw Error.revisionNotDetected(product.package.manifest.displayName)
guard let pin = pinsStore.pinsMap[package.identity] else {
throw Error.revisionNotDetected(package.manifest.displayName)
}
#endif
return pin
Expand Down Expand Up @@ -306,3 +312,30 @@ public struct VersionFileDecoder {
)
}
}

#if compiler(>=6.0)

extension ResolvedPackage {
fileprivate func makePinFromRevision() -> PinsStore.Pin? {
let repository = GitRepository(path: path)

guard let tag = repository.getCurrentTag(), let version = Version(tag: tag) else {
return nil
}

// TODO: Even though the version requirement already covers the vast majority of cases,
// supporting `branch` and `revision` requirements should, in theory, also be possible.
return PinsStore.Pin(
packageRef: PackageReference(
identity: identity,
kind: manifest.packageKind
),
state: .version(
version,
revision: try? repository.getCurrentRevision().identifier
)
)
}
}

#endif
161 changes: 113 additions & 48 deletions Tests/ScipioKitTests/CacheSystemTests.swift
Original file line number Diff line number Diff line change
@@ -1,31 +1,22 @@
import Foundation
@testable import ScipioKit
import XCTest
import Basics

private let fixturePath = URL(fileURLWithPath: #filePath)
.deletingLastPathComponent()
.appendingPathComponent("Resources")
.appendingPathComponent("Fixtures")

final class CacheSystemTests: XCTestCase {

final class ClangCheckerTests: XCTestCase {
private let clangVersion = """
Apple clang version 14.0.0 (clang-1400.0.29.102)
Target: arm64-apple-darwin21.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
"""
private let customModuleMap = """
framework module MyTarget {
umbrella header "umbrella.h"
export *
}
"""

func testParseClangVersion() async throws {
let hook = { arguments in
XCTAssertEqual(arguments, ["/usr/bin/xcrun", "clang", "--version"])
return StubbableExecutorResult(arguments: arguments, success: self.clangVersion)
}
let clangParser = ClangChecker(executor: StubbableExecutor(executeHook: hook))
let version = try await clangParser.fetchClangVersion()
XCTAssertEqual(version, "clang-1400.0.29.102")
}
Comment on lines -19 to -27
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope it's fine that I extracted the Clang version checking logic into a separate test case.


func testEncodeCacheKey() throws {
let cacheKey = SwiftPMCacheKey(targetName: "MyTarget",
pin: .revision("111111111"),
Expand All @@ -45,38 +36,112 @@ InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault
encoder.outputFormatting = [.sortedKeys, .prettyPrinted]
let data = try encoder.encode(cacheKey)
let rawString = try XCTUnwrap(String(decoding: data, as: UTF8.self))

// swiftlint:disable line_length
let expected = """
{
"buildOptions" : {
"buildConfiguration" : "release",
"customFrameworkModuleMapContents" : "ZnJhbWV3b3JrIG1vZHVsZSBNeVRhcmdldCB7CiAgICB1bWJyZWxsYSBoZWFkZXIgInVtYnJlbGxhLmgiCiAgICBleHBvcnQgKgp9",
"enableLibraryEvolution" : true,
"extraBuildParameters" : {
"SWIFT_OPTIMIZATION_LEVEL" : "-Osize"
},
"extraFlags" : {
"swiftFlags" : [
"-D",
"SOME_FLAG"
]
},
"frameworkType" : "dynamic",
"isDebugSymbolsEmbedded" : false,
"sdks" : [
"iOS"
]
},
"clangVersion" : "clang-1400.0.29.102",
"pin" : {
"revision" : "111111111"
},
"targetName" : "MyTarget",
"xcodeVersion" : {
"xcodeBuildVersion" : "15F31d",
"xcodeVersion" : "15.4"
}
}
"""
{
"buildOptions" : {
"buildConfiguration" : "release",
"customFrameworkModuleMapContents" : "ZnJhbWV3b3JrIG1vZHVsZSBNeVRhcmdldCB7CiAgICB1bWJyZWxsYSBoZWFkZXIgInVtYnJlbGxhLmgiCiAgICBleHBvcnQgKgp9",
"enableLibraryEvolution" : true,
"extraBuildParameters" : {
"SWIFT_OPTIMIZATION_LEVEL" : "-Osize"
},
"extraFlags" : {
"swiftFlags" : [
"-D",
"SOME_FLAG"
]
},
"frameworkType" : "dynamic",
"isDebugSymbolsEmbedded" : false,
"sdks" : [
"iOS"
]
},
"clangVersion" : "clang-1400.0.29.102",
"pin" : {
"revision" : "111111111"
},
"targetName" : "MyTarget",
"xcodeVersion" : {
"xcodeBuildVersion" : "15F31d",
"xcodeVersion" : "15.4"
}
}
"""
// swiftlint:enable line_length
XCTAssertEqual(rawString, expected)
}

#if compiler(>=6.0)

func testCacheKeyCalculationForRootPackageTarget() async throws {
let fileSystem = localFileSystem
let testingPackagePath = fixturePath.appendingPathComponent("TestingPackage")
let tempTestingPackagePath = testingPackagePath.absolutePath.parentDirectory.appending(component: "temp_TestingPackage")

try fileSystem.removeFileTree(tempTestingPackagePath)
try fileSystem.copy(from: testingPackagePath.absolutePath, to: tempTestingPackagePath)

defer { try? fileSystem.removeFileTree(tempTestingPackagePath) }

let descriptionPackage = try DescriptionPackage(
packageDirectory: tempTestingPackagePath,
mode: .createPackage,
onlyUseVersionsFromResolvedFile: false
)
let cacheSystem = CacheSystem(
pinsStore: try descriptionPackage.workspace.pinsStore.load(),
outputDirectory: FileManager.default.temporaryDirectory.appendingPathComponent("XCFrameworks"),
storage: nil
)
let testingPackage = descriptionPackage
.graph
.packages
.first { $0.manifest.displayName == descriptionPackage.manifest.displayName }!

let myTarget = testingPackage.modules.first { $0.name == "MyTarget" }!
let cacheTarget = CacheSystem.CacheTarget(
buildProduct: BuildProduct(
package: testingPackage,
target: myTarget
),
buildOptions: BuildOptions(
buildConfiguration: .release,
isDebugSymbolsEmbedded: false,
frameworkType: .dynamic,
sdks: [.iOS, .iOSSimulator],
extraFlags: nil,
extraBuildParameters: nil,
enableLibraryEvolution: false,
customFrameworkModuleMapContents: nil
)
)

// Ensure that the cache key cannot be calculated if the package is not in the Git repository.
do {
_ = try await cacheSystem.calculateCacheKey(of: cacheTarget)
XCTFail("A cache key should not be possible to calculate if the package is not in a repository.")
} catch let error as CacheSystem.Error {
XCTAssertEqual(error.errorDescription, "Repository version is not detected for \(descriptionPackage.name).")
} catch {
XCTFail("Wrong error type.")
}
giginet marked this conversation as resolved.
Show resolved Hide resolved

// Ensure that the cache key is properly calculated when the package is in a repository with the correct tag."
let processExecutor: Executor = ProcessExecutor()
try await processExecutor.execute(["git", "init", tempTestingPackagePath.pathString])
try await processExecutor.execute(["git", "-C", tempTestingPackagePath.pathString, "add", tempTestingPackagePath.pathString])
try await processExecutor.execute(["git", "-C", tempTestingPackagePath.pathString, "commit", "-m", "Initial commit"])
try await processExecutor.execute(["git", "-C", tempTestingPackagePath.pathString, "tag", "v1.1"])

let cacheKey = try await cacheSystem.calculateCacheKey(of: cacheTarget)

XCTAssertEqual(cacheKey.targetName, myTarget.name)
XCTAssertEqual(cacheKey.pin.description, "1.1.0")
}

#endif

}
21 changes: 21 additions & 0 deletions Tests/ScipioKitTests/ClangCheckerTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
@testable import ScipioKit
import XCTest

final class ClangCheckerTests: XCTestCase {
private let clangVersion = """
Apple clang version 14.0.0 (clang-1400.0.29.102)
Target: arm64-apple-darwin21.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
"""

func testParseClangVersion() async throws {
let hook = { arguments in
XCTAssertEqual(arguments, ["/usr/bin/xcrun", "clang", "--version"])
return StubbableExecutorResult(arguments: arguments, success: self.clangVersion)
}
let clangParser = ClangChecker(executor: StubbableExecutor(executeHook: hook))
let version = try await clangParser.fetchClangVersion()
XCTAssertEqual(version, "clang-1400.0.29.102")
}
}
Loading