Skip to content

Remove Product Filters from All Persistent Files #2794

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
Jul 1, 2020
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
4 changes: 2 additions & 2 deletions Sources/SPMTestSupport/TestWorkspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -346,15 +346,15 @@ public final class TestWorkspace {
}

public func set(
pins: [PackageReference: (version: CheckoutState, products: ProductFilter)] = [:],
pins: [PackageReference: CheckoutState] = [:],
managedDependencies: [ManagedDependency] = [],
managedArtifacts: [ManagedArtifact] = []
) throws {
let workspace = createWorkspace()
let pinsStore = try workspace.pinsStore.load()

for (ref, state) in pins {
pinsStore.pin(packageRef: ref, state: state.version)
pinsStore.pin(packageRef: ref, state: state)
}

for dependency in managedDependencies {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Workspace/ResolverPrecomputationProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ struct ResolverPrecomputationProvider: PackageContainerProvider {
completion: @escaping (Result<PackageContainer, Error>) -> Void
) {
// Start by searching manifests from the Workspace's resolved dependencies.
if let manifest = dependencyManifests.dependencies.first(where: { $1.packageRef == identifier }) {
if let manifest = dependencyManifests.dependencies.first(where: { _, managed, _ in managed.packageRef == identifier }) {
let container = LocalPackageContainer(
package: identifier,
manifest: manifest.manifest,
Expand Down
52 changes: 25 additions & 27 deletions Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,13 @@ public class Workspace {
let root: PackageGraphRoot

/// The dependency manifests in the transitive closure of root manifest.
let dependencies: [(manifest: Manifest, dependency: ManagedDependency)]
let dependencies: [(manifest: Manifest, dependency: ManagedDependency, productFilter: ProductFilter)]

let workspace: Workspace

fileprivate init(
root: PackageGraphRoot,
dependencies: [(Manifest, ManagedDependency)],
dependencies: [(Manifest, ManagedDependency, ProductFilter)],
workspace: Workspace
) {
self.root = root
Expand Down Expand Up @@ -244,7 +244,7 @@ public class Workspace {
func dependencyConstraints() -> [RepositoryPackageConstraint] {
var allConstraints = [RepositoryPackageConstraint]()

for (externalManifest, managedDependency) in dependencies {
for (externalManifest, managedDependency, productFilter) in dependencies {
// For edited packages, add a constraint with unversioned requirement so the
// resolver doesn't try to resolve it.
switch managedDependency.state {
Expand All @@ -259,13 +259,13 @@ public class Workspace {
let constraint = RepositoryPackageConstraint(
container: ref,
requirement: .unversioned,
products: managedDependency.productFilter)
products: productFilter)
allConstraints.append(constraint)
case .checkout, .local:
break
}
allConstraints += externalManifest.dependencyConstraints(
productFilter: managedDependency.productFilter,
productFilter: productFilter,
config: workspace.config
)
}
Expand All @@ -277,7 +277,7 @@ public class Workspace {
public func editedPackagesConstraints() -> [RepositoryPackageConstraint] {
var constraints = [RepositoryPackageConstraint]()

for (_, managedDependency) in dependencies {
for (_, managedDependency, productFilter) in dependencies {
switch managedDependency.state {
case .checkout, .local: continue
case .edited: break
Expand All @@ -292,7 +292,7 @@ public class Workspace {
let constraint = RepositoryPackageConstraint(
container: ref,
requirement: .unversioned,
products: managedDependency.productFilter)
products: productFilter)
constraints.append(constraint)
}
return constraints
Expand Down Expand Up @@ -568,7 +568,8 @@ extension Workspace {
requirement = currentState.requirement()
}
let constraint = RepositoryPackageConstraint(
container: dependency.packageRef, requirement: requirement, products: dependency.productFilter)
// If any products are required, the rest of the package graph will supply those constraints.
container: dependency.packageRef, requirement: requirement, products: .specific([]))

// Run the resolution.
_resolve(root: root, forceResolution: false, extraConstraints: [constraint], diagnostics: diagnostics)
Expand Down Expand Up @@ -1039,12 +1040,11 @@ extension Workspace {
try fileSystem.removeFileTree(editablesPath)
}

if let checkoutState = dependency.basedOn?.checkoutState,
let productFilter = dependency.basedOn?.productFilter {
if let checkoutState = dependency.basedOn?.checkoutState {
// Restore the original checkout.
//
// The clone method will automatically update the managed dependency state.
_ = try clone(package: dependency.packageRef, at: checkoutState, productFilter: productFilter)
_ = try clone(package: dependency.packageRef, at: checkoutState)
} else {
// The original dependency was removed, update the managed dependency state.
state.dependencies.remove(forURL: dependency.packageRef.path)
Expand Down Expand Up @@ -1191,17 +1191,17 @@ extension Workspace {
var loadedManifests = [String: Manifest]()

// Compute the transitive closure of available dependencies.
let allManifests = try! topologicalSort(inputManifests.map({ KeyedPair($0, key: $0.name) })) { node in
return node.item.dependencies.compactMap({ dependency in
let url = config.mirroredURL(forURL: dependency.url)
let allManifests = try! topologicalSort(inputManifests.map({ KeyedPair(($0, ProductFilter.everything), key: $0.name)})) { node in
return node.item.0.dependenciesRequired(for: node.item.1).compactMap({ dependency in
let url = config.mirroredURL(forURL: dependency.declaration.url)
let manifest = loadedManifests[url] ?? loadManifest(forURL: url, diagnostics: diagnostics)
loadedManifests[url] = manifest
return manifest.flatMap({ KeyedPair($0, key: $0.name) })
return manifest.flatMap({ KeyedPair(($0, dependency.productFilter), key: $0.name) })
})
}

let allDependencyManifests = allManifests.map({ $0.item }).filter({ !root.manifests.contains($0) })
let deps = allDependencyManifests.map({ ($0, state.dependencies[forURL: $0.url]!) })
let allDependencyManifests = allManifests.map({ $0.item }).filter({ !root.manifests.contains($0.0) })
let deps = allDependencyManifests.map({ ($0, state.dependencies[forURL: $0.url]!, $1) })

return DependencyManifests(root: root, dependencies: deps, workspace: self)
}
Expand Down Expand Up @@ -1365,7 +1365,7 @@ extension Workspace {
private func artifacts(from manifests: DependencyManifests) -> [ManagedArtifact] {
let packageAndManifests: [(PackageReference, Manifest)] =
zip(manifests.root.packageRefs, manifests.root.manifests) + // Root package and manifests.
manifests.dependencies.map({ ($1.packageRef, $0) }) // Dependency package and manifests.
manifests.dependencies.map({ manifest, managed, _ in (managed.packageRef, manifest) }) // Dependency package and manifests.

var artifacts: [ManagedArtifact] = []

Expand Down Expand Up @@ -1541,7 +1541,7 @@ extension Workspace {
// Clone the required pins.
for pin in requiredPins {
diagnostics.wrap {
_ = try self.clone(package: pin.packageRef, at: pin.state, productFilter: .everything)
_ = try self.clone(package: pin.packageRef, at: pin.state)
}
}

Expand All @@ -1552,7 +1552,7 @@ extension Workspace {
let dependencies = rootManifest.dependencies.filter{ $0.requirement == .localPackage }
for localPackage in dependencies {
let package = localPackage.createPackageRef(config: self.config)
state.dependencies.add(ManagedDependency.local(packageRef: package, productFilter: .everything))
state.dependencies.add(ManagedDependency.local(packageRef: package))
}
}
diagnostics.wrap { try state.saveState() }
Expand Down Expand Up @@ -2090,7 +2090,7 @@ extension Workspace {
switch dependency.state {
case .checkout(let checkoutState):
// If some checkout dependency has been removed, clone it again.
_ = try clone(package: dependency.packageRef, at: checkoutState, productFilter: dependency.productFilter)
_ = try clone(package: dependency.packageRef, at: checkoutState)
diagnostics.emit(.checkedOutDependencyMissing(packageName: dependency.packageRef.name))

case .edited:
Expand Down Expand Up @@ -2235,8 +2235,7 @@ extension Workspace {
/// - Throws: If the operation could not be satisfied.
func clone(
package: PackageReference,
at checkoutState: CheckoutState,
productFilter: ProductFilter
at checkoutState: CheckoutState
) throws -> AbsolutePath {
// Get the repository.
let path = try fetch(package: package)
Expand All @@ -2256,8 +2255,7 @@ extension Workspace {
state.dependencies.add(ManagedDependency(
packageRef: package,
subpath: path.relative(to: checkoutsPath),
checkoutState: checkoutState,
productFilter: productFilter))
checkoutState: checkoutState))
try state.saveState()

return path
Expand Down Expand Up @@ -2287,12 +2285,12 @@ extension Workspace {
checkoutState = CheckoutState(revision: revision, branch: branch)

case .unversioned:
state.dependencies.add(ManagedDependency.local(packageRef: package, productFilter: productFilter))
state.dependencies.add(ManagedDependency.local(packageRef: package))
try state.saveState()
return AbsolutePath(package.path)
}

return try self.clone(package: package, at: checkoutState, productFilter: productFilter)
return try self.clone(package: package, at: checkoutState)
}

/// Removes the clone and checkout of the provided specifier.
Expand Down
20 changes: 4 additions & 16 deletions Sources/Workspace/WorkspaceState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ public final class ManagedDependency {
/// The state of the managed dependency.
public let state: State

/// The product filter applied to the package.
public let productFilter: ProductFilter

/// The checked out path of the dependency on disk, relative to the workspace checkouts path.
public let subpath: RelativePath

Expand All @@ -65,25 +62,21 @@ public final class ManagedDependency {
public init(
packageRef: PackageReference,
subpath: RelativePath,
checkoutState: CheckoutState,
productFilter: ProductFilter
checkoutState: CheckoutState
) {
self.packageRef = packageRef
self.state = .checkout(checkoutState)
self.productFilter = productFilter
self.basedOn = nil
self.subpath = subpath
}

/// Create a dependency present locally on the filesystem.
public static func local(
packageRef: PackageReference,
productFilter: ProductFilter
packageRef: PackageReference
) -> ManagedDependency {
return ManagedDependency(
packageRef: packageRef,
state: .local,
productFilter: productFilter,
// FIXME: This is just a fake entry, we should fix it.
subpath: RelativePath(packageRef.identity),
basedOn: nil
Expand All @@ -93,15 +86,13 @@ public final class ManagedDependency {
private init(
packageRef: PackageReference,
state: State,
productFilter: ProductFilter,
subpath: RelativePath,
basedOn: ManagedDependency?
) {
self.packageRef = packageRef
self.subpath = subpath
self.basedOn = basedOn
self.state = state
self.productFilter = productFilter
}

private init(
Expand All @@ -114,7 +105,6 @@ public final class ManagedDependency {
self.packageRef = dependency.packageRef
self.subpath = subpath
self.state = .edited(unmanagedPath)
self.productFilter = dependency.productFilter
}

/// Create an editable managed dependency based on a dependency which
Expand Down Expand Up @@ -358,7 +348,6 @@ extension ManagedDependency: JSONMappable, JSONSerializable, CustomStringConvert
try self.init(
packageRef: json.get("packageRef"),
state: json.get("state"),
productFilter: json.get("products"),
subpath: RelativePath(json.get("subpath")),
basedOn: json.get("basedOn")
)
Expand All @@ -369,13 +358,12 @@ extension ManagedDependency: JSONMappable, JSONSerializable, CustomStringConvert
"packageRef": packageRef.toJSON(),
"subpath": subpath,
"basedOn": basedOn.toJSON(),
"state": state,
"products": productFilter
"state": state
])
}

public var description: String {
return "<ManagedDependency: \(packageRef.name) \(state) \(productFilter)>"
return "<ManagedDependency: \(packageRef.name) \(state)>"
}
}

Expand Down
7 changes: 2 additions & 5 deletions Tests/WorkspaceTests/PinsStoreTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ final class PinsStoreTests: XCTestCase {
let barRef = PackageReference(identity: bar, path: barRepo.url)

let state = CheckoutState(revision: revision, version: v1)
let products: ProductFilter = .everything
let pin = PinsStore.Pin(packageRef: fooRef, state: state)
// We should be able to round trip from JSON.
XCTAssertEqual(try PinsStore.Pin(json: pin.toJSON()), pin)
Expand Down Expand Up @@ -120,8 +119,7 @@ final class PinsStoreTests: XCTestCase {
"branch": null,
"revision": "90a9574276f0fd17f02f58979423c3fd4d73b59e",
"version": "1.0.2",
},
"products": ["a", "b"]
}
},
{
"package": "Commandant",
Expand All @@ -130,8 +128,7 @@ final class PinsStoreTests: XCTestCase {
"branch": null,
"revision": "c281992c31c3f41c48b5036c5a38185eaec32626",
"version": "0.12.0"
},
"products": "all"
}
}
]
},
Expand Down
Loading