Merge pull request #227 from CharlieSuP1/master

prebuild should run artifactsOrganizer.prepare(artifact:) method even  if artifacts exists locally

.github/workflows/ci.yaml

@@ -13,14 +13,16 @@ jobs:
           args: --strict
 
   macOS:
-    runs-on: macos-12
+    runs-on: macos-14
     env:
-      XCODE_VERSION: ${{ '14.2' }}
+      XCODE_VERSION: ${{ '14.3.1' }}
     steps:
       - name: Select Xcode
         run: "sudo xcode-select -s /Applications/Xcode_$XCODE_VERSION.app"
       - name: Checkout
         uses: actions/checkout@v1
+      - name: Install nginx
+        run: brew install nginx
       - name: Build and Run
         run: rake build[release]
       - name: Test

.github/workflows/docs.yaml

@@ -7,9 +7,9 @@ on:
 
 jobs:
   docs:
-    runs-on: macos-12
+    runs-on: macos-14
     env:
-      XCODE_VERSION: ${{ '14.2' }}
+      XCODE_VERSION: ${{ '14.3.1' }}
     steps:
       - uses: actions/checkout@v2
       - name: Select Xcode

.github/workflows/release.yaml

@@ -6,9 +6,9 @@ on:
 jobs:
   macOS:
     name: Add macOS binaries to release
-    runs-on: macos-12
+    runs-on: macos-14
     env:
-      XCODE_VERSION: ${{ '14.2' }}
+      XCODE_VERSION: ${{ '14.3.1' }}
     steps:
       - name: Select Xcode
         run: "sudo xcode-select -s /Applications/Xcode_$XCODE_VERSION.app"

README.md

