[SwiftDriver] Part III - Introduce synchronization between swift-frontend invocations

README.md

@@ -45,6 +45,7 @@ _XCRemoteCache is a remote cache tool for Xcode projects. It reuses target artif
 - [Limitations](#limitations)
 - [FAQ](#faq)
 - [Development](#development)
+- [Architectural Designs](#architectural-designs)
 - [Release](#release)
   * [Releasing CocoaPods plugin](#releasing-cocoapods-plugin)
   * [Building release package](#building-release-package)
@@ -291,7 +292,7 @@ where
 
 ```shell
 ditto "${SCRIPT_INPUT_FILE_0}" "${SCRIPT_OUTPUT_FILE_0}"
-[ -f "${SCRIPT_INPUT_FILE_1}" ] && ditto "${SCRIPT_INPUT_FILE_1}" "${SCRIPT_OUTPUT_FILE_1}" || rm "${SCRIPT_OUTPUT_FILE_1}"
+[ -f "${SCRIPT_INPUT_FILE_1}" ] && ditto "${SCRIPT_INPUT_FILE_1}" "${SCRIPT_OUTPUT_FILE_1}" || rm -f "${SCRIPT_OUTPUT_FILE_1}"
 ```
 
 where
@@ -469,6 +470,10 @@ Follow the [FAQ](docs/FAQ.md) page.
 
 Follow the [Development](docs/Development.md) guide. It has all the information on how to get started.
 
+## Architectural designs
+
+Follow the [Architectural designs](docs/design/ArchitecturalDesigns.md) document that describes and documents XCRemoteCache designs and implementation details.
+
 ## Release 
 
 To release a version, in [Releases](https://github.com/spotify/XCRemoteCache/releases) draft a new release with `v0.3.0{-rc0}` tag format. 

Sources/XCRemoteCache/Commands/Postbuild/PostbuildContext.swift

@@ -89,6 +89,9 @@ public struct PostbuildContext {
     var publicHeadersFolderPath: URL?
     /// XCRemoteCache is explicitly disabled
     let disabled: Bool
+    /// The LLBUILD_BUILD_ID ENV that describes the compilation identifier
+    /// it is used in the swift-frontend flow
+    let llbuildIdLockFile: URL
 }
 
 extension PostbuildContext {
@@ -149,5 +152,10 @@ extension PostbuildContext {
             publicHeadersFolderPath = builtProductsDir.appendingPathComponent(publicHeadersPath)
         }
         disabled = try env.readEnv(key: "XCRC_DISABLED") ?? false
+        let llbuildId: String = try env.readEnv(key: "LLBUILD_BUILD_ID")
+        llbuildIdLockFile = SwiftFrontendContext.buildLlbuildIdSharedLockUrl(
+            llbuildId: llbuildId,
+            tmpDir: targetTempDir
+        )
     }
 }

Sources/XCRemoteCache/Commands/Postbuild/XCPostbuild.swift

@@ -60,6 +60,7 @@ public class XCPostbuild {
             dependenciesWriter: FileDependenciesWriter.init,
             dependenciesReader: FileDependenciesReader.init,
             markerWriter: NoopMarkerWriter.init,
+            llbuildLockFile: context.llbuildIdLockFile,
             fileManager: fileManager
         )
 

Sources/XCRemoteCache/Commands/Prebuild/PrebuildContext.swift

@@ -48,6 +48,9 @@ public struct PrebuildContext {
     let overlayHeadersPath: URL
     /// XCRemoteCache is explicitly disabled
     let disabled: Bool
+    /// The LLBUILD_BUILD_ID ENV that describes the compilation identifier
+    /// it is used in the swift-frontend flow
+    let llbuildIdLockFile: URL
 }
 
 extension PrebuildContext {
@@ -72,5 +75,10 @@ extension PrebuildContext {
         /// Note: The file has yaml extension, even it is in the json format
         overlayHeadersPath = targetTempDir.appendingPathComponent("all-product-headers.yaml")
         disabled = try env.readEnv(key: "XCRC_DISABLED") ?? false
+        let llbuildId: String = try env.readEnv(key: "LLBUILD_BUILD_ID")
+        llbuildIdLockFile = SwiftFrontendContext.buildLlbuildIdSharedLockUrl(
+            llbuildId: llbuildId,
+            tmpDir: targetTempDir
+        )
     }
 }

Sources/XCRemoteCache/Commands/Prebuild/XCPrebuild.swift

@@ -55,6 +55,7 @@ public class XCPrebuild {
             dependenciesWriter: FileDependenciesWriter.init,
             dependenciesReader: FileDependenciesReader.init,
             markerWriter: lazyMarkerWriterFactory,
+            llbuildLockFile: context.llbuildIdLockFile,
             fileManager: fileManager
         )
 

Sources/XCRemoteCache/Commands/SwiftFrontend/SwiftFrontendContext.swift

@@ -0,0 +1,42 @@
+// Copyright (c) 2023 Spotify AB.
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import Foundation
+
+struct SwiftFrontendContext {
+    /// File lock used for synchronizing multiple invocations
+    let invocationLockFile: URL
+}
+
+extension SwiftFrontendContext {
+    init(_ swiftcContext: SwiftcContext, env: [String: String]) throws {
+        /// The LLBUILD_BUILD_ID ENV that describes the swiftc (parent) invocation
+        let llbuildId: String = try env.readEnv(key: "LLBUILD_BUILD_ID")
+        invocationLockFile = Self.self.buildLlbuildIdSharedLockUrl(
+            llbuildId: llbuildId,
+            tmpDir: swiftcContext.tempDir
+        )
+    }
+
+    /// Generate the filename to be used to sycnhronize mutliple swift-frontend invocations
+    /// The same file is used in prebuild, xcswift-frontend and postbuild (to clean it up)
+    static func buildLlbuildIdSharedLockUrl(llbuildId: String, tmpDir: URL) -> URL {
+        return tmpDir.appendingPathComponent(llbuildId).appendingPathExtension("lock")
+    }
+}

Sources/XCRemoteCache/Commands/SwiftFrontend/SwiftFrontendOrchestrator.swift

@@ -32,14 +32,105 @@ protocol SwiftFrontendOrchestrator {
 /// For the compilation action, tries to ackquire a lock and waits until the "emit-module" makes a decision
 /// if the compilation should be skipped and a "mocking" should used instead
 class CommonSwiftFrontendOrchestrator {
+    /// Content saved to the shared file
+    /// Safe to use forced unwrapping
+    private static let emitModuleContent = "done".data(using: .utf8)!
+
+    enum Action {
+        case emitModule
+        case compile
+    }
     private let mode: SwiftcContext.SwiftcMode
+    private let action: Action
+    private let lockAccessor: ExclusiveFileAccessor
+    private let maxLockTimeout: TimeInterval
 
-    init(mode: SwiftcContext.SwiftcMode) {
+    init(
+        mode: SwiftcContext.SwiftcMode,
+        action: Action,
+        lockAccessor: ExclusiveFileAccessor,
+        maxLockTimeout: TimeInterval
+    ) {
         self.mode = mode
+        self.action = action
+        self.lockAccessor = lockAccessor
+        self.maxLockTimeout = maxLockTimeout
     }
 
     func run(criticalSection: () throws -> Void) throws {
-        // TODO: implement synchronization in a separate PR
-        try criticalSection()
+        guard case .consumer(commit: .available) = mode else {
+            // no need to lock anything - just allow fallbacking to the `swiftc or swift-frontend`
+            // for a producer or a consumer where RC is disabled (we have already caught the
+            // cache miss)
+            try criticalSection()
+            return
+        }
+        try executeMockAttemp(criticalSection: criticalSection)
+    }
+
+    private func executeMockAttemp(criticalSection: () throws -> Void) throws {
+        switch action {
+        case .emitModule:
+            try validateEmitModuleStep(criticalSection: criticalSection)
+        case .compile:
+            try waitForEmitModuleLock(criticalSection: criticalSection)
+        }
+    }
+
+
+    /// Foor emit-module, wrap the critical section with the shared lock so other processes (compilation)
+    /// have to wait until emit-module finishes
+    /// Once the emit-module is done, the "magical" string is saved to the file and the lock is released
+    ///
+    /// Note: The design of wrapping the entire "emit-module" has a small performance downside if inside
+    /// the critical section, the code realizes that remote cache cannot be used
+    /// (in practice - a new file has been added)
+    /// None of compilation process (so with '-c' args) can continue until the entire emit-module logic finishes
+    /// Because it is expected to happen no that often and emit-module is usually quite fast, this makes the
+    /// implementation way simpler. If we ever want to optimize it, we should release the lock as early
+    /// as we know, the remote cache cannot be used. Then all other compilation process (-c) can run
+    /// in parallel with emit-module
+    private func validateEmitModuleStep(criticalSection: () throws -> Void) throws {
+        debugLog("starting the emit-module step: locking")
+        try lockAccessor.exclusiveAccess { handle in
+            debugLog("starting the emit-module step: locked")
+            // writing to the file content proactively - incase the critical section never returns
+            // (in case of a fallback to the local compilation), all awaiting swift-frontent processes
+            // will be immediatelly unblocked
+            handle.write(Self.self.emitModuleContent)
+            try criticalSection()
+            debugLog("lock file emit-module criticial end")
+        }
+    }
+
+    /// Locks a shared file in a loop until its content is non-empty - meaning the "parent" emit-module
+    /// has already finished
+    private func waitForEmitModuleLock(criticalSection: () throws -> Void) throws {
+        // emit-module process should really quickly obtain a lock (it is always invoked
+        // by Xcode as a first process)
+        var executed = false
+        let startingDate = Date()
+        while !executed {
+            debugLog("lock file compilation trying to acquire a lock ....")
+            try lockAccessor.exclusiveAccess { handle in
+                if !handle.availableData.isEmpty {
+                    // the file is not empty so the emit-module process is done with the "check"
+                    debugLog("swift-frontend lock file is unlocked for compilation")
+                    try criticalSection()
+                    executed = true
+                } else {
+                    debugLog("swift-frontend lock file is not ready for compilation")
+                }
+            }
+            // When a max locking time is achieved, execute anyway
+            if !executed && Date().timeIntervalSince(startingDate) > self.maxLockTimeout {
+                errorLog("""
+                Executing command \(action) without lock synchronization. That may be cause by the\
+                crashed or extremly long emit-module. Contact XCRemoteCache authors about this error.
+                """)
+                try criticalSection()
+                executed = true
+            }
+        }
     }
 }

Sources/XCRemoteCache/Commands/SwiftFrontend/XCSwiftFrontend.swift

@@ -55,6 +55,26 @@ public class XCSwiftFrontend: XCSwiftAbstract<SwiftFrontendArgInput> {
     }
 
     override public func run() throws {
-        // TODO: implement in a follow-up PR
+        do {
+            let (_, context) = try buildContext()
+
+            let frontendContext = try SwiftFrontendContext(context, env: env)
+            let sharedLock = ExclusiveFile(frontendContext.invocationLockFile, mode: .override)
+
+            let action: CommonSwiftFrontendOrchestrator.Action = inputArgs.emitModule ? .emitModule : .compile
+            let swiftFrontendOrchestrator = CommonSwiftFrontendOrchestrator(
+                mode: context.mode,
+                action: action,
+                lockAccessor: sharedLock,
+                maxLockTimeout: Self.self.MaxLockingTimeout
+            )
+
+            try swiftFrontendOrchestrator.run(criticalSection: super.run)
+        } catch {
+            // Splitting into 2 invocations as os_log truncates a massage
+            defaultLog("Cannot correctly orchestrate the \(command) with params \(inputArgs)")
+            defaultLog("Cannot correctly orchestrate error: \(error)")
+            throw error
+        }
     }
 }

Sources/XCRemoteCache/Dependencies/CacheModeController.swift

@@ -48,6 +48,7 @@ class PhaseCacheModeController: CacheModeController {
     private let dependenciesWriter: DependenciesWriter
     private let dependenciesReader: DependenciesReader
     private let markerWriter: MarkerWriter
+    private let llbuildLockFile: URL
     private let fileManager: FileManager
 
     init(
@@ -59,6 +60,7 @@ class PhaseCacheModeController: CacheModeController {
         dependenciesWriter: (URL, FileManager) -> DependenciesWriter,
         dependenciesReader: (URL, FileManager) -> DependenciesReader,
         markerWriter: (URL, FileManager) -> MarkerWriter,
+        llbuildLockFile: URL,
         fileManager: FileManager
     ) {
 
@@ -69,10 +71,12 @@ class PhaseCacheModeController: CacheModeController {
         let discoveryURL = tempDir.appendingPathComponent(phaseDependencyPath)
         self.dependenciesWriter = dependenciesWriter(discoveryURL, fileManager)
         self.dependenciesReader = dependenciesReader(discoveryURL, fileManager)
+        self.llbuildLockFile = llbuildLockFile
         self.markerWriter = markerWriter(modeMarker, fileManager)
     }
 
     func enable(allowedInputFiles: [URL], dependencies: [URL]) throws {
+        try cleanupLlBuildLock()
         // marker file contains filepaths that contribute to the build products
         // and should invalidate all other target steps (swiftc,libtool etc.)
         let targetSensitiveFiles = dependencies + [modeMarker, Self.xcodeSelectLink]
@@ -84,6 +88,7 @@ class PhaseCacheModeController: CacheModeController {
     }
 
     func disable() throws {
+        try cleanupLlBuildLock()
         guard !forceCached else {
             throw PhaseCacheModeControllerError.cannotUseRemoteCacheForForcedCacheMode
         }
@@ -114,4 +119,20 @@ class PhaseCacheModeController: CacheModeController {
         }
         return false
     }
+
+    // cleanup the build lock file (if exists) as the very last step of this controller
+    // this is just a non-critical cleanup step to not leave {{LLBUILD_BUILD_ID}}.lock
+    // files in $TARGET_TEMP_DIR. It is expected that both prebuild and postbuild will
+    // invoke it, to ensure:
+    // - swift-frontent synchronization is done per-target build
+    // - no .lock leftover files
+    private func cleanupLlBuildLock() throws {
+        if fileManager.fileExists(atPath: llbuildLockFile.path) {
+            do {
+                try fileManager.removeItem(at: llbuildLockFile)
+            } catch {
+                printWarning("Removing llbuild lock at \(llbuildLockFile.path) failed. Error: \(error)")
+            }
+        }
+    }
 }

Sources/xcswift-frontend/XCSwiftcFrontendMain.swift

@@ -29,7 +29,16 @@ public class XCSwiftcFrontendMain {
     // swiftlint:disable:next function_body_length cyclomatic_complexity
     public func main() {
         let env = ProcessInfo.processInfo.environment
-        let command = ProcessInfo().processName
+        // Do not invoke raw swift-frontend because that would lead to the invifnite loop
+        // swift-frontent -> xcswift-frontent -> swift-frontent
+        //
+        // Note: Returning the `swiftc` executaion here because it is possible to pass all arguments
+        // from swift-frontent to `swiftc` and swiftc will be able to redirect to swift-frontend
+        // (because the first argument is `-frontend`). If that is not a case (might change in
+        // future swift compiler versions), invoke swift-frontent from the Xcode, but that introduces
+        // a limitation that disallows custom toolchains in Xcode:
+        // $DEVELOPER_DIR/Toolchains/XcodeDefault.xctoolchain/usr/bin/{ ProcessInfo().processName}
+        let command = "swiftc"
         let args = ProcessInfo().arguments
         var compile = false
         var emitModule = false

Tests/XCRemoteCacheTests/Commands/PostbuildContextTests.swift

@@ -28,7 +28,7 @@ class PostbuildContextTests: FileXCTestCase {
         "TARGET_TEMP_DIR": "TARGET_TEMP_DIR",
         "DERIVED_FILE_DIR": "DERIVED_FILE_DIR",
         "ARCHS": "x86_64",
-        "OBJECT_FILE_DIR_normal": "/OBJECT_FILE_DIR_normal" ,
+        "OBJECT_FILE_DIR_normal": "/OBJECT_FILE_DIR_normal",
         "CONFIGURATION": "CONFIGURATION",
         "PLATFORM_NAME": "PLATFORM_NAME",
         "XCODE_PRODUCT_BUILD_VERSION": "XCODE_PRODUCT_BUILD_VERSION",
@@ -45,6 +45,7 @@ class PostbuildContextTests: FileXCTestCase {
         "DERIVED_SOURCES_DIR": "DERIVED_SOURCES_DIR",
         "CURRENT_VARIANT": "normal",
         "PUBLIC_HEADERS_FOLDER_PATH": "/usr/local/include",
+        "LLBUILD_BUILD_ID": "1",
     ]
 
     override func setUpWithError() throws {
@@ -186,4 +187,17 @@ class PostbuildContextTests: FileXCTestCase {
 
         XCTAssertFalse(context.disabled)
     }
+
+    func testFailsIfLlBuildIdEnvIsMissing() throws {
+        var envs = Self.SampleEnvs
+        envs.removeValue(forKey: "LLBUILD_BUILD_ID")
+
+        XCTAssertThrowsError(try PostbuildContext(config, env: envs))
+    }
+
+    func testBuildsLockValidFileUrl() throws {
+        let context = try PostbuildContext(config, env: Self.SampleEnvs)
+
+        XCTAssertEqual(context.llbuildIdLockFile, "TARGET_TEMP_DIR/1.lock")
+    }
 }

Tests/XCRemoteCacheTests/Commands/PostbuildTests.swift

@@ -57,7 +57,8 @@ class PostbuildTests: FileXCTestCase {
         overlayHeadersPath: "",
         irrelevantDependenciesPaths: [],
         publicHeadersFolderPath: nil,
-        disabled: false
+        disabled: false,
+        llbuildIdLockFile: "/file"
     )
     private var network = RemoteNetworkClientImpl(
         NetworkClientFake(fileManager: .default),