Skip to content

[Preview] Fix preview support for swift caching build #562

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+43 −20
Open

[Preview] Fix preview support for swift caching build #562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions Sources/SWBCore/SpecImplementations/Tools/SwiftCompiler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ public struct SwiftDriverPayload: Serializable, TaskPayload, Encodable {
public let uniqueID: String
public let compilerLocation: LibSwiftDriver.CompilerLocation
public let moduleName: String
public let outputPrefix: String
public let tempDirPath: Path
public let explicitModulesTempDirPath: Path
public let variant: String
Expand All @@ -352,10 +353,11 @@ public struct SwiftDriverPayload: Serializable, TaskPayload, Encodable {
public let dependencyFilteringRootPath: Path?
public let verifyScannerDependencies: Bool

internal init(uniqueID: String, compilerLocation: LibSwiftDriver.CompilerLocation, moduleName: String, tempDirPath: Path, explicitModulesTempDirPath: Path, variant: String, architecture: String, eagerCompilationEnabled: Bool, explicitModulesEnabled: Bool, commandLine: [String], ruleInfo: [String], isUsingWholeModuleOptimization: Bool, casOptions: CASOptions?, reportRequiredTargetDependencies: BooleanWarningLevel, linkerResponseFilePath: Path?, dependencyFilteringRootPath: Path?, verifyScannerDependencies: Bool) {
internal init(uniqueID: String, compilerLocation: LibSwiftDriver.CompilerLocation, moduleName: String, outputPrefix: String, tempDirPath: Path, explicitModulesTempDirPath: Path, variant: String, architecture: String, eagerCompilationEnabled: Bool, explicitModulesEnabled: Bool, commandLine: [String], ruleInfo: [String], isUsingWholeModuleOptimization: Bool, casOptions: CASOptions?, reportRequiredTargetDependencies: BooleanWarningLevel, linkerResponseFilePath: Path?, dependencyFilteringRootPath: Path?, verifyScannerDependencies: Bool) {
self.uniqueID = uniqueID
self.compilerLocation = compilerLocation
self.moduleName = moduleName
self.outputPrefix = outputPrefix
self.tempDirPath = tempDirPath
self.explicitModulesTempDirPath = explicitModulesTempDirPath
self.variant = variant
Expand All @@ -373,10 +375,11 @@ public struct SwiftDriverPayload: Serializable, TaskPayload, Encodable {
}

public init(from deserializer: any Deserializer) throws {
try deserializer.beginAggregate(17)
try deserializer.beginAggregate(18)
self.uniqueID = try deserializer.deserialize()
self.compilerLocation = try deserializer.deserialize()
self.moduleName = try deserializer.deserialize()
self.outputPrefix = try deserializer.deserialize()
self.tempDirPath = try deserializer.deserialize()
self.explicitModulesTempDirPath = try deserializer.deserialize()
self.variant = try deserializer.deserialize()
Expand All @@ -394,10 +397,11 @@ public struct SwiftDriverPayload: Serializable, TaskPayload, Encodable {
}

public func serialize<T>(to serializer: T) where T : Serializer {
serializer.serializeAggregate(17) {
serializer.serializeAggregate(18) {
serializer.serialize(self.uniqueID)
serializer.serialize(self.compilerLocation)
serializer.serialize(self.moduleName)
serializer.serialize(self.outputPrefix)
serializer.serialize(self.tempDirPath)
serializer.serialize(self.explicitModulesTempDirPath)
serializer.serialize(self.variant)
Expand Down Expand Up @@ -2502,7 +2506,7 @@ public final class SwiftCompilerSpec : CompilerSpec, SpecIdentifierType, SwiftDi
let explicitModuleBuildEnabled = await swiftExplicitModuleBuildEnabled(cbc.producer, cbc.scope, delegate)
let verifyScannerDependencies = explicitModuleBuildEnabled && cbc.scope.evaluate(BuiltinMacros.SWIFT_DEPENDENCY_REGISTRATION_MODE) == .verifySwiftDependencyScanner

return SwiftDriverPayload(uniqueID: uniqueID, compilerLocation: compilerLocation, moduleName: scope.evaluate(BuiltinMacros.SWIFT_MODULE_NAME), tempDirPath: tempDirPath, explicitModulesTempDirPath: explicitModulesTempDirPath, variant: variant, architecture: arch, eagerCompilationEnabled: eagerCompilationEnabled(args: args, scope: scope, compilationMode: compilationMode, isUsingWholeModuleOptimization: isUsingWholeModuleOptimization), explicitModulesEnabled: explicitModuleBuildEnabled, commandLine: commandLine, ruleInfo: ruleInfo, isUsingWholeModuleOptimization: isUsingWholeModuleOptimization, casOptions: casOptions, reportRequiredTargetDependencies: scope.evaluate(BuiltinMacros.DIAGNOSE_MISSING_TARGET_DEPENDENCIES), linkerResponseFilePath: linkerResponseFilePath, dependencyFilteringRootPath: cbc.producer.sdk?.path, verifyScannerDependencies: verifyScannerDependencies)
return SwiftDriverPayload(uniqueID: uniqueID, compilerLocation: compilerLocation, moduleName: scope.evaluate(BuiltinMacros.SWIFT_MODULE_NAME), outputPrefix: scope.evaluate(BuiltinMacros.TARGET_NAME) + compilationMode.moduleBaseNameSuffix, tempDirPath: tempDirPath, explicitModulesTempDirPath: explicitModulesTempDirPath, variant: variant, architecture: arch, eagerCompilationEnabled: eagerCompilationEnabled(args: args, scope: scope, compilationMode: compilationMode, isUsingWholeModuleOptimization: isUsingWholeModuleOptimization), explicitModulesEnabled: explicitModuleBuildEnabled, commandLine: commandLine, ruleInfo: ruleInfo, isUsingWholeModuleOptimization: isUsingWholeModuleOptimization, casOptions: casOptions, reportRequiredTargetDependencies: scope.evaluate(BuiltinMacros.DIAGNOSE_MISSING_TARGET_DEPENDENCIES), linkerResponseFilePath: linkerResponseFilePath, dependencyFilteringRootPath: cbc.producer.sdk?.path, verifyScannerDependencies: verifyScannerDependencies)
}

func constructSwiftResponseFileTask(path: Path) {
Expand Down Expand Up @@ -3397,10 +3401,12 @@ public final class SwiftCompilerSpec : CompilerSpec, SpecIdentifierType, SwiftDi
removeWithParameter(arg)
}

// We need to ignore precompiled headers due to:
// For some old version of swift driver, the output path for bridging header pch is not stable,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we have any compatibility requirement for older swift-driver? If we only need to support recent swift-driver, this workaround can be removed to make preview build faster

Copy link
Collaborator

Choose a reason for hiding this comment

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

Generally speaking I try not to immediately break compatibility with old drivers to minimize revlock when building the package for local development. But supporting a driver that's more than a few weeks old is a non-goal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change in swift-driver I am talking about is: swiftlang/swift-driver#1832

It has been few months but not in official release yet.

// we need to disable bridging header pch when caching or bridging header chaining is not enabled as a workaround:
// rdar://126212044 ([JIT] iOS test Failures: Thunk build failure, unable to read PCH file)
removeWithPrefix("-cache-compile-job")
commandLine.append("-disable-bridging-pch")
if !commandLine.contains("-cache-compile-job") || !commandLine.contains("-auto-bridging-header-chaining") {
commandLine.append("-disable-bridging-pch")
}

if payload.previewStyle == .dynamicReplacement {
for (arg, newValue) in [
Expand Down Expand Up @@ -3440,6 +3446,7 @@ public final class SwiftCompilerSpec : CompilerSpec, SpecIdentifierType, SwiftDi
}

let selectedInputPath: Path
let newVFSOverlayPath: Path?
if payload.previewStyle == .xojit {
// Also pass the auxiliary Swift files.
commandLine.append(contentsOf: originalInputs.map(\.str))
Expand All @@ -3448,7 +3455,8 @@ public final class SwiftCompilerSpec : CompilerSpec, SpecIdentifierType, SwiftDi
if let driverPayload = payload.driverPayload {
do {
// Inject the thunk source into the output file map
let map = SwiftOutputFileMap(files: [sourceFile.str: .init(object: outputPath.str)])
let pchPath = driverPayload.tempDirPath.join(driverPayload.outputPrefix + "-primary-Bridging-header.pch")
let map = SwiftOutputFileMap(files: [sourceFile.str: .init(object: outputPath.str), "": .init(pch: pchPath.str)])
let newOutputFileMap = driverPayload.tempDirPath.join(UUID().uuidString)
try fs.createDirectory(newOutputFileMap.dirname, recursive: true)
try fs.write(newOutputFileMap, contents: ByteString(JSONEncoder(outputFormatting: [.prettyPrinted, .sortedKeys, .withoutEscapingSlashes]).encode(map)))
Expand All @@ -3457,19 +3465,20 @@ public final class SwiftCompilerSpec : CompilerSpec, SpecIdentifierType, SwiftDi
// rdar://127735418 ([JIT] Emit a vfsoverlay for JIT preview thunk compiler arguments so clients can specify the original file path when substituting contents)
let vfs = VFS()
vfs.addMapping(sourceFile, externalContents: inputPath)
let newVFSOverlayPath = driverPayload.tempDirPath.join("vfsoverlay-\(inputPath.basename).json")
newVFSOverlayPath = driverPayload.tempDirPath.join("vfsoverlay-\(inputPath.basename).json")
try fs.createDirectory(newOutputFileMap.dirname, recursive: true)
let overlay = try vfs.toVFSOverlay().propertyListItem.asJSONFragment().asString
try fs.write(newVFSOverlayPath, contents: ByteString(encodingAsUTF8: overlay))

commandLine.append(contentsOf: ["-vfsoverlay", newVFSOverlayPath.str])
try fs.write(newVFSOverlayPath!, contents: ByteString(encodingAsUTF8: overlay))
} catch {
return []
}
} else {
newVFSOverlayPath = nil
}
}
else {
selectedInputPath = inputPath
newVFSOverlayPath = nil
commandLine.append(contentsOf: [inputPath.str])
}

Expand Down Expand Up @@ -3516,6 +3525,21 @@ public final class SwiftCompilerSpec : CompilerSpec, SpecIdentifierType, SwiftDi
casOptions: driverPayload.casOptions
) {
commandLine = newCommandLine
// For swift caching jobs, add extra flags.
if commandLine.contains("-cache-compile-job") {
// Ideally, we should ask if swift-frontend supports the flag but we can only ask driver for an approximation.
if LibSwiftDriver.supportsDriverFlag(spelled: "-module-import-from-cas") {
commandLine.append("-module-import-from-cas")
}
// Then drop the cache build flag to do uncached preview compilation.
commandLine.removeAll {
$0 == "-cache-compile-job"
}
}
// Add vfsoverlay after the driver invocation as it can affect the module dependencies, causing modules from regular builds not being reused here.
if let vfsOverlay = newVFSOverlayPath {
commandLine.append(contentsOf: ["-vfsoverlay", vfsOverlay.str])
}
} else {
commandLine = []
}
Expand Down
12 changes: 6 additions & 6 deletions Tests/SWBBuildSystemTests/PreviewsBuildOperationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,6 @@ fileprivate struct PreviewsBuildOperationTests: CoreBasedTests {
"\(srcRoot.str)/build/Debug-iphonesimulator",
"-F",
"\(srcRoot.str)/build/Debug-iphonesimulator",
"-vfsoverlay",
"\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/vfsoverlay-main.selection.preview-thunk.swift.json",
"-no-color-diagnostics",
"-g",
"-debug-info-format=dwarf",
Expand Down Expand Up @@ -278,7 +276,9 @@ fileprivate struct PreviewsBuildOperationTests: CoreBasedTests {
"-target-sdk-name",
"iphonesimulator\(core.loadSDK(.iOSSimulator).defaultDeploymentTarget)",
"-o",
"\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/main.selection.preview-thunk.o"
"\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/main.selection.preview-thunk.o",
"-vfsoverlay",
"\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/vfsoverlay-main.selection.preview-thunk.swift.json",
]
)
#expect(previewInfo.thunkInfo?.thunkSourceFile == Path("\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/main.selection.preview-thunk.swift"))
Expand Down Expand Up @@ -531,8 +531,6 @@ fileprivate struct PreviewsBuildOperationTests: CoreBasedTests {
"\(srcRoot.str)/build/Debug-iphonesimulator",
"-F",
"\(srcRoot.str)/build/Debug-iphonesimulator",
"-vfsoverlay",
"\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/vfsoverlay-File1.selection.preview-thunk.swift.json",
"-no-color-diagnostics",
"-g",
"-debug-info-format=dwarf",
Expand Down Expand Up @@ -582,7 +580,9 @@ fileprivate struct PreviewsBuildOperationTests: CoreBasedTests {
"-target-sdk-name",
"iphonesimulator\(core.loadSDK(.iOSSimulator).defaultDeploymentTarget)",
"-o",
"\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/File1.selection.preview-thunk.o"
"\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/File1.selection.preview-thunk.o",
"-vfsoverlay",
"\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/vfsoverlay-File1.selection.preview-thunk.swift.json",
]
)
#expect(previewInfo.thunkInfo?.thunkSourceFile == Path("\(srcRoot.str)/build/ProjectName.build/Debug-iphonesimulator/AppTarget.build/Objects-normal/\(results.runDestinationTargetArchitecture)/File1.selection.preview-thunk.swift"))
Expand Down
3 changes: 1 addition & 2 deletions Tests/SwiftBuildTests/GeneratePreviewInfoTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,13 @@ fileprivate struct GeneratePreviewInfoTests: CoreBasedTests {
.anySequence,
"-sdk", "\(sdkroot)",
.anySequence,
"-vfsoverlay", "\(tmpDir.str)/Test/build/Test.build/Debug-iphoneos/App.build/Objects-normal/\(activeRunDestination.targetArchitecture)/vfsoverlay-TestFile4.canary.preview-thunk.swift.json",
.anySequence,
"-Onone",
.anySequence,
"-module-name", "App",
"-target-sdk-version", "\(deploymentTarget)",
"-target-sdk-name", "iphoneos\(deploymentTarget)",
"-o", "\(tmpDir.str)/Test/build/Test.build/Debug-iphoneos/App.build/Objects-normal/\(activeRunDestination.targetArchitecture)/TestFile4.canary.preview-thunk.o",
"-vfsoverlay", "\(tmpDir.str)/Test/build/Test.build/Debug-iphoneos/App.build/Objects-normal/\(activeRunDestination.targetArchitecture)/vfsoverlay-TestFile4.canary.preview-thunk.swift.json",
.end
])
// Also spot-check that some options which were removed in SwiftCompilerSpec.generatePreviewInfo() for XOJIT are not present.
Expand Down
Loading