Do not store products in resolved file

[neonichu]

We can safely make the assumption that any dependency in the resolved file was required.

[neonichu]

@swift-ci please smoke test

[aciidgh]

you can remove this method, it's dead code

[neonichu]

Ah right, I'll make a follow up to clean this up.

[neonichu]

#2793

[SDGGiesbrecht]

Won’t this constraint mean that every pin could potentially want more of its transitive dependencies after it’s loaded than when it was pinned?

If a package is resolved was at 1.0.0 for use of product B, then it will be pinned at 1.0.0 with no product information (which ought to be somehow inferable). Upon loading the pins, it will be loaded at 1.0.0 for use of products A and B, and the resolver will go hunting for all of product A’s dependencies. Or is that not what happens?

Passing .specific([]) might work instead, because I think the resolver will still get the remaining product‐based constraints from the manifests anyway.

I will try both variants and get back to you.

[neonichu]

These constraints are no longer actually being used by the resolver, I removed this method in #2793

[SDGGiesbrecht]

Yes, I see that now. It makes things much simpler.

[SDGGiesbrecht]

I did have time to try it in several variations before I got your last message, and it functioned just fine in everything I tried.

But there was one minor quirk. If a leftover .build is still there from Swift 5.2 and you do swift build with this, there is still a message unable to restore state from [...]/.build/workspace-state.json; missingKey("products"), although it doesn’t cause any actual problems, and goes away for subsequent builds. I’m still looking into whether that message indicates a real problem and if can be avoided.

[neonichu]

Is there a strong reason to store products in WorkspaceState?

[SDGGiesbrecht]

No. It seems to have been an unintentional side effect of adding the productFilter property to ManagedDependency, so that it could hold onto the filter and determine constraints later when asked. Basically it’s the same kind of query as the dead code you removed, just with ManagedDependency instead of Pin. I tried removing it to see how call sites need adjusting. I’m running the tests locally right now.

[SDGGiesbrecht]

If I simply bump the WorkspaceState schema version, the warning vanishes and everything works flawlessly.

diff --git a/Sources/Workspace/WorkspaceState.swift b/Sources/Workspace/WorkspaceState.swift
index 764923b2..3f4b6021 100644
--- a/Sources/Workspace/WorkspaceState.swift
+++ b/Sources/Workspace/WorkspaceState.swift
@@ -205,11 +205,12 @@ public final class WorkspaceState: SimplePersistanceProtocol {
 
     /// The schema version of the resolved file.
     ///
+    /// * 5: Product filters.
     /// * 4: Artifacts.
     /// * 3: Package kind.
     /// * 2: Package identity.
     /// * 1: Initial version.
-    static let schemaVersion: Int = 4
+    static let schemaVersion: Int = 5
 
     /// The dependencies managed by the Workspace.
     public private(set) var dependencies: ManagedDependencies
@@ -232,7 +233,7 @@ public final class WorkspaceState: SimplePersistanceProtocol {
         self.persistence = SimplePersistence(
             fileSystem: fileSystem,
             schemaVersion: WorkspaceState.schemaVersion,
-            supportedSchemaVersions: [2, 3],
+            supportedSchemaVersions: [],
             statePath: statePath,
             otherStatePaths: [dataPath.appending(component: "dependencies-state.json")]
         )

I think it ought to be possible to go without recording the product filter in the ManagedDependency, but there are four places where the version/filter combination currently comes from the ManagedDependency:

• Workspace
  • DependencyManifests
    • dependencyConstraints()
      • Line 262
      • Line 268
    • editedPackagesConstraints()
      • Line 295
  • resolve(packageName:root:version:branch:revision:diagnostics:)
    • • Line 571

I’m now investigating other ways of determining the proper filter at those points.

[SDGGiesbrecht]

I think I’ve gotten it working without needing a record of the products in WorkspaceState. I’ll clean up what I have, do some more tests, and then submit a PR.

[SDGGiesbrecht]

I filed #2794 to complete the clean‐up.