Fix a SwiftPM 5.2 regression that cause it to try to compile unknown files in pre-5.3 manifests #2708
Conversation
…how package manifests declaring a tools version earlier than 5.3 were parsed. Specifically, files with an unknown source type are not ignored, as they were in SwiftPM 5.1.x and earlier. This leads to issues such as the one reported here: https://swiftpm.slack.com/archives/C14QU6CUU/p1587442104122300
|
@swift-ci smoke test |
| @@ -1659,6 +1659,30 @@ class PackageBuilderTests: XCTestCase { | |||
| } | |||
| } | |||
|
|
|||
| func testUnknownSourceFilesUnderDeclaredSourcesIgnoredInV5_2Manifest() throws { | |||
There was a problem hiding this comment.
Can you also write a unit test for 5.3 or do we already cover that using some other test?
There was a problem hiding this comment.
It's already covered, but maybe a more targeted one is a good idea.
There was a problem hiding this comment.
I've added a more targeted test. I'm not sure that SwiftPM is doing the right thing in the case of missing rules for a 5.3 manifest, though... it doesn't seem logical to try to compile files with unknown suffixes.
There was a problem hiding this comment.
That's a separate issue, and shouldn't block this fix, but something to discuss.
There was a problem hiding this comment.
But that does seem to be the intentional 5.3 semantics in the source code.
There was a problem hiding this comment.
This is discussed in the "Rules for determining resource files" section of the proposal, but perhaps not in sufficient detail. There were many iterations on the proposed semantics, but I thought where we ended up is that, while unambiguous types of files such as .metal, .storyboard, .xcassets, etc should be treated as resources, any other types would need to be specified as resources. So this should result in an error requiring the user to either specify it as excluded or as a resource.
It sounds as if this needs to be clarified in a follow-on proposal, and the whole question of how to select rules for input files is of course related to the question of how to extend those rules for custom types (which is clearly beyond the scope of just clarifying the behavior here).
As things stand, what will happen if you mix in an unknown type with known compilable types (such as .c, .swift, etc) in 5.3 is that you'll get an error saying that multi-language targets aren't supported. That's not ideal, but it's not a silent issue. It's only in the case of an entire target full of unknown types of the same kind that it will try to compile them.
There was a problem hiding this comment.
Are we comfortable with the fix for the regression, and taking the question of 5.3 behavior as a follow-up discussion?
There was a problem hiding this comment.
I've been doing some more local testing of this, and will merge.
There was a problem hiding this comment.
Can you file a JIRA for tracking the issue for 5.3?
There was a problem hiding this comment.
…le in 5.3 manifests (there would still be a later error about mismatched languages in a single target.
|
@swift-ci please smoke test |
abertelrud
deleted the
61061140-unknown-files-under-declared-source-directories-are-not-ignored-in-pre-5.3-manifests
branch
SwiftPM 5.2 introduced a regression that caused a semantic change in how package manifests declaring a tools version earlier than 5.3 were parsed. Specifically, files with an unknown source type are not ignored, as they were in SwiftPM 5.1.x and earlier. This leads to issues such as the one reported here: https://swiftpm.slack.com/archives/C14QU6CUU/p1587442104122300