From d5e95962b54a85ccc06cbd2dd11812b21e959067 Mon Sep 17 00:00:00 2001 From: Bartosz Polaczyk Date: Mon, 12 Jun 2023 17:59:05 -0700 Subject: [PATCH 1/4] Keep fresh meta in active artifact dir --- .../Artifacts/ArtifactMetaUpdater.swift | 73 +++++++++++++++++++ .../Artifacts/ArtifactOrganizer.swift | 19 +++-- .../Commands/Prebuild/XCPrebuild.swift | 8 +- .../Dependencies/MarkerReader.swift | 8 +- 4 files changed, 97 insertions(+), 11 deletions(-) create mode 100644 Sources/XCRemoteCache/Artifacts/ArtifactMetaUpdater.swift diff --git a/Sources/XCRemoteCache/Artifacts/ArtifactMetaUpdater.swift b/Sources/XCRemoteCache/Artifacts/ArtifactMetaUpdater.swift new file mode 100644 index 00000000..df36741e --- /dev/null +++ b/Sources/XCRemoteCache/Artifacts/ArtifactMetaUpdater.swift @@ -0,0 +1,73 @@ +// 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 ArtifactMetaProcessorError: Error { + /// The prebuild plugin execution was called but the local + /// path to the artifact directory is unknown + /// Might happen that the artifact processor didn't invoke + /// a request to process an artifact + case artifactLocationIsUnknown +} + +/// Processes downloaded artifact by placing an up-to-date and remapped meta file +/// in the artifact directory +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 artifact + // Note, the location might include a meta (generated by producer + // while compiling and building an artifact), which might be outdated + // (e.g. a new schema has been applied to the meta format, while artifacts + // format is backward-compatible) + artifactLocation = url + } + + func process(localArtifact url: URL) throws { + // No need to do anything in the postbuild + } +} + +extension ArtifactMetaUpdater: ArtifactConsumerPrebuildPlugin { + + /// Overrides the meta json file in the downloaded artifact with a meta including + /// remapped paths + func run(meta: MainArtifactMeta) throws { + guard let artifactLocation = artifactLocation else { + throw ArtifactMetaProcessorError.artifactLocationIsUnknown + } + _ = try metaWriter.write(meta, locationDir: artifactLocation) + // TODO: extract the meta json location logic to a shared class + let metaURL = artifactLocation + .appendingPathComponent(meta.fileKey) + .appendingPathExtension("json") + try fileRemapper.remap(fromGeneric: metaURL) + } +} diff --git a/Sources/XCRemoteCache/Artifacts/ArtifactOrganizer.swift b/Sources/XCRemoteCache/Artifacts/ArtifactOrganizer.swift index 2139b4c7..4a9ec73b 100644 --- a/Sources/XCRemoteCache/Artifacts/ArtifactOrganizer.swift +++ b/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) diff --git a/Sources/XCRemoteCache/Commands/Prebuild/XCPrebuild.swift b/Sources/XCRemoteCache/Commands/Prebuild/XCPrebuild.swift index b4c8dd8c..40f38d45 100644 --- a/Sources/XCRemoteCache/Commands/Prebuild/XCPrebuild.swift +++ b/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 { diff --git a/Sources/XCRemoteCache/Dependencies/MarkerReader.swift b/Sources/XCRemoteCache/Dependencies/MarkerReader.swift index 424cfa52..3ad8baf3 100644 --- a/Sources/XCRemoteCache/Dependencies/MarkerReader.swift +++ b/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) } } From 6a98cb41275b9706e40439c15dc801910d11d6b4 Mon Sep 17 00:00:00 2001 From: Bartosz Polaczyk Date: Mon, 12 Jun 2023 18:59:35 -0700 Subject: [PATCH 2/4] Add unit tests for updater --- .../Artifacts/ArtifactMetaUpdater.swift | 41 +++++---- .../Artifacts/ArtifactMetaUpdaterTests.swift | 85 +++++++++++++++++++ 2 files changed, 105 insertions(+), 21 deletions(-) create mode 100644 Tests/XCRemoteCacheTests/Artifacts/ArtifactMetaUpdaterTests.swift diff --git a/Sources/XCRemoteCache/Artifacts/ArtifactMetaUpdater.swift b/Sources/XCRemoteCache/Artifacts/ArtifactMetaUpdater.swift index df36741e..9cea56b7 100644 --- a/Sources/XCRemoteCache/Artifacts/ArtifactMetaUpdater.swift +++ b/Sources/XCRemoteCache/Artifacts/ArtifactMetaUpdater.swift @@ -19,34 +19,37 @@ import Foundation -enum ArtifactMetaProcessorError: Error { +enum ArtifactMetaUpdaterError: Error { /// The prebuild plugin execution was called but the local - /// path to the artifact directory is unknown - /// Might happen that the artifact processor didn't invoke - /// a request to process an artifact + /// 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 } -/// Processes downloaded artifact by placing an up-to-date and remapped meta file -/// in the artifact directory +/// 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) { + 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 artifact - // Note, the location might include a meta (generated by producer - // while compiling and building an artifact), which might be outdated - // (e.g. a new schema has been applied to the meta format, while artifacts - // format is backward-compatible) + // 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 } @@ -57,17 +60,13 @@ class ArtifactMetaUpdater: ArtifactProcessor { extension ArtifactMetaUpdater: ArtifactConsumerPrebuildPlugin { - /// Overrides the meta json file in the downloaded artifact with a meta including - /// remapped paths + /// 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 ArtifactMetaProcessorError.artifactLocationIsUnknown + throw ArtifactMetaUpdaterError.artifactLocationIsUnknown } - _ = try metaWriter.write(meta, locationDir: artifactLocation) - // TODO: extract the meta json location logic to a shared class - let metaURL = artifactLocation - .appendingPathComponent(meta.fileKey) - .appendingPathExtension("json") + let metaURL = try metaWriter.write(meta, locationDir: artifactLocation) try fileRemapper.remap(fromGeneric: metaURL) } } diff --git a/Tests/XCRemoteCacheTests/Artifacts/ArtifactMetaUpdaterTests.swift b/Tests/XCRemoteCacheTests/Artifacts/ArtifactMetaUpdaterTests.swift new file mode 100644 index 00000000..feb08213 --- /dev/null +++ b/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") + } + } + } +} From 42a56b4f5b9069dfce64cda11fe61959fe5cf7b7 Mon Sep 17 00:00:00 2001 From: Bartosz Polaczyk Date: Mon, 12 Jun 2023 19:23:36 -0700 Subject: [PATCH 3/4] Add tests for ZipArtifactOrganizer --- .../Artifacts/ZipArtifactOrganizerTests.swift | 35 +++++++++++++++++ .../DestroyerArtifactProcessor.swift | 38 +++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 Tests/XCRemoteCacheTests/TestDoubles/DestroyerArtifactProcessor.swift diff --git a/Tests/XCRemoteCacheTests/Artifacts/ZipArtifactOrganizerTests.swift b/Tests/XCRemoteCacheTests/Artifacts/ZipArtifactOrganizerTests.swift index 8be8478d..05fa6d2c 100644 --- a/Tests/XCRemoteCacheTests/Artifacts/ZipArtifactOrganizerTests.swift +++ b/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)) + } + } diff --git a/Tests/XCRemoteCacheTests/TestDoubles/DestroyerArtifactProcessor.swift b/Tests/XCRemoteCacheTests/TestDoubles/DestroyerArtifactProcessor.swift new file mode 100644 index 00000000..250ad1aa --- /dev/null +++ b/Tests/XCRemoteCacheTests/TestDoubles/DestroyerArtifactProcessor.swift @@ -0,0 +1,38 @@ +// 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) + } +} + From 587840ddbcc5e8735b366e42e206bcd11cc73913 Mon Sep 17 00:00:00 2001 From: Bartosz Polaczyk Date: Mon, 12 Jun 2023 19:44:08 -0700 Subject: [PATCH 4/4] Fix linter --- .../TestDoubles/DestroyerArtifactProcessor.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/XCRemoteCacheTests/TestDoubles/DestroyerArtifactProcessor.swift b/Tests/XCRemoteCacheTests/TestDoubles/DestroyerArtifactProcessor.swift index 250ad1aa..7b04c114 100644 --- a/Tests/XCRemoteCacheTests/TestDoubles/DestroyerArtifactProcessor.swift +++ b/Tests/XCRemoteCacheTests/TestDoubles/DestroyerArtifactProcessor.swift @@ -35,4 +35,3 @@ class DestroyerArtifactProcessor: ArtifactProcessor { try dirAccesor.removeItem(atPath: url.path) } } -