integrate registry dependencies into dependency resolution #3873

tomerd · 2021-11-17T06:44:59Z

motivation: support registries

changes:

create RegistryPackageContainer to handle dependencies from registry, integrated with the previously introduces registry client
update workspace to create RegistryPackageContainer when dealing with registry dependencies
update registry client (RegitryManager) to perform download request (buffered) when downloading the package archive
update registry client manifest listing and handling so it supportes multi-manifest setups. simplified the client to return the manifest content instead of attempting to load it which is done in RegistryPackageContainer which has additional context
update registry client to use Codable
add testing infrastructure to MockWorkspace to mock registry dependencies
add basic tests for registry scenarios

tomerd · 2021-11-17T06:47:29Z

Tests/WorkspaceTests/WorkspaceTests.swift

+        XCTAssertMatch(workspace.delegate.events, ["did load manifest for remoteSourceControl package: http://localhost/org/bar"])
+    }
+
+    func testBasicResolutionFromRegistry() throws {


tomerd · 2021-11-17T06:48:36Z

@swift-ci please smoke test

tomerd · 2021-11-17T06:50:12Z

@yim-lee note changes to RegistryClient (previously RegistryManager). one important change is the client no longer assume the server buffers the content of the package in the response, and instead supports steaming. this is something we should check in the compatibility tests suite if we have not already

tomerd · 2021-11-17T06:52:11Z

Sources/PackageRegistry/RegistryClient.swift

+        }
+
+        // <http://packages.example.com/mona/LinkedList/1.1.1/Package.swift?swift-version=4>; rel="alternate"; filename="[email protected]"; swift-tools-version="4.0",
+        func parseLinkLine(_ value: String) throws -> ManifestLink? {


this is kinda terrible, open to better ideas

Maybe move all this into an HTTPHeader extension

tomerd · 2021-11-17T19:35:12Z

@swift-ci please smoke test

tomerd · 2021-11-18T00:28:42Z

@swift-ci please smoke test

yim-lee · 2021-11-18T00:39:13Z

Sources/Commands/SwiftPackageRegistryTool.swift

-            guard let url = URL(string: self.url),
-                  url.scheme == "https"
-            else {
+            guard let url = URL(string: self.url), url.scheme == "https" else {


Safer to do case-insensitive compare

Sources/PackageGraph/PackageGraph+Loading.swift

yim-lee · 2021-11-18T00:41:17Z

Sources/PackageModel/PackageIdentity.swift

@@ -65,14 +65,14 @@ public struct PackageIdentity: CustomStringConvertible {
    }

    // TODO: formalize package registry identifier


Unrelated to this PR: is this done?

I would need to take another look. will address in follow up PR of needs more attention, it works fine for the test cases I ran

yim-lee · 2021-11-18T00:46:28Z

Sources/PackageRegistry/RegistryClient.swift

+        }
+
+        // <http://packages.example.com/mona/LinkedList/1.1.1/Package.swift?swift-version=4>; rel="alternate"; filename="[email protected]"; swift-tools-version="4.0",
+        func parseLinkLine(_ value: String) throws -> ManifestLink? {


Maybe move all this into an HTTPHeader extension

Sources/PackageRegistry/RegistryClient.swift

yim-lee · 2021-11-18T01:00:27Z

Sources/PackageRegistry/RegistryClient.swift

+            public var id: String
+            public var version: String
+            public var resources: [Resource]
+            public var metadata: AdditionalMetadata


Do we need this?

I was assuming we would need this, see #3873 (comment)

yim-lee · 2021-11-18T01:04:08Z

Sources/PackageRegistry/RegistryClient.swift

+// MARK: - Serialization
+
+extension RegistryClient {
+    public enum Serialization {


I am not sure expanding a few lines of JSON processing code into JSON models is necessary. I don't think we care about most of the contents. e.g., we only care if a release has problem and don't care what the actual problem is.

I ended up with needing these for the test infrastructure which mimics the server, so instead of doing it only there I reused on both ends

Sources/SPMTestSupport/MockRegistry.swift

motivation: support registries changes: * create RegistryPackageContainer to handle dependencies from registry, integrated with the previously introduces registry client * update workspace to create RegistryPackageContainer when dealing with registry dependencies * update registry client (RegitryManager) to perform download request (buffered) when downloading the package archive * update registry client manifest listing and handling so it supportes multi-manifest setups. simplified the client to return the manifest content instead of attempting to load it which is done in RegistryPackageContainer which has additional context * update registry client to use Codable * add testing infrastructure to MockWorkspace to mock registry dependencies * add basic tests for regsitry scenarios

tomerd · 2021-11-18T06:40:59Z

@swift-ci please smoke test

tomerd requested review from abertelrud, elsh and neonichu as code owners November 17, 2021 06:45

tomerd requested review from yim-lee and mattt November 17, 2021 06:45

tomerd force-pushed the feature/registry-02 branch from 003df90 to 09a9bd8 Compare November 17, 2021 06:46

tomerd commented Nov 17, 2021

View reviewed changes

tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Nov 17, 2021

tomerd force-pushed the feature/registry-02 branch from 6aecf95 to fba8248 Compare November 18, 2021 00:27

yim-lee reviewed Nov 18, 2021

View reviewed changes

tomerd added 4 commits November 17, 2021 22:31

cleanup and more tests

2386bc3

fixup

9aa46a3

rebase

ce1bf91

tomerd force-pushed the feature/registry-02 branch from fba8248 to ce1bf91 Compare November 18, 2021 06:33

address review fedback

f472c78

yim-lee approved these changes Nov 18, 2021

View reviewed changes

abertelrud approved these changes Nov 18, 2021

View reviewed changes

tomerd merged commit c313dea into swiftlang:main Nov 18, 2021

tomerd mentioned this pull request Nov 19, 2021

Implement dependency resolution through package registry #3640

Closed

4 tasks

		@@ -65,14 +65,14 @@ public struct PackageIdentity: CustomStringConvertible {
		}

		// TODO: formalize package registry identifier

integrate registry dependencies into dependency resolution #3873

integrate registry dependencies into dependency resolution #3873

Uh oh!

Conversation

tomerd commented Nov 17, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomerd commented Nov 17, 2021

Uh oh!

tomerd commented Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomerd commented Nov 17, 2021

Uh oh!

tomerd commented Nov 18, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tomerd commented Nov 18, 2021

Uh oh!

Uh oh!

tomerd commented Nov 17, 2021 •

edited

Loading