Support for prebuilt packages in the SDK #7337
Conversation
|
@swift-ci please test |
| } | ||
|
|
||
| // FIXME: We need a real makefile deps parser here... | ||
| let deps = contents.description.split(whereSeparator: { $0.isWhitespace }) |
There was a problem hiding this comment.
This is something we need to solve, but I'll unlikely be able to do it for this PR. One path forward could be exposing llbuild's parser via the Swift API.
| import class TSCUtility.PercentProgressAnimation | ||
| import protocol TSCUtility.ProgressAnimationProtocol | ||
| import var TSCUtility.verbosity | ||
| import TSCUtility // cannot be scoped because of `String.spm_mangleToC99ExtendedIdentifier()` |
There was a problem hiding this comment.
Is there a better way? @MaxDesiatov maybe you have suggestions?
Sources/PackageModel/InstalledLibrariesSupport/LibraryMetadata.swift
Outdated
Show resolved
Hide resolved
|
CMake config needs to be updated as well |
Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift
Outdated
Show resolved
Hide resolved
This should allow satisfying a package dependency with a package in the SDK (or toolchain) based on metadata. If the given prebuilt library is version-compatible, we essentially elide the dependency from the graph. If it is not, we will fetch and build the dependency as normal. If a library with associated metadata is linked without a corresponding package dependency, we'll emit a warning.
|
@swift-ci please test |
|
@swift-ci please test windows |
|
I don't think I will address installing the config as part of this PR which will also conveniently work as a feature flag because no config == no metadata which leads to skipping most of the new logic. |
neonichu
requested review from
bnbarham,
MaxDesiatov and
tomerd
as code owners
|
@swift-ci please test |
|
@swift-ci please test windows |
|
I think this is in a reasonable state to land since it does achieve all basic functionality goals:
Tests are still passing and the new functionality is disabled in production until we actually install a metadata file into the toolchain. During development, we pick up the metadata file from the build directory. |
|
It should be possible to add some tests for the resolution functionality, probably should have one where we use the metadata and one where we fallback to the source package. Not as clear whether we can test the warning behavior since that is implemented in the build system which isn't as amenable to testing with mock data. |
| @@ -32,6 +32,7 @@ extension PackageGraph { | |||
| customPlatformsRegistry: PlatformRegistry? = .none, | |||
| customXCTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion]? = .none, | |||
| testEntryPointPath: AbsolutePath? = nil, | |||
| availableLibraries: [LibraryMetadata], | |||
There was a problem hiding this comment.
Can we have a default empty array argument here so that clients outside of this package relying on this function don't break? This would also fix the smoke test CI jobs, as SourceKit-LSP can't be built with this PR as is.
There was a problem hiding this comment.
I think it's correct that this is a breaking change?
I would see it similar to the binaryArtifacts parameter which also doesn't offer a default value. Whatever gets passed in has to match what was provided to the underlying workspace, so it's important that clients make a deliberate choice here.
I think in the fullness of time this probably shouldn't be a public API, though, or it should be refactored in a way that parameters that are associated with the workspace can't be arbitrarily passed here.
There was a problem hiding this comment.
Oh, this is the loadPackageGraph() on the workspace.
Yah it's a little unfortunate that it needs the parameter, but it's basically the consequence of the toolchain being private and that code being in an extension of workspace. I didn't really want to change the existing assumption that the toolchain object wouldn't be usable there.
|
swiftlang/sourcekit-lsp#1054 |
|
swiftlang/sourcekit-lsp#1054 |
|
swiftlang/sourcekit-lsp#1054 |
1 similar comment
|
swiftlang/sourcekit-lsp#1054 |
#7337 is breaking toolchain builds, so we should pass `SKIP_RESOURCE_SUPPORT` by default to avoid that.
This should allow satisfying a package dependency with a package in the SDK (or toolchain) based on metadata. If the given prebuilt library is version-compatible, we essentially elide the dependency from the graph. If it is not, we will fetch and build the dependency as normal. If a library with associated metadata is linked without a corresponding package dependency, we'll emit a warning.
swiftlang#7337 is breaking toolchain builds, so we should pass `SKIP_RESOURCE_SUPPORT` by default to avoid that.
This should allow satisfying a package dependency with a package in the SDK (or toolchain) based on metadata. If the given prebuilt library is version-compatible, we essentially elide the dependency from the graph. If it is not, we will fetch and build the dependency as normal. If a library with associated metadata is linked without a corresponding package dependency, we'll emit a warning.
swiftlang#7337 is breaking toolchain builds, so we should pass `SKIP_RESOURCE_SUPPORT` by default to avoid that.
This reverts commit cae580e.
This reverts commit cae580e.
This reverts commit cae580e.
This should allow satisfying a package dependency with a package in the SDK (or toolchain) based on metadata. If the given prebuilt library is version-compatible, we essentially elide the dependency from the graph. If it is not, we will fetch and build the dependency as normal. If a library with associated metadata is linked without a corresponding package dependency, we'll emit a warning.