Support background indexing when cross-compiling #1923
Conversation
|
Would be nice to add tests for this but as far as I can tell
|
|
Thank you @kabiroberai. Looks like I indeed just missed passing those options through. Regarding a test case: If you can conjure one up with the iOS SDK, that would be great. To account for Xcode installations that might not include an iOS SDK, I think we should guard it behind a |
| let project = try await SwiftPMTestProject( | ||
| files: [ | ||
| "Lib/MyFile.swift": """ | ||
| public func foo() {} | ||
| """, | ||
| ], | ||
| manifest: """ | ||
| let package = Package( | ||
| name: "MyLibrary", | ||
| targets: [ | ||
| .target(name: "Lib"), | ||
| ] | ||
| ) | ||
| """ | ||
| ) |
There was a problem hiding this comment.
I think you should be able to reduce this a little bit further to
| let project = try await SwiftPMTestProject( | |
| files: [ | |
| "Lib/MyFile.swift": """ | |
| public func foo() {} | |
| """, | |
| ], | |
| manifest: """ | |
| let package = Package( | |
| name: "MyLibrary", | |
| targets: [ | |
| .target(name: "Lib"), | |
| ] | |
| ) | |
| """ | |
| ) | |
| let project = try await SwiftPMTestProject( | |
| files: [ | |
| "MyFile.swift": """ | |
| public func foo() {} | |
| """ | |
| ) |
| var arguments = [ | ||
| try swift.filePath, "build", "--package-path", try project.scratchDirectory.filePath, "--target", "Lib", | ||
| "--swift-sdk", "arm64-apple-ios", | ||
| ] | ||
| if let globalModuleCache = try globalModuleCache { | ||
| arguments += ["-Xswiftc", "-module-cache-path", "-Xswiftc", try globalModuleCache.filePath] | ||
| } | ||
| let status = try await Process.run(arguments: arguments, workingDirectory: nil) | ||
| guard case .terminated(code: 0) = status.exitStatus else { | ||
| let error = (try? String(decoding: status.stderrOutput.get(), as: UTF8.self)) ?? "unknown error" | ||
| return .featureUnsupported(skipMessage: "Cannot build for iOS: \(error)") | ||
| } |
There was a problem hiding this comment.
Could this be the following?
| var arguments = [ | |
| try swift.filePath, "build", "--package-path", try project.scratchDirectory.filePath, "--target", "Lib", | |
| "--swift-sdk", "arm64-apple-ios", | |
| ] | |
| if let globalModuleCache = try globalModuleCache { | |
| arguments += ["-Xswiftc", "-module-cache-path", "-Xswiftc", try globalModuleCache.filePath] | |
| } | |
| let status = try await Process.run(arguments: arguments, workingDirectory: nil) | |
| guard case .terminated(code: 0) = status.exitStatus else { | |
| let error = (try? String(decoding: status.stderrOutput.get(), as: UTF8.self)) ?? "unknown error" | |
| return .featureUnsupported(skipMessage: "Cannot build for iOS: \(error)") | |
| } | |
| try await SwiftPMTestProject.build(at: try project.scratchDirectory, extraArguments: ["--swift-sdk", "arm64-apple-ios"]) |
|
Good callouts, ty! |
|
@swift-ci Please test |
|
Could you format your changes? (https://github.com/swiftlang/sourcekit-lsp/blob/main/CONTRIBUTING.md#formatting) Also looks like the new test failed on Linux, though I’m not sure why the skipping wouldn’t work. Could you take a look? |
|
Can format, my bad! As for the test not being skipped on Linux, it looks like sourcekit-lsp/Sources/SKTestSupport/SkipUnless.swift Lines 98 to 100 in f1251c0 Looks like we can override this with |
Head branch was pushed to by a user without write access
Feedback More cleanup Allow skipping canSwiftPMCompileForIOS on CI Format
kabiroberai
force-pushed
the
kabir/cross-index
branch
from
da15845 to
6b533b3
Compare
|
Formatted and fixed the CI issue. I think we should be good now! |
|
Ah, completely forgot about |
|
@swift-ci Please test |
|
@ahoppen Linux + macOS CI is green, do we need to request a Windows run? |
Previously, if you selected a Swift SDK via the
swiftPM.swiftSDKconfiguration option, background indexing would fail because theswift build --experimental-prepare-for-indexinginvocation did not pass along the--swift-sdkargument. This PR fixes this by passing along that flag (+ other relevant cross-compilation flags) toswift build.Best I can tell, the reason this was previously missing is simply that the PRs that added these two features (background indexing and Swift SDK support) were being worked on in parallel and didn't end up accounting for each other. Fortunately it's a trivial fix.