Support vending products that are backed by binaryTargets #5810
Conversation
|
@swift-ci please smoke test |
|
Some tests need updating due to new diagnostic, but this one surprises me a bit: I wonder if |
neonichu
force-pushed
the
support-binary-executable-products
branch
2 times, most recently
from
5fbbcb9
to
799aeee
Compare
| @@ -638,6 +638,11 @@ public final class BinaryTarget: Target { | |||
| } | |||
| } | |||
|
|
|||
| public var containsExecutable: Bool { | |||
| // FIXME: needs to be revisited once libraries are supported in artifact bundles | |||
There was a problem hiding this comment.
I opted to not open this can of worms here since right now .artifactsArchive implies that there is an executable. Addressing this required too many changes that are completely unrelated to this PR and which should be done once we implement support for libraries in artifact bundles.
There was a problem hiding this comment.
I think this is fair, but would be nice if there was some kind of associated data on the artifactsArchive that would allow us to throw an error if it is not an executable. eg .artifactsArchive(type:) not sure if that is the can of warms tho :)
There was a problem hiding this comment.
Yah, that was my approach, but it did require a bunch of changes 🪱 Could be a good change for a separate PR, though.
| } | ||
| // For an executable target we create a `builtTool`. | ||
| else if target.type == .executable { | ||
| // TODO: How do we determine what the executable name will be for the host platform? |
There was a problem hiding this comment.
Note: I deleted this TODO because supporting the case where plugins work when the target platform isn't the host requires significant work in SwiftPM's build system.
| let (_, validationDiagnostics) = try loadAndValidateManifest(content, observabilityScope: observability.topScope) | ||
| XCTAssertNoDiagnostics(observability.diagnostics) | ||
| testDiagnostics(validationDiagnostics) { result in | ||
| result.check(diagnostic: "invalid type for binary product 'FooLibrary'; products referencing only binary targets must have a type of 'library'", severity: .error) |
There was a problem hiding this comment.
This diagnostic can longer be emitted for this particular case, because we cannot know whether a binary target is vending an executable or a library during manifest parsing since we need to crack open the referenced artifact to determine that. There will still be a diagnostic for this case, but it is handled by PackageBuilder and already covered by other existing tests.
neonichu
changed the title
|
@swift-ci please smoke test |
neonichu
force-pushed
the
support-binary-executable-products
branch
from
799aeee
to
45f6c40
Compare
|
@swift-ci please smoke test |
Fixtures/Miscellaneous/Plugins/BinaryToolProductPlugin/Dependency/.gitignore
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
overall looks good. some small suggestions. would be good to get @abertelrud review as he knows this part of the code best
This adds support for vending an executable product that consists solely of a binary target that is backed by an artifact bundle. This allows vending binary executables as their own separate package, independently of the plugins that are using them. rdar://101096803
neonichu
force-pushed
the
support-binary-executable-products
branch
from
45f6c40
to
47aefe7
Compare
|
@swift-ci please smoke test |
This adds support for vending an executable product that consists solely of a binary target that is backed by an artifact bundle. This allows vending binary executables as their own separate package, independently of the plugins that are using them.
rdar://101096803