Skip to content

Conversation

@compnerd
Copy link
Member

Some targets use pre-compiled manifests instead of interpretation. Such a manifest will copy the Package.swift to manifest.swift and change the location. As a result, the prefix expectation on the filename will fail. Loosen the condition to accommodate the implementation.

Some targets use pre-compiled manifests instead of interpretation.  Such
a manifest will copy the `Package.swift` to `manifest.swift` and change
the location.  As a result, the prefix expectation on the filename will
fail.  Loosen the condition to accommodate the implementation.
@compnerd
Copy link
Member Author

@swift-ci please smoke test

@neonichu
Copy link
Contributor

I think this is because we don't yet generate a proper VFS for manifest loading on Windows: #5980 -- can we #if os(Windows) the looser expectation, since we do eventually want the stricter one.

@compnerd
Copy link
Member Author

Oh, this is related to VFS stuff? Do you know how difficult it would be to just fix that? I think that the problem was that we didn't invoke the ._nativePathString(escaped: true) when generating the content.

diagnostic: .contains(
"\(pkgDir.appending("Package.swift")):4:8: error: An error in MyPkg"
),
diagnostic: .contains(":4:8: error: An error in MyPkg"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe keep the Package.swift part?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the name of the file also is different: manifest.swift vs Package.swift

@neonichu
Copy link
Contributor

Oh, this is related to VFS stuff? Do you know how difficult it would be to just fix that? I think that the problem was that we didn't invoke the ._nativePathString(escaped: true) when generating the content.

I am not 100% sure, but probably not the difficult?

@compnerd
Copy link
Member Author

@neonichu indeed not: #6693 should do that and does seem to repair the test locally.

@compnerd compnerd closed this Jul 10, 2023
@compnerd compnerd deleted the low-expectations branch July 10, 2023 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants