Skip to content

api cleanup in preperation for identity improvements #3214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added Bar-1.2.3.zip
Binary file not shown.
2 changes: 1 addition & 1 deletion Sources/Commands/SwiftPackageCollectionsTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
mutating func run() throws {
try with { collections in
let identity = PackageIdentity(url: packageUrl)
let reference = PackageReference(identity: identity, path: packageUrl)
let reference = PackageReference.remote(identity: identity, location: packageUrl)

do { // assume URL is for a package
let result = try tsc_await { collections.getPackageMetadata(reference, callback: $0) }
Expand Down
2 changes: 1 addition & 1 deletion Sources/Commands/SwiftTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate {
case .forced:
self.stdoutStream <<< "it was forced"
case .newPackages(let packages):
let dependencies = packages.lazy.map({ "'\($0.path)'" }).joined(separator: ", ")
let dependencies = packages.lazy.map({ "'\($0.location)'" }).joined(separator: ", ")
self.stdoutStream <<< "the following dependencies were added: \(dependencies)"
case .packageRequirementChange(let package, let state, let requirement):
self.stdoutStream <<< "dependency '\(package.name)' was "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
guard reference.kind == .remote else {
return callback(.failure(Errors.invalidReferenceType(reference)))
}
guard let baseURL = self.apiURL(reference.path) else {
return callback(.failure(Errors.invalidGitURL(reference.path)))
guard let baseURL = self.apiURL(reference.location) else {
return callback(.failure(Errors.invalidGitURL(reference.location)))
}

let metadataURL = baseURL
Expand Down
4 changes: 2 additions & 2 deletions Sources/PackageCollections/Utility.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ extension PackageReference {
init(repository: RepositorySpecifier, kind: PackageReference.Kind = .remote) {
self.init(
identity: PackageIdentity(url: repository.url),
path: repository.url,
kind: kind
kind: kind,
location: repository.url
)
}
}
4 changes: 2 additions & 2 deletions Sources/PackageGraph/DependencyResolutionNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public enum DependencyResolutionNode {
// Don’t create a version lock for anything but a product.
guard specificProduct != nil else { return nil }
return PackageContainerConstraint(
container: package,
package: package,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call this packageReference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neonichu we could, but in many other places in the code vase it is called "package". I actually have a commit with this change but its large if we want to be consistent. wdyt?

versionRequirement: .exact(version),
products: .specific([])
)
Expand All @@ -108,7 +108,7 @@ public enum DependencyResolutionNode {
// Don’t create a revision lock for anything but a product.
guard specificProduct != nil else { return nil }
return PackageContainerConstraint(
container: package,
package: package,
requirement: .revision(revision),
products: .specific([])
)
Expand Down
30 changes: 15 additions & 15 deletions Sources/PackageGraph/LocalPackageContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,35 +24,35 @@ import TSCUtility
/// should be used as-is. Infact, they might not even have a git repository.
/// Examples: Root packages, local dependencies, edited packages.
public final class LocalPackageContainer: PackageContainer {
public let identifier: PackageReference
public let package: PackageReference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call this packageReference?

private let mirrors: DependencyMirrors
private let manifestLoader: ManifestLoaderProtocol
private let toolsVersionLoader: ToolsVersionLoaderProtocol
private let currentToolsVersion: ToolsVersion

/// The file system that shoud be used to load this package.
private let fs: FileSystem
private let fileSystem: FileSystem

/// cached version of the manifest
private let manifest = ThreadSafeBox<Manifest>()

private func loadManifest() throws -> Manifest {
try manifest.memoize() {
// Load the tools version.
let toolsVersion = try toolsVersionLoader.load(at: AbsolutePath(identifier.path), fileSystem: fs)
let toolsVersion = try toolsVersionLoader.load(at: AbsolutePath(package.location), fileSystem: fileSystem)

// Validate the tools version.
try toolsVersion.validateToolsVersion(self.currentToolsVersion, packagePath: identifier.path)
try toolsVersion.validateToolsVersion(self.currentToolsVersion, packagePath: package.location)

// Load the manifest.
// FIXME: this should not block
return try temp_await {
manifestLoader.load(package: AbsolutePath(identifier.path),
baseURL: identifier.path,
manifestLoader.load(package: AbsolutePath(package.location),
baseURL: package.location,
version: nil,
toolsVersion: toolsVersion,
packageKind: identifier.kind,
fileSystem: fs,
packageKind: package.kind,
fileSystem: fileSystem,
on: .global(),
completion: $0)
}
Expand All @@ -66,24 +66,24 @@ public final class LocalPackageContainer: PackageContainer {
public func getUpdatedIdentifier(at boundVersion: BoundVersion) throws -> PackageReference {
assert(boundVersion == .unversioned, "Unexpected bound version \(boundVersion)")
let manifest = try loadManifest()
return identifier.with(newName: manifest.name)
return package.with(newName: manifest.name)
}

public init(
_ identifier: PackageReference,
package: PackageReference,
mirrors: DependencyMirrors,
manifestLoader: ManifestLoaderProtocol,
toolsVersionLoader: ToolsVersionLoaderProtocol,
currentToolsVersion: ToolsVersion,
fs: FileSystem = localFileSystem
fileSystem: FileSystem = localFileSystem
) {
assert(URL.scheme(identifier.path) == nil, "unexpected scheme \(URL.scheme(identifier.path)!) in \(identifier.path)")
self.identifier = identifier
assert(URL.scheme(package.location) == nil, "unexpected scheme \(URL.scheme(package.location)!) in \(package.location)")
self.package = package
self.mirrors = mirrors
self.manifestLoader = manifestLoader
self.toolsVersionLoader = toolsVersionLoader
self.currentToolsVersion = currentToolsVersion
self.fs = fs
self.fileSystem = fileSystem
}

public func isToolsVersionCompatible(at version: Version) -> Bool {
Expand Down Expand Up @@ -113,6 +113,6 @@ public final class LocalPackageContainer: PackageContainer {

extension LocalPackageContainer: CustomStringConvertible {
public var description: String {
return "LocalPackageContainer(\(identifier.path))"
return "LocalPackageContainer(\(package.location))"
}
}
16 changes: 8 additions & 8 deletions Sources/PackageGraph/PackageContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import struct TSCUtility.Version
public protocol PackageContainer {

/// The identifier for the package.
var identifier: PackageReference { get }
var package: PackageReference { get }

/// Returns true if the tools version is compatible at the given version.
func isToolsVersionCompatible(at version: Version) -> Bool
Expand Down Expand Up @@ -116,7 +116,7 @@ extension PackageContainer {
public struct PackageContainerConstraint: Equatable, Hashable {

/// The identifier for the container the constraint is on.
public let identifier: PackageReference
public let package: PackageReference

/// The constraint requirement.
public let requirement: PackageRequirement
Expand All @@ -126,22 +126,22 @@ public struct PackageContainerConstraint: Equatable, Hashable {

/// Create a constraint requiring the given `container` satisfying the
/// `requirement`.
public init(container identifier: PackageReference, requirement: PackageRequirement, products: ProductFilter) {
self.identifier = identifier
public init(package: PackageReference, requirement: PackageRequirement, products: ProductFilter) {
self.package = package
self.requirement = requirement
self.products = products
}

/// Create a constraint requiring the given `container` satisfying the
/// `versionRequirement`.
public init(container identifier: PackageReference, versionRequirement: VersionSetSpecifier, products: ProductFilter) {
self.init(container: identifier, requirement: .versionSet(versionRequirement), products: products)
public init(package: PackageReference, versionRequirement: VersionSetSpecifier, products: ProductFilter) {
self.init(package: package, requirement: .versionSet(versionRequirement), products: products)
}
}

extension PackageContainerConstraint: CustomStringConvertible {
public var description: String {
return "Constraint(\(identifier), \(requirement), \(products)"
return "Constraint(\(self.package), \(requirement), \(products)"
}
}

Expand All @@ -151,7 +151,7 @@ extension PackageContainerConstraint: CustomStringConvertible {
public protocol PackageContainerProvider {
/// Get the container for a particular identifier asynchronously.
func getContainer(
for identifier: PackageReference,
for package: PackageReference,
skipUpdate: Bool,
on queue: DispatchQueue,
completion: @escaping (Result<PackageContainer, Swift.Error>) -> Void
Expand Down
2 changes: 1 addition & 1 deletion Sources/PackageGraph/PackageGraph+Loading.swift
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ private func createResolvedPackages(
guard let package = manifestToPackage[node.manifest] else {
return nil
}
let isAllowedToVendUnsafeProducts = unsafeAllowedPackages.contains{ $0.path == package.manifest.url }
let isAllowedToVendUnsafeProducts = unsafeAllowedPackages.contains{ $0.location == package.manifest.url }
return ResolvedPackageBuilder(
package,
productFilter: node.productFilter,
Expand Down
6 changes: 3 additions & 3 deletions Sources/PackageGraph/PackageGraphRoot.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public struct PackageGraphRoot {
public init(input: PackageGraphRootInput, manifests: [Manifest], explicitProduct: String? = nil) {
self.packageRefs = zip(input.packages, manifests).map { (path, manifest) in
let identity = PackageIdentity(url: manifest.url)
return PackageReference(identity: identity, path: path.pathString, kind: .root)
return .root(identity: identity, path: path)
}
self.manifests = manifests

Expand All @@ -73,11 +73,11 @@ public struct PackageGraphRoot {
/// Returns the constraints imposed by root manifests + dependencies.
public func constraints(mirrors: DependencyMirrors) -> [PackageContainerConstraint] {
let constraints = packageRefs.map({
PackageContainerConstraint(container: $0, requirement: .unversioned, products: .everything)
PackageContainerConstraint(package: $0, requirement: .unversioned, products: .everything)
})
return constraints + dependencies.map({
PackageContainerConstraint(
container: $0.createPackageRef(mirrors: mirrors),
package: $0.createPackageRef(mirrors: mirrors),
requirement: $0.requirement.toConstraintRequirement(),
products: $0.productFilter
)
Expand Down
14 changes: 7 additions & 7 deletions Sources/PackageGraph/PackageModel+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ extension PackageDependencyDescription {

return PackageReference(
identity: identity,
path: effectiveURL,
kind: requirement == .localPackage ? .local : .remote
kind: requirement == .localPackage ? .local : .remote,
location: effectiveURL
)
}
}
Expand All @@ -38,7 +38,7 @@ extension Manifest {
public func dependencyConstraints(productFilter: ProductFilter, mirrors: DependencyMirrors) -> [PackageContainerConstraint] {
return dependenciesRequired(for: productFilter).map({
return PackageContainerConstraint(
container: $0.createPackageRef(mirrors: mirrors),
package: $0.createPackageRef(mirrors: mirrors),
requirement: $0.requirement.toConstraintRequirement(),
products: $0.productFilter)
})
Expand All @@ -49,17 +49,17 @@ extension PackageContainerConstraint {
internal func nodes() -> [DependencyResolutionNode] {
switch products {
case .everything:
return [.root(package: identifier)]
return [.root(package: self.package)]
case .specific:
switch products {
case .everything:
assertionFailure("Attempted to enumerate a root package’s product filter; root packages have no filter.")
return []
case .specific(let set):
if set.isEmpty { // Pointing at the package without a particular product.
return [.empty(package: identifier)]
return [.empty(package: self.package)]
} else {
return set.sorted().map { .product($0, package: identifier) }
return set.sorted().map { .product($0, package: self.package) }
}
}
}
Expand All @@ -72,6 +72,6 @@ extension PackageReference {
/// This should only be accessed when the reference is not local.
public var repository: RepositorySpecifier {
precondition(kind == .remote)
return RepositorySpecifier(url: path)
return RepositorySpecifier(url: self.location)
}
}
4 changes: 2 additions & 2 deletions Sources/PackageGraph/PinsStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ extension PinsStore.Pin: JSONMappable, JSONSerializable {
let name: String? = json.get("package")
let url: String = try json.get("repositoryURL")
let identity = PackageIdentity(url: url)
let ref = PackageReference(identity: identity, path: url)
let ref = PackageReference.remote(identity: identity, location: url)
self.packageRef = name.flatMap(ref.with(newName:)) ?? ref
self.state = try json.get("state")
}
Expand All @@ -145,7 +145,7 @@ extension PinsStore.Pin: JSONMappable, JSONSerializable {
public func toJSON() -> JSON {
return .init([
"package": packageRef.name.toJSON(),
"repositoryURL": packageRef.path,
"repositoryURL": packageRef.location,
"state": state
])
}
Expand Down
Loading