Merge pull request #225 from polac24/fix-spaces-libtool

Support spaces in libtools paths

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/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/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"]
+        ))
+    }
 }

cocoapods-plugin/lib/cocoapods-xcremotecache/command/hooks.rb

@@ -409,6 +409,11 @@ module CocoapodsXCRemoteCacheModifier
         File.write(LLDB_INIT_PATH, lldbinit_lines.join("\n"), mode: "w")
       end
 
+      # Contrary to AbstractTarget.source_build_phase, it only finds a build phase, without creating one if it doesn't exist
+      def self.find_source_build_phase(target)
+        target.build_phases.find { |bp| bp.class == Xcodeproj::Project::Object::PBXSourcesBuildPhase }
+      end
+
       Pod::HooksManager.register('cocoapods-xcremotecache', :pre_install) do |installer_context|
         # The main responsibility of that hook is forcing Pods regeneration when XCRemoteCache is enabled for the first time
         # In the post_install hook, this plugin adds extra build settings and steps to all Pods targets, but only when XCRemoteCache
@@ -520,6 +525,8 @@ module CocoapodsXCRemoteCacheModifier
             # Attach XCRemoteCache to Pods targets
             # Enable only for native targets which can have compilation steps
             installer_context.pods_project.native_targets.each do |target|
+                # Ensure the PBXSourcesBuildPhase exists as the flow would unnecessary create an empty source build phase otherwise
+                next if find_source_build_phase(target).nil?
                 next if target.source_build_phase.files_references.empty?
                 next if target.name.start_with?("Pods-")
                 next if target.name.end_with?("Tests")

cocoapods-plugin/lib/cocoapods-xcremotecache/gem_version.rb

@@ -13,5 +13,5 @@
 # limitations under the License.
 
 module CocoapodsXcremotecache
-  VERSION = "0.0.17"
+  VERSION = "0.0.18"
 end