initial implmentation of dependencies coming from registry #3635

tomerd · 2021-07-23T23:27:44Z

motivation: with SE-0292 accepted, we can start implementing the SwiftPM side of it

the scope of this work only includes infrastructure to parse dependencies coming from registry, significant follow up work is required to complete the feature

changes:

add manifest support for dependencies coming from registry
add manifest parsing support for dependencies coming from registry
refactor relevant code to support new dependency type
add and adjust tests

tomerd · 2021-07-23T23:42:12Z

cc @mattt @yim-lee

tomerd · 2021-07-23T23:42:20Z

@swift-ci please smoke test

mattt

Thanks for digging into this, @tomerd. Just a few suggestions to get this aligned with the spec. Once we have this in, I should be able to wire up support with the registry client quickly.

Sources/PackageDescription/PackageDependency.swift

Tests/PackageLoadingTests/PD_Next_LoadingTests.swift

Sources/PackageDescription/PackageDependency.swift

tomerd · 2021-07-27T01:22:43Z

thanks for the feedback @mattt addressed with 713a453

tomerd · 2021-07-27T01:34:50Z

@swift-ci please smoke test

tomerd · 2021-07-27T02:14:21Z

macOS self hosted uses 5.3 which does not support codable synthesis for enums with associated values. checking in with @shahmishal if we can update that to 5.4

tomerd · 2021-07-27T18:06:34Z

@swift-ci please smoke test

Tests/PackageLoadingTests/PD_Next_LoadingTests.swift

tomerd · 2021-07-27T18:09:18Z

@swift-ci please smoke test

tomerd · 2021-07-27T18:11:42Z

@yim-lee please note this will change the output of swift package describe which may impact the collections generator?

tomerd · 2021-07-27T18:14:51Z

Sources/PackageModel/Manifest/PackageDependencyDescription.swift


+    case fileSystem(FileSystem)
+    case sourceControl(SourceControl)
+    case registry(Registry)


@neonichu @abertelrud are you happy with these modified names? note local -> fileSystem and scm -> sourceControl

The names seem good, and in general, this PR is an improvement. I'm a bit concerned about the absolute path being stored for the path, which isn't easily round-trippable when generating the source for a package manifest, but it looks as if that was already the case. That might be a bug that we have today (I think I've seen the same thing when describing a package, i.e. that the absolute path is emitted rather than the one that had been specified). Since it isn't new in this PR, that can be fixed later.

yim-lee · 2021-07-27T19:09:00Z

please note this will change the output of swift package describe which may impact the collections generator?

Currently the generator has minimal usage of describe and only extracts name and targets (name and c99name) from the output. AFAICT this PR doesn't change that part of the code, so I think we are ok.

mattt · 2021-07-27T19:25:23Z

Please note this will change the output of swift package describe which may impact the collections generator?

I'm relying on the output of swift package describe for a few different tools that integrate with Swift Package Manager, including https://github.com/spdx/spdx-sbom-generator. While we're making changes, would you be interested in a PR / proposal to expand and stabilize the format of this command?

tomerd · 2021-07-27T19:42:08Z

I'm relying on the output of swift package describe for a few different tools that integrate with Swift Package Manager, including https://github.com/spdx/spdx-sbom-generator. While we're making changes, would you be interested in a PR / proposal to expand and stabilize the format of this command?

that sounds great! probably material for follow up PR tho?

tomerd · 2021-07-27T19:42:38Z

@shahmishal macOS Platform (self hosted) still on 5.3 causing this PR test failure

mattt · 2021-07-27T21:12:30Z

I'm relying on the output of swift package describe for a few different tools that integrate with Swift Package Manager, including https://github.com/spdx/spdx-sbom-generator. While we're making changes, would you be interested in a PR / proposal to expand and stabilize the format of this command?

that sounds great! probably material for follow up PR tho?

Yes, definitely. Not at all a blocker for this PR or the registry feature generally.

tomerd · 2021-07-31T00:01:31Z

@swift-ci please smoke test

abertelrud · 2021-08-02T22:48:40Z

I'm relying on the output of swift package describe for a few different tools that integrate with Swift Package Manager, including https://github.com/spdx/spdx-sbom-generator. While we're making changes, would you be interested in a PR / proposal to expand and stabilize the format of this command?

that sounds great! probably material for follow up PR tho?

I agree, that would be fantastic (as follow-on PR). There are some quirks to the describe command that are a bit unfortunate, especially around how dependencies are represented, and it could also use some control over how much details is provided. I'd love to see what you had in mind.

abertelrud

Thanks for these changes, this looks like good cleanup!

abertelrud · 2021-08-02T23:30:11Z

Sources/PackageDescription/PackageDependency.swift

-    @available(_PackageDescription, obsoleted: 5.2)
+    /// The following example instruct the Swift Package Manager to use version `1.2.3`.
+    ///
+    ///    .package(identity: "scope.name", exact: "1.2.3"),


Since this is the documentation for the URL version, should the example use URL and not identity?

abertelrud · 2021-08-02T23:30:46Z

Sources/PackageDescription/PackageDependency.swift

-    @available(_PackageDescription, introduced: 5.2)
+    /// The following example instruct the Swift Package Manager to use version `1.2.3`.
+    ///
+    ///    .package(identity: "scope.name", exact: "1.2.3"),


Same comment about URL and identity.

abertelrud · 2021-08-03T03:03:19Z

Sources/PackageModel/Manifest/PackageDependencyDescription.swift


+    case fileSystem(FileSystem)
+    case sourceControl(SourceControl)
+    case registry(Registry)


The names seem good, and in general, this PR is an improvement. I'm a bit concerned about the absolute path being stored for the path, which isn't easily round-trippable when generating the source for a package manifest, but it looks as if that was already the case. That might be a bug that we have today (I think I've seen the same thing when describing a package, i.e. that the absolute path is emitted rather than the one that had been specified). Since it isn't new in this PR, that can be fixed later.

motivation: with SE-0292 accepted, we can start implementing the SwiftPM side of it the scope of this work only includes basic infrastrucuture to parse dependencies coming from registry, follow up work is reauired to complete the feature changes: * add manifest support for dependencies coming from registry * add manifest parsing support for dependencies coming from registry * refactor relevant code to support new dependency type * add and adjust tests

* refactor manifet encoding and parsing to more clearly define dependency type and associated metadata * make .upToNextMajor and .upToNextMinor extensions on Range<Version> to reuse across new requirment types * adjust tests

tomerd · 2021-08-03T22:30:32Z

@swift-ci please smoke test

tomerd · 2021-08-03T23:36:28Z

@swift-ci please smoke test

compnerd · 2021-08-04T03:23:49Z

This seems to have regressed the Windows build: https://ci-external.swift.org/job/oss-swift-windows-toolchain-x86_64-vs2019/93/consoleText

…3635)" This reverts commit 8dd0202.

The C imports are important, particularly on Windows, where `HANDLE` is not available without the platform SDK (`WinSDK` is roughly synonymous to `Darwin`).

tomerd requested review from abertelrud, friedbunny and neonichu as code owners July 23, 2021 23:27

tomerd force-pushed the feature/registry branch from 0deff0d to 13a9d80 Compare July 23, 2021 23:41

mattt reviewed Jul 26, 2021

View reviewed changes

Sources/PackageDescription/PackageDependency.swift Outdated Show resolved Hide resolved

Tests/PackageLoadingTests/PD_Next_LoadingTests.swift Outdated Show resolved Hide resolved

Sources/PackageDescription/PackageDependency.swift Outdated Show resolved Hide resolved

tomerd self-assigned this Jul 26, 2021

tomerd force-pushed the feature/registry branch 6 times, most recently from 6d77a3c to 713a453 Compare July 27, 2021 01:22

neonichu approved these changes Jul 27, 2021

View reviewed changes

mattt approved these changes Jul 27, 2021

View reviewed changes

Tests/PackageLoadingTests/PD_Next_LoadingTests.swift Show resolved Hide resolved

tomerd commented Jul 27, 2021

View reviewed changes

mattt mentioned this pull request Jul 29, 2021

Implement dependency resolution through package registry #3640

Closed

4 tasks

tomerd force-pushed the feature/registry branch from 27d2b45 to 41b4bc0 Compare July 31, 2021 00:00

tomerd mentioned this pull request Jul 31, 2021

deprecate name attribute on package dependencies #3641

Merged

abertelrud approved these changes Aug 3, 2021

View reviewed changes

tomerd mentioned this pull request Aug 3, 2021

Add tools version 5.6 and set it as the current version for main branch #3643

Merged

tomerd added 8 commits August 3, 2021 14:14

further api cleanup

aff3f34

cmake

843295e

cleanup

81cf2a4

cleanup

1c163c8

wip

1c96ddb

wip

7e7c9ba

tomerd force-pushed the feature/registry branch from 41b4bc0 to 7e7c9ba Compare August 3, 2021 22:30

wip

2e02bf3

tomerd merged commit 8dd0202 into swiftlang:main Aug 4, 2021

compnerd added a commit that referenced this pull request Aug 4, 2021

Revert "initial implementation of dependencies coming from registry (#…

ae7334e

…3635)" This reverts commit 8dd0202.

compnerd mentioned this pull request Aug 4, 2021

Revert "initial implmentation of dependencies coming from registry" #3645

Closed

compnerd added a commit that referenced this pull request Aug 4, 2021

PackageDescription: repair the build after #3635 (#3646)

0a17db0

glessard mentioned this pull request May 4, 2022

[SR-15558] Swift Package Manager (main branch) fails to build swift-standard-library-preview #4371

Closed

Uh oh!

initial implmentation of dependencies coming from registry #3635

initial implmentation of dependencies coming from registry #3635

Uh oh!

Conversation

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

Uh oh!

Uh oh!

tomerd commented Jul 23, 2021

Uh oh!

tomerd commented Jul 23, 2021

Uh oh!

mattt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomerd commented Jul 27, 2021

Uh oh!

tomerd commented Jul 27, 2021

Uh oh!

tomerd commented Jul 27, 2021

Uh oh!

tomerd commented Jul 27, 2021

Uh oh!

Uh oh!

tomerd commented Jul 27, 2021

Uh oh!

tomerd commented Jul 27, 2021

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abertelrud Aug 3, 2021

Choose a reason for hiding this comment

Uh oh!

yim-lee commented Jul 27, 2021

Uh oh!

mattt commented Jul 27, 2021

Uh oh!

tomerd commented Jul 27, 2021

Uh oh!

tomerd commented Jul 27, 2021

Uh oh!

mattt commented Jul 27, 2021

Uh oh!

tomerd commented Jul 31, 2021

Uh oh!

abertelrud commented Aug 2, 2021

Uh oh!

abertelrud left a comment

Choose a reason for hiding this comment

Uh oh!

abertelrud Aug 2, 2021

Choose a reason for hiding this comment

Uh oh!

abertelrud Aug 2, 2021

Choose a reason for hiding this comment

Uh oh!

abertelrud Aug 3, 2021

Choose a reason for hiding this comment

Uh oh!

tomerd commented Aug 3, 2021

Uh oh!

tomerd commented Aug 3, 2021

Uh oh!

compnerd commented Aug 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tomerd commented Jul 23, 2021 •

edited

Loading

tomerd Jul 27, 2021 •

edited

Loading