Skip to content

Commit 7ba9c27

Browse files
authored
[NFC] Split Workspace.swift into smaller files (#7006)
### Motivation: `Workspace.swift` has grown to an unmanageable size: ~4k lines, close to 4200 before #7000 was merged. The `Workspace` type is the main API for SourceKit-LSP interactions with SwiftPM. As we'd like to add more cross-compilation functionality here (e.g. allowing SourceKit-LSP to select a different Swift SDK), this API should be more navigable and easier to work with. Since the amount of `public` functions on `Workspace` is limited, we should keep them grouped in the main `Workspace.swift` file, but the actual implementations can be moved to separate files for easier subsequent refactoring and further changes. ### Modifications: Split `Workspace.swift` into more files, with each file corresponding to an implementation aspect of the `Workspace` API. Also applied our formatting rules to the new files. ### Result: `Workspace.swift` is now only 1384 lines long. When looking for a specific `Workspace` functionality implementation, you can look it up in separate files that correspond to a larger area grouping that functionality.
1 parent 855b129 commit 7ba9c27

15 files changed

+3510
-2789
lines changed

Sources/Workspace/CMakeLists.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ add_library(Workspace
1111
DefaultPluginScriptRunner.swift
1212
Diagnostics.swift
1313
InitPackage.swift
14+
LoadableResult.swift
1415
ManagedArtifact.swift
1516
ManagedDependency.swift
1617
PackageContainer/FileSystemPackageContainer.swift
@@ -22,7 +23,15 @@ add_library(Workspace
2223
Workspace.swift
2324
Workspace+BinaryArtifacts.swift
2425
Workspace+Configuration.swift
26+
Workspace+Delegation.swift
27+
Workspace+Dependencies.swift
28+
Workspace+Editing.swift
2529
Workspace+Manifests.swift
30+
Workspace+PackageContainer.swift
31+
Workspace+Pinning.swift
32+
Workspace+Registry.swift
33+
Workspace+Signing.swift
34+
Workspace+SourceControl.swift
2635
Workspace+State.swift)
2736
target_link_libraries(Workspace PUBLIC
2837
TSCBasic
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift open source project
4+
//
5+
// Copyright (c) 2014-2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See http://swift.org/LICENSE.txt for license information
9+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
/// A result which can be loaded.
14+
///
15+
/// It is useful for objects that hold a state on disk and need to be
16+
/// loaded frequently.
17+
public final class LoadableResult<Value> {
18+
/// The constructor closure for the value.
19+
private let construct: () throws -> Value
20+
21+
/// Create a loadable result.
22+
public init(_ construct: @escaping () throws -> Value) {
23+
self.construct = construct
24+
}
25+
26+
/// Load and return the result.
27+
public func loadResult() -> Result<Value, Error> {
28+
Result(catching: {
29+
try self.construct()
30+
})
31+
}
32+
33+
/// Load and return the value.
34+
public func load() throws -> Value {
35+
try self.loadResult().get()
36+
}
37+
}

Sources/Workspace/Workspace+BinaryArtifacts.swift

Lines changed: 153 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212

1313
import Basics
1414
import Foundation
15+
import PackageLoading
1516
import PackageModel
1617
import SPMBuildCore
17-
import PackageLoading
1818

1919
import struct TSCBasic.ByteString
2020
import protocol TSCBasic.HashAlgorithm
@@ -696,7 +696,8 @@ extension Workspace.BinaryArtifactsManager {
696696
_ = try decoder.decode(XCFrameworkMetadata.self, from: fileSystem.readFileContents(infoPlist))
697697
return .xcframework
698698
} catch {
699-
observabilityScope.emit(debug: "info.plist found in '\(path)' but failed to parse: \(error.interpolationDescription)")
699+
observabilityScope
700+
.emit(debug: "info.plist found in '\(path)' but failed to parse: \(error.interpolationDescription)")
700701
}
701702
}
702703

@@ -716,6 +717,156 @@ extension Workspace.BinaryArtifactsManager {
716717
}
717718
}
718719

720+
extension Workspace {
721+
func updateBinaryArtifacts(
722+
manifests: DependencyManifests,
723+
addedOrUpdatedPackages: [PackageReference],
724+
observabilityScope: ObservabilityScope
725+
) throws {
726+
let manifestArtifacts = try self.binaryArtifactsManager.parseArtifacts(
727+
from: manifests,
728+
observabilityScope: observabilityScope
729+
)
730+
731+
var artifactsToRemove: [ManagedArtifact] = []
732+
var artifactsToAdd: [ManagedArtifact] = []
733+
var artifactsToDownload: [BinaryArtifactsManager.RemoteArtifact] = []
734+
var artifactsToExtract: [ManagedArtifact] = []
735+
736+
for artifact in state.artifacts {
737+
if !manifestArtifacts.local
738+
.contains(where: { $0.packageRef == artifact.packageRef && $0.targetName == artifact.targetName }) &&
739+
!manifestArtifacts.remote
740+
.contains(where: { $0.packageRef == artifact.packageRef && $0.targetName == artifact.targetName })
741+
{
742+
artifactsToRemove.append(artifact)
743+
}
744+
}
745+
746+
for artifact in manifestArtifacts.local {
747+
let existingArtifact = self.state.artifacts[
748+
packageIdentity: artifact.packageRef.identity,
749+
targetName: artifact.targetName
750+
]
751+
752+
if artifact.path.extension?.lowercased() == "zip" {
753+
// If we already have an artifact that was extracted from an archive with the same checksum,
754+
// we don't need to extract the artifact again.
755+
if case .local(let existingChecksum) = existingArtifact?.source,
756+
try existingChecksum == (self.binaryArtifactsManager.checksum(forBinaryArtifactAt: artifact.path))
757+
{
758+
continue
759+
}
760+
761+
artifactsToExtract.append(artifact)
762+
} else {
763+
guard let _ = try BinaryArtifactsManager.deriveBinaryArtifact(
764+
fileSystem: self.fileSystem,
765+
path: artifact.path,
766+
observabilityScope: observabilityScope
767+
) else {
768+
observabilityScope.emit(.localArtifactNotFound(
769+
artifactPath: artifact.path,
770+
targetName: artifact.targetName
771+
))
772+
continue
773+
}
774+
artifactsToAdd.append(artifact)
775+
}
776+
777+
if let existingArtifact, isAtArtifactsDirectory(existingArtifact) {
778+
// Remove the old extracted artifact, be it local archived or remote one.
779+
artifactsToRemove.append(existingArtifact)
780+
}
781+
}
782+
783+
for artifact in manifestArtifacts.remote {
784+
let existingArtifact = self.state.artifacts[
785+
packageIdentity: artifact.packageRef.identity,
786+
targetName: artifact.targetName
787+
]
788+
789+
if let existingArtifact {
790+
if case .remote(let existingURL, let existingChecksum) = existingArtifact.source {
791+
// If we already have an artifact with the same checksum, we don't need to download it again.
792+
if artifact.checksum == existingChecksum {
793+
continue
794+
}
795+
796+
let urlChanged = artifact.url != URL(string: existingURL)
797+
// If the checksum is different but the package wasn't updated, this is a security risk.
798+
if !urlChanged && !addedOrUpdatedPackages.contains(artifact.packageRef) {
799+
observabilityScope.emit(.artifactChecksumChanged(targetName: artifact.targetName))
800+
continue
801+
}
802+
}
803+
804+
if isAtArtifactsDirectory(existingArtifact) {
805+
// Remove the old extracted artifact, be it local archived or remote one.
806+
artifactsToRemove.append(existingArtifact)
807+
}
808+
}
809+
810+
artifactsToDownload.append(artifact)
811+
}
812+
813+
// Remove the artifacts and directories which are not needed anymore.
814+
observabilityScope.trap {
815+
for artifact in artifactsToRemove {
816+
state.artifacts.remove(packageIdentity: artifact.packageRef.identity, targetName: artifact.targetName)
817+
818+
if isAtArtifactsDirectory(artifact) {
819+
try fileSystem.removeFileTree(artifact.path)
820+
}
821+
}
822+
823+
for directory in try fileSystem.getDirectoryContents(self.location.artifactsDirectory) {
824+
let directoryPath = self.location.artifactsDirectory.appending(component: directory)
825+
if try fileSystem.isDirectory(directoryPath) && fileSystem.getDirectoryContents(directoryPath).isEmpty {
826+
try fileSystem.removeFileTree(directoryPath)
827+
}
828+
}
829+
}
830+
831+
guard !observabilityScope.errorsReported else {
832+
throw Diagnostics.fatalError
833+
}
834+
835+
// Download the artifacts
836+
let downloadedArtifacts = try self.binaryArtifactsManager.download(
837+
artifactsToDownload,
838+
artifactsDirectory: self.location.artifactsDirectory,
839+
observabilityScope: observabilityScope
840+
)
841+
artifactsToAdd.append(contentsOf: downloadedArtifacts)
842+
843+
// Extract the local archived artifacts
844+
let extractedLocalArtifacts = try self.binaryArtifactsManager.extract(
845+
artifactsToExtract,
846+
artifactsDirectory: self.location.artifactsDirectory,
847+
observabilityScope: observabilityScope
848+
)
849+
artifactsToAdd.append(contentsOf: extractedLocalArtifacts)
850+
851+
// Add the new artifacts
852+
for artifact in artifactsToAdd {
853+
self.state.artifacts.add(artifact)
854+
}
855+
856+
guard !observabilityScope.errorsReported else {
857+
throw Diagnostics.fatalError
858+
}
859+
860+
observabilityScope.trap {
861+
try self.state.save()
862+
}
863+
864+
func isAtArtifactsDirectory(_ artifact: ManagedArtifact) -> Bool {
865+
artifact.path.isDescendant(of: self.location.artifactsDirectory)
866+
}
867+
}
868+
}
869+
719870
extension FileSystem {
720871
// helper to decide if an archive directory would benefit from stripping first level
721872
fileprivate func shouldStripFirstLevel(

Sources/Workspace/Workspace+Configuration.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -179,14 +179,14 @@ extension Workspace {
179179
/// - Parameters:
180180
/// - rootPath: Path to the root of the package, from which other locations can be derived.
181181
public init(forRootPackage rootPath: AbsolutePath, fileSystem: FileSystem) throws {
182-
self.init(
182+
try self.init(
183183
scratchDirectory: DefaultLocations.scratchDirectory(forRootPackage: rootPath),
184184
editsDirectory: DefaultLocations.editsDirectory(forRootPackage: rootPath),
185185
resolvedVersionsFile: DefaultLocations.resolvedVersionsFile(forRootPackage: rootPath),
186186
localConfigurationDirectory: DefaultLocations.configurationDirectory(forRootPackage: rootPath),
187-
sharedConfigurationDirectory: try fileSystem.swiftPMConfigurationDirectory,
188-
sharedSecurityDirectory: try fileSystem.swiftPMSecurityDirectory,
189-
sharedCacheDirectory: try fileSystem.swiftPMCacheDirectory
187+
sharedConfigurationDirectory: fileSystem.swiftPMConfigurationDirectory,
188+
sharedSecurityDirectory: fileSystem.swiftPMSecurityDirectory,
189+
sharedCacheDirectory: fileSystem.swiftPMCacheDirectory
190190
)
191191
}
192192
}
@@ -208,7 +208,7 @@ extension Workspace {
208208
}
209209

210210
public static func resolvedVersionsFile(forRootPackage rootPath: AbsolutePath) -> AbsolutePath {
211-
rootPath.appending(Self.resolvedFileName)
211+
rootPath.appending(self.resolvedFileName)
212212
}
213213

214214
public static func configurationDirectory(forRootPackage rootPath: AbsolutePath) -> AbsolutePath {
@@ -300,7 +300,7 @@ extension Workspace.Configuration {
300300
guard fileSystem.exists(path) else {
301301
throw StringError("Did not find netrc file at \(path).")
302302
}
303-
providers.append(try NetrcAuthorizationProvider(path: path, fileSystem: fileSystem))
303+
try providers.append(NetrcAuthorizationProvider(path: path, fileSystem: fileSystem))
304304
case .user:
305305
// user .netrc file (most typical)
306306
let userHomePath = try fileSystem.homeDirectory.appending(".netrc")
@@ -360,7 +360,7 @@ extension Workspace.Configuration {
360360
guard fileSystem.exists(path) else {
361361
throw StringError("did not find netrc file at \(path)")
362362
}
363-
providers.append(try NetrcAuthorizationProvider(path: path, fileSystem: fileSystem))
363+
try providers.append(NetrcAuthorizationProvider(path: path, fileSystem: fileSystem))
364364
case .user:
365365
let userHomePath = try fileSystem.homeDirectory.appending(".netrc")
366366
// Add user .netrc file unless we don't have access
@@ -523,7 +523,7 @@ extension Workspace.Configuration {
523523
return try DependencyMirrors()
524524
}
525525
return try self.fileSystem.withLock(on: self.path.parentDirectory, type: .shared) {
526-
return try DependencyMirrors(try Self.load(self.path, fileSystem: self.fileSystem))
526+
try DependencyMirrors(Self.load(self.path, fileSystem: self.fileSystem))
527527
}
528528
}
529529

@@ -534,7 +534,7 @@ extension Workspace.Configuration {
534534
try self.fileSystem.createDirectory(self.path.parentDirectory, recursive: true)
535535
}
536536
return try self.fileSystem.withLock(on: self.path.parentDirectory, type: .exclusive) {
537-
let mirrors = try DependencyMirrors(try Self.load(self.path, fileSystem: self.fileSystem))
537+
let mirrors = try DependencyMirrors(Self.load(self.path, fileSystem: self.fileSystem))
538538
var updatedMirrors = try DependencyMirrors(mirrors.mapping)
539539
try handler(&updatedMirrors)
540540
if updatedMirrors != mirrors {

0 commit comments

Comments
 (0)