Handle generated resources from plugins

[neonichu]

rdar://89693335

[neonichu]

Biggest question I have: since we don't support generated sources for Clang targets, yet, I am assuming we do the same for resources for now and handle support for Clang targets separately?

[neonichu]

Oh, there's one major limitation right now: because we do not retain information from the manifest about non-existent files during the initial pass of TargetSourcesBuilder, this will only work for built-in resource types.

Maybe we should treat any unknown resources as to be processed right now? The main reason we don't include anything but certain known types automatically was that we wanted to prevent people from accidentally shipping files with their products, but since any files here are basically already opt-in through depending on a given plugin, that might not be a concern?

[abertelrud]

Oh, there's one major limitation right now: because we do not retain information from the manifest about non-existent files during the initial pass of TargetSourcesBuilder, this will only work for built-in resource types.

Because unknown files are vended to the plugin, I did actually extend TargetSourcesBuilder to carry these through and pass them to the plugin rather than warn about them correctly — so we should be seeing those files now.

Maybe we should treat any unknown resources as to be processed right now? The main reason we don't include anything but certain known types automatically was that we wanted to prevent people from accidentally shipping files with their products, but since any files here are basically already opt-in through depending on a given plugin, that might not be a concern?

To do this completely correctly I think would involve reworking TargetSourcesBuilder to be able to work piecemeal on sets of files, and to also keep more information from the manifest (or pass it in each time). But I do think there is great value in even supporting less flexible situations, so always processing the resource files makes sense for now.

[abertelrud]

Biggest question I have: since we don't support generated sources for Clang targets, yet, I am assuming we do the same for resources for now and handle support for Clang targets separately?

Yes, I think that makes sense. It's unfortunate that there's such a split between Clang targets and Swift targets in SwiftPM, causing overlap for the things that aren't difference. But I think it's a good idea to tackle this piecemeal, as you're doing.

[neonichu]

OK, I think this is basically ready for review now, just need to add some tests.

[neonichu]

@swift-ci please smoke test

[abertelrud]

LGTM (modulo the need for tests as you mentioned). Thank you for taking this on!

[neonichu]

I moved the big switch statement into TargetSourcesBuilder now, so that clients just call a single API and automatically get the correct handling, also taking into account the tools version. This seems preferable to burying this logic in the build system.

[neonichu]

/private/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/Miscellaneous_PluginGeneratedResources.qrxDtx/Miscellaneous_PluginGeneratedResources/.build/plugins/cache/Generator>, <output:
'dyld: lazy symbol binding failed: can'\''t resolve symbol _$ss13_runAsyncMainyyyyYaKcF in /private/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/Miscellaneous_PluginGeneratedResources.qrxDtx/Miscellaneous_PluginGeneratedResources/.build/plugins/cache/Generator because dependent dylib /usr/lib/swift/libswift_Concurrency.dylib could not be loaded
dyld: can'\''t resolve symbol _$ss13_runAsyncMainyyyyYaKcF in /private/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/Miscellaneous_PluginGeneratedResources.qrxDtx/Miscellaneous_PluginGeneratedResources/.build/plugins/cache/Generator because dependent dylib /usr/lib/swift/libswift_Concurrency.dylib could not be loaded
'>

[neonichu]

Self-hosted is running macOS 11.5.2, so we shouldn't be trying to find the concurrency libs in the OS, I think. Also odd that the smoke test passes just fine. Maybe an issue with the superior toolchain?

[neonichu]

It is using

swift --version
swift-driver version: 1.26.7 Apple Swift version 5.5 (swiftlang-1300.0.27.6 clang-1300.0.27.2)
Target: x86_64-apple-macosx11.0

So maybe we'll just disable the test for < 5.6

[neonichu]

@swift-ci please smoke test

[neonichu]

@swift-ci please smoke test

[abertelrud]

It is using

swift --version
swift-driver version: 1.26.7 Apple Swift version 5.5 (swiftlang-1300.0.27.6 clang-1300.0.27.2)
Target: x86_64-apple-macosx11.0

So maybe we'll just disable the test for < 5.6

Oh I see! Yes, this needs at least 5.6.

[tomerd]

looks good, some style nits

[neonichu]

@swift-ci please smoke test

[neonichu]

@swift-ci please smoke test linux