-
Notifications
You must be signed in to change notification settings - Fork 314
Use DocumentURI
instead of URL
in more locations
#1395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@swift-ci Please test |
try await swiftpmBuildSystem.generateBuildGraph(allowFileSystemWrites: false) | ||
|
||
let aPlusSomething = packageRoot.appending(components: "Sources", "lib", "a+something.swift") | ||
print(aPlusSomething.asURL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left over from debugging?
) | ||
) | ||
.compilerArguments | ||
print(arguments) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left over from debugging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I guess I should add an actual check 🙈
@swift-ci Please test |
@swift-ci Please test Windows |
let substituteFile = buildTarget.sources.sorted(by: { $0.path < $1.path }).first | ||
{ | ||
logger.info("😅 Getting compiler arguments for \(url) using substitute file \(substituteFile)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the emoji intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, added it during debugging so I could find this log message more easily. Intended to remove it afterwards and forgot.
@swift-ci Please test |
@swift-ci Please test Windows |
The originator for this change was that I noticed that we weren’t able to index files that contain a
+
because we wouldn’t get build settings for them. This is becauseSwiftPMBuildSystem
had a URL comparison to check if the file that we are requesting build settings for is part of the target’s source files. Now, the target’s source files represented the+
as a literal+
while the requested file name had it escaped as%2B
, so we incorrectly inferred that the source file was not part of the target.DocumentURI
accounts for this and compares the scheme + normalized path separately. So, the solution is to work in terms ofDocumentURI
inSwiftPMBuildSystem
.While doing this, I also changed other files to use
DocumentURI
instead ofURL
in case they have similar issues.