-
Notifications
You must be signed in to change notification settings - Fork 31
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
Changes from 10 commits
1e96e3c
ba0e317
bd84919
47a6681
ef12f80
acba575
bf891e2
b9ba5d3
19c2c45
0d7d795
daf435e
3887b39
1ee3410
22f87d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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() | ||
|
@@ -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 | ||
|
@@ -269,14 +271,14 @@ struct CacheSystem: Sendable { | |
) | ||
} | ||
|
||
private func retrievePin(product: BuildProduct) throws -> PinsStore.Pin { | ||
private func retrievePin(package: ResolvedPackage) throws -> PinsStore.Pin { | ||
#if swift(>=5.10) | ||
guard let pin = pinsStore.pins[product.package.identity] else { | ||
throw Error.revisionNotDetected(product.package.manifest.displayName) | ||
guard let pin = pinsStore.pins[package.identity] ?? package.pin 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] ?? package.pin else { | ||
throw Error.revisionNotDetected(package.manifest.displayName) | ||
} | ||
#endif | ||
return pin | ||
|
@@ -306,3 +308,28 @@ public struct VersionFileDecoder { | |
) | ||
} | ||
} | ||
|
||
fileprivate extension ResolvedPackage { | ||
|
||
var pin: PinsStore.Pin? { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel This property generates a pin it is using How about it be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, I like using the optional factory-like |
||
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 | ||
) | ||
) | ||
} | ||
|
||
} |
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"), | ||
|
@@ -46,37 +37,103 @@ InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault | |
let data = try encoder.encode(cacheKey) | ||
let rawString = try XCTUnwrap(String(decoding: data, as: UTF8.self)) | ||
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" | ||
} | ||
} | ||
""" | ||
XCTAssertEqual(rawString, expected) | ||
} | ||
|
||
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") | ||
} | ||
} |
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") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be
private extension
orfileprivate func
.I prefer to add the accessor on the function sides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.