Improved error messages for when swiftc, clang, or sysroot can't be found #588
Conversation
| @@ -19,7 +19,7 @@ import var Utility.stderr | |||
|
|
|||
| public enum Error: Swift.Error { | |||
| case noManifestFound | |||
| case invalidToolchain | |||
| case invalidToolchain(problem: String) | |||
There was a problem hiding this comment.
I realize that having a named property here is inconsistent with the other enum values; I will add names to them in a separate diff but wanted to keep this one small. The reason for adding names will become more clear when we add a solution property as well.
|
I am going to do local checking on Linux and also check various error conditions before doing a smoke test and a merge. |
|
I would like to add unit tests for this, but we don't seem to have any good structure for testing various invocations of the |
|
@swift-ci please test |
|
Linux failure is: fatal: unable to access 'https://github.com/apple/swift-corelibs-libdispatch.git/': Could not resolve host: github.com |
| @@ -14,7 +14,7 @@ import PackageLoading | |||
| import Utility | |||
|
|
|||
| extension Command { | |||
| static func compile(swiftModule module: SwiftModule, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], SWIFT_EXEC: String) throws -> Command { | |||
| static func compile(swiftModule module: SwiftModule, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], compilerExec: AbsolutePath) throws -> Command { | |||
There was a problem hiding this comment.
It seems like we should clean up this area to just pass the toolchain around...
There was a problem hiding this comment.
I agree. I wanted to keep this diff minimal while still doing the basic work of cleaning up the error message, but it make sense to do it as a separate diff in the same PR. I'll do that.
|
LGTM, it is unfortunate we won't have test coverage of it though. |
|
@swift-ci please test os x |
abertelrud
force-pushed
the
error-improvements
branch
from
68f732b to
7bd919e
Compare
|
I agree. I forgot where, but it one of the commit messages (I think) I mentioned the reason. We need to add an infrastructure for testing environment settings and other external conditions (e.g. what We have a similar issue in not having testing of the command lines. We definitely need to add testing of that but I didn't want to block this change on that. I'll make sure that there is a JIRA, though. |
|
One way would be to never call getenv() or shell out to |
|
I have updated to top-of-tree and will land after it passes tests (tests seem to get cancelled when you push an update). |
|
@swift-ci please smoke test |
…ound. No fix-its yet, but they will come. https://bugs.swift.org/browse/SR-2271
…mes and types, and also add comments to document what they are.
…improvements to toolchain loading error messages propagate through the APIs.
abertelrud
force-pushed
the
error-improvements
branch
from
7bd919e to
2736dd9
Compare
|
@swift-ci please smoke test |
No fix-its yet, but they will be added in further diffs.
https://bugs.swift.org/browse/SR-2271