@@ -7,7 +7,6 @@ _XCRemoteCache is a remote cache tool for Xcode projects. It reuses target artif
 
 [![Build Status](https://github.com/spotify/XCRemoteCache/workflows/CI/badge.svg)](https://github.com/spotify/XCRemoteCache/workflows/CI/badge.svg)
 [![License](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](LICENSE)
-[![Slack](https://slackin.spotify.com/badge.svg)](https://slackin.spotify.com)
 [![Docs](https://github.com/spotify/XCRemoteCache/workflows/Docs/badge.svg)](https://spotify.github.io/XCRemoteCache/documentation/xcremotecache/)
 
 - [How and Why?](#how-and-why)
@@ -499,8 +498,6 @@ The zip package will be generated at `releases/XCRemoteCache.zip`.
 
 Create a [new issue](https://github.com/spotify/XCRemoteCache/issues/new) with as many details as possible.
 
-Reach us at the `#xcremotecache` channel in [Slack](https://slackin.spotify.com/).
-
 ## Contributing
 
 We feel that a welcoming community is important and we ask that you follow Spotify's 

Sources/XCRemoteCache/Artifacts/ArtifactMetaUpdater.swift

@@ -0,0 +1,72 @@
+// 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
+
+enum ArtifactMetaUpdaterError: Error {
+    /// The prebuild plugin execution was called but the local
+    /// path to the artifact directory is still unknown
+    /// Might happen that the artifact processor didn't invoke the updater's
+    /// .process() after downloading/activating an artifact
+    case artifactLocationIsUnknown
+}
+
+/// Updates the meta file in an unzipped artifact directory, by placing an up-to-date
+/// and remapped meta file. Updating the meta in the artifact allows reusing existing
+/// artifacts it a new meta.json schema has been released to the meta format, while
+/// artifacts are still backward-compatible
+class ArtifactMetaUpdater: ArtifactProcessor {
+    private var artifactLocation: URL?
+    private let metaWriter: MetaWriter
+    private let fileRemapper: FileDependenciesRemapper
+
+    init(
+        fileRemapper: FileDependenciesRemapper,
+        metaWriter: MetaWriter
+    ) {
+        self.metaWriter = metaWriter
+        self.fileRemapper = fileRemapper
+    }
+
+    /// Remembers the artifact location, used later in the plugin
+    /// - Parameter url: artifact's root directory
+    func process(rawArtifact url: URL) throws {
+        // Storing the location of the just downloaded/activated artifact
+        // Note, the `url` location already includes a meta (generated by producer
+        // while compiling and building an artifact)
+        artifactLocation = url
+    }
+
+    func process(localArtifact url: URL) throws {
+        // No need to do anything in the postbuild
+    }
+}
+
+extension ArtifactMetaUpdater: ArtifactConsumerPrebuildPlugin {
+
+    /// Updates the meta json file in a local, unzipped, artifact location. It also remaps
+    /// all paths so other steps (like actool or postbuild) don't have to do it again
+    func run(meta: MainArtifactMeta) throws {
+        guard let artifactLocation = artifactLocation else {
+            throw ArtifactMetaUpdaterError.artifactLocationIsUnknown
+        }
+        let metaURL = try metaWriter.write(meta, locationDir: artifactLocation)
+        try fileRemapper.remap(fromGeneric: metaURL)
+    }
+}

Sources/XCRemoteCache/Artifacts/ArtifactOrganizer.swift

@@ -47,6 +47,8 @@ protocol ArtifactOrganizer {
 }
 
 class ZipArtifactOrganizer: ArtifactOrganizer {
+    static let activeArtifactLocation = "active"
+
     private let cacheDir: URL
     // all processors that should "prepare" the unzipped raw artifact
     private let artifactProcessors: [ArtifactProcessor]
@@ -63,7 +65,7 @@ class ZipArtifactOrganizer: ArtifactOrganizer {
     }
 
     func getActiveArtifactLocation() -> URL {
-        return cacheDir.appendingPathComponent("active")
+        return cacheDir.appendingPathComponent(Self.self.activeArtifactLocation)
     }
 
     func getActiveArtifactFilekey() throws -> String {
@@ -90,20 +92,27 @@ class ZipArtifactOrganizer: ArtifactOrganizer {
         let destinationURL = artifact.deletingPathExtension()
         guard fileManager.fileExists(atPath: destinationURL.path) == false else {
             infoLog("Skipping artifact, already existing at \(destinationURL)")
+            try runArtifactProcessors(artifactLocation: destinationURL)
             return destinationURL
         }
-        // Uzipping to a temp file first to never leave the uncompleted zip in the final location
+        // Unzipping to a temp file first to never leave the uncompleted zip in the final location
         // when the command was interrupted (internal crash or `kill -9` signal)
         let tempDestination = destinationURL.appendingPathExtension("tmp")
         try Zip.unzipFile(artifact, destination: tempDestination, overwrite: true, password: nil)
 
-        try artifactProcessors.forEach { processor in
-            try processor.process(rawArtifact: tempDestination)
-        }
         try fileManager.moveItem(at: tempDestination, to: destinationURL)
+        try runArtifactProcessors(artifactLocation: destinationURL)
         return destinationURL
     }
 
+    /// Iterates all processor when an artifact has been just downloaded or reused from already downloaded
+    /// and previously processed location
+    private func runArtifactProcessors(artifactLocation: URL) throws {
+        try artifactProcessors.forEach { processor in
+            try processor.process(rawArtifact: artifactLocation)
+        }
+    }
+
     func activate(extractedArtifact: URL) throws {
         let activeLocationURL = getActiveArtifactLocation()
         try fileManager.spt_forceSymbolicLink(at: activeLocationURL, withDestinationURL: extractedArtifact)

Sources/XCRemoteCache/Commands/Prebuild/Prebuild.swift

@@ -91,6 +91,7 @@ class Prebuild {
             switch artifactPreparationResult {
             case .artifactExists(let artifactDir):
                 infoLog("Artifact exists locally at \(artifactDir)")
+                _ = try artifactsOrganizer.prepare(artifact: artifactDir)
                 try artifactsOrganizer.activate(extractedArtifact: artifactDir)
             case .preparedForArtifact(let artifactPackage):
                 infoLog("Downloading artifact to \(artifactPackage)")

Sources/XCRemoteCache/Commands/Prebuild/XCPrebuild.swift

@@ -158,13 +158,17 @@ public class XCPrebuild {
                 fileAccessor: fileManager
             )
             let artifactProcessor = UnzippedArtifactProcessor(fileRemapper: fileRemapper, dirScanner: fileManager)
+            let metaUpdater = ArtifactMetaUpdater(
+                fileRemapper: fileRemapper,
+                metaWriter: JsonMetaWriter(fileWriter: fileManager, pretty: true)
+            )
             let organizer = ZipArtifactOrganizer(
                 targetTempDir: context.targetTempDir,
-                artifactProcessors: [artifactProcessor],
+                artifactProcessors: [artifactProcessor, metaUpdater],
                 fileManager: fileManager
             )
             let metaReader = JsonMetaReader(fileAccessor: fileManager)
-            var consumerPlugins: [ArtifactConsumerPrebuildPlugin] = []
+            var consumerPlugins: [ArtifactConsumerPrebuildPlugin] = [metaUpdater]
 
             if config.thinningEnabled {
                 if context.moduleName == config.thinningTargetModuleName, let thinnedTarget = context.thinnedTargets {

Sources/XCRemoteCache/Dependencies/MarkerReader.swift

@@ -22,12 +22,12 @@ import Foundation
 /// Reads a list of files from a marker file
 class FileMarkerReader: ListReader {
     private let file: URL
-    private let fileManager: FileManager
+    private let fileReader: FileReader
     private var cachedFiles: [URL]?
 
-    init(_ file: URL, fileManager: FileManager) {
+    init(_ file: URL, fileManager: FileReader) {
         self.file = file
-        self.fileManager = fileManager
+        self.fileReader = fileManager
     }
 
     func listFilesURLs() throws -> [URL] {
@@ -45,6 +45,6 @@ class FileMarkerReader: ListReader {
     }
 
     func canRead() -> Bool {
-        return fileManager.fileExists(atPath: file.path)
+        return fileReader.fileExists(atPath: file.path)
     }
 }

Sources/XCRemoteCache/Shell/Shell.swift

@@ -84,13 +84,13 @@ private func shellInternal(_ cmd: String, args: [String] = [], stdout: PipeLike?
         task.currentDirectoryPath = dir
     }
     task.launch()
+    let errorData = (stderr != nil) ? nil : errorHandle.fileHandleForReading.readDataToEndOfFile()
     task.waitUntilExit()
     if task.terminationStatus != 0 {
-        if stderr != nil {
+        guard let errorData = errorData else {
             // Error stream was captured so cannot inspect its content
             throw ShellError.statusError("Failed command", task.terminationStatus)
         }
-        let errorData = errorHandle.fileHandleForReading.readDataToEndOfFile()
         let errorString = String(data: errorData, encoding: .utf8)?.trim() ?? "No error returned from the process."
         throw ShellError.statusError(
             "status \(task.terminationStatus): \(errorString)", task.terminationStatus

Sources/XCRemoteCache/Stats/CacheHitLogger.swift

@@ -37,7 +37,7 @@ class ActionSpecificCacheHitLogger: CacheHitLogger {
         case .index:
             hitCounter = .indexingTargetHitCount
             missCounter = .indexingTargetMissCount
-        case .build:
+        case .build, .archive:
             hitCounter = .targetCacheHit
             missCounter = .targetCacheMiss
         case .unknown:

Sources/XCRemoteCache/Xcode/BuildActionType.swift

@@ -25,6 +25,8 @@ enum BuildActionType: String, Codable {
     case build
     /// An extra build, exclusive for indexing (Introduced in Xcode 13)
     case index = "indexbuild"
+    /// Archive build
+    case archive = "install"
     /// Unknown type, probably incompatible Xcode version used
     case unknown
 }

Sources/xclibtool/XCLibtoolMain.swift

@@ -35,7 +35,7 @@ public class XCLibtoolMain {
         let args = ProcessInfo().arguments
 
         do {
-            let mode = try XCLibtoolHelper.buildMode(args: args)
+            let mode = try XCLibtoolHelper.buildMode(args: Array(args.dropFirst()))
             try XCLibtool(mode).run()
         } catch {
             exit(1, "Failed with: \(error). Args: \(args)")

Sources/xclibtoolSupport/XCLibtoolHelper.swift

@@ -47,7 +47,9 @@ public class XCLibtoolHelper {
             case "-dependency_info":
                 dependencyInfo = args[i + 1]
                 i += 1
-            case let input where ["", "a"].contains(URL(string: args[i])?.pathExtension):
+            case let input where input.starts(with: "/") &&
+                ["", "a"].contains(URL(fileURLWithPath: input).pathExtension):
+                // Assume always absolute paths to the library
                 // Support for static frameworks (no extension) and static libraries (.a)
                 inputLibraries.append(input)
             default:

Tests/XCRemoteCacheTests/Artifacts/ArtifactMetaUpdaterTests.swift

@@ -0,0 +1,85 @@
+// 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.
+
+@testable import XCRemoteCache
+import XCTest
+
+class ArtifactMetaUpdaterTests: XCTestCase {
+    private let accessorFake = FileAccessorFake(mode: .normal)
+    private var metaWriter: MetaWriter!
+    private var fileRemapper: FileDependenciesRemapper!
+    private var updater: ArtifactMetaUpdater!
+    private let sampleMeta = MainArtifactMeta(
+        dependencies: [],
+        fileKey: "abc",
+        rawFingerprint: "",
+        generationCommit: "",
+        targetName: "",
+        configuration: "",
+        platform: "",
+        xcode: "",
+        inputs: ["$(BASE)/myFile.swift"],
+        pluginsKeys: [:]
+    )
+
+    override func setUp() async throws {
+        metaWriter = JsonMetaWriter(
+            fileWriter: accessorFake,
+            pretty: true
+        )
+        fileRemapper = TextFileDependenciesRemapper(
+            remapper: StringDependenciesRemapper(
+                mappings: [
+                    .init(generic: "$(BASE)", local: "/base")
+                ]
+            ),
+            fileAccessor: accessorFake
+        )
+        updater = ArtifactMetaUpdater(
+            fileRemapper: fileRemapper,
+            metaWriter: metaWriter
+        )
+    }
+
+    func testStoresInTheRawArtifact() throws {
+        try updater.process(rawArtifact: "/artifact")
+        try updater.run(meta: sampleMeta)
+
+        XCTAssertTrue(accessorFake.fileExists(atPath: "/artifact/abc.json"))
+    }
+
+    func testRewirtesMetaPaths() throws {
+        try updater.process(rawArtifact: "/artifact")
+        try updater.run(meta: sampleMeta)
+
+        let diskMetaData = try XCTUnwrap(accessorFake.contents(atPath: "/artifact/abc.json"))
+        let diskMeta = try JSONDecoder().decode(MainArtifactMeta.self, from: diskMetaData)
+        XCTAssertEqual(diskMeta.inputs, ["/base/myFile.swift"])
+    }
+
+    func testFailsIfProcessorTriggerIsNotCalledBeforeRunningAPlugin() throws {
+        XCTAssertThrowsError(try updater.run(meta: sampleMeta)) { error in
+            switch error {
+            case ArtifactMetaUpdaterError.artifactLocationIsUnknown: break
+            default:
+                XCTFail("Not expected error")
+            }
+        }
+    }
+}

Tests/XCRemoteCacheTests/Artifacts/ZipArtifactOrganizerTests.swift

@@ -156,4 +156,39 @@ class ZipArtifactOrganizerTests: XCTestCase {
 
         XCTAssertEqual(fileKey, expectedFileKey)
     }
+
+    func testPrepareRunsProcessorsForAlreadyExistingArtifacts() throws {
+        let zipURL = try prepareZipFile(content: "Magic", fileName: "content.txt")
+        let artifactURL = zipURL.deletingPathExtension()
+        let processor = DestroyerArtifactProcessor(fileManager)
+        let organizer: ZipArtifactOrganizer = ZipArtifactOrganizer(
+            targetTempDir: workingDirectory,
+            artifactProcessors: [processor],
+            fileManager: fileManager
+        )
+        try fileManager.createDirectory(
+            at: artifactURL,
+            withIntermediateDirectories: true
+        )
+
+        let preparedArtifact = try organizer.prepare(artifact: zipURL)
+
+        XCTAssertFalse(fileManager.fileExists(atPath: preparedArtifact.path))
+
+    }
+
+    func testPrepareRunsProcessorsForNewlyUnzippedArtifacts() throws {
+        let zipURL = try prepareZipFile(content: "Magic", fileName: "content.txt")
+        let processor = DestroyerArtifactProcessor(fileManager)
+        let organizer: ZipArtifactOrganizer = ZipArtifactOrganizer(
+            targetTempDir: workingDirectory,
+            artifactProcessors: [processor],
+            fileManager: fileManager
+        )
+
+        let preparedArtifact = try organizer.prepare(artifact: zipURL)
+
+        XCTAssertFalse(fileManager.fileExists(atPath: preparedArtifact.path))
+    }
+
 }

Tests/XCRemoteCacheTests/TestDoubles/DestroyerArtifactProcessor.swift

@@ -0,0 +1,37 @@
+// Copyright (c) 2022 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
+@testable import XCRemoteCache
+
+/// A Processor fake that deletes the artifact
+class DestroyerArtifactProcessor: ArtifactProcessor {
+    private let dirAccesor: DirAccessor
+
+    init(_ dirAccesor: DirAccessor) {
+        self.dirAccesor = dirAccesor
+    }
+    func process(rawArtifact url: URL) throws {
+        try dirAccesor.removeItem(atPath: url.path)
+    }
+    func process(localArtifact url: URL) throws {
+        try dirAccesor.removeItem(atPath: url.path)
+    }
+}

Tests/xclibtoolSupportTests/XCLibtoolHelperTests.swift

@@ -23,23 +23,23 @@ import XCTest
 class XCLibtoolHelperTests: XCTestCase {
     func testStaticFrameworkUniversalBinary() throws {
         let mode = try XCLibtoolHelper.buildMode(
-            args: ["-o", "/universal/static", "/arch1/static", "arch2/static"]
+            args: ["-o", "/universal/static", "/arch1/static", "/arch2/static"]
         )
 
         XCTAssertEqual(mode, .createUniversalBinary(
             output: "/universal/static",
-            inputs: ["/arch1/static", "arch2/static"]
+            inputs: ["/arch1/static", "/arch2/static"]
         ))
     }
 
     func testStaticLibraryUniversalBinary() throws {
         let mode = try XCLibtoolHelper.buildMode(
-            args: ["-o", "/universal/static.a", "/arch1/static.a", "arch2/static.a"]
+            args: ["-o", "/universal/static.a", "/arch1/static.a", "/arch2/static.a"]
         )
 
         XCTAssertEqual(mode, .createUniversalBinary(
             output: "/universal/static.a",
-            inputs: ["/arch1/static.a", "arch2/static.a"]
+            inputs: ["/arch1/static.a", "/arch2/static.a"]
         ))
     }
 
@@ -63,4 +63,26 @@ class XCLibtoolHelperTests: XCTestCase {
             }
         }
     }
+
+    func testExplicitStaticFrameworkUniversalBinary() throws {
+        let mode = try XCLibtoolHelper.buildMode(
+            args: ["-static", "-o", "/universal/static", "/arch1/static", "/arch2/static"]
+        )
+
+        XCTAssertEqual(mode, .createUniversalBinary(
+            output: "/universal/static",
+            inputs: ["/arch1/static", "/arch2/static"]
+        ))
+    }
+
+    func testRecognizesPathsWithSpaces() throws {
+        let mode = try XCLibtoolHelper.buildMode(
+            args: ["-static", "-o", "/universal/static", "/arch/with space/static"]
+        )
+
+        XCTAssertEqual(mode, .createUniversalBinary(
+            output: "/universal/static",
+            inputs: ["/arch/with space/static"]
+        ))
+    }
 }