-
Notifications
You must be signed in to change notification settings - Fork 48
JavaKit: check each classPath element exists before starting VM #150
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
f66e017
to
fa30fb0
Compare
Todo: check open-ing a directory is portable (specifically on Windows). |
For sure, just thought avoiding Foundation was preferable. This won't pull in ICU? |
b4f47c3
to
a35d4ab
Compare
OK, use FoundationEssentials where possible. |
a35d4ab
to
dcbd6b4
Compare
maybe we move VMError under this? |
import FoundationEssentials | ||
#else | ||
import Foundation | ||
#endif |
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.
Yup that's the way, thank you
dcbd6b4
to
4a92f99
Compare
Thanks for dealing with the nitpicks :) |
@@ -268,4 +280,8 @@ extension JavaVirtualMachine { | |||
} | |||
} | |||
} | |||
|
|||
enum JavaKitError: Error { | |||
case classPathEntryNotFound(entry: String, classPath: String) |
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.
case classPathEntryNotFound(entry: String, classPath: String) | |
case classPathEntryNotFound(entry: String, classPath: [String]) |
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.
Fixing compilation
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.
Oh, it was supposed to be colonSeparatedClassPath
– must have dropped that diff somehow. But I suppose the colon separation is an implementation detail. I'll do as you suggest above.
If you do the "maintainers can edit" on the right I can more easily fix simple stuff like that :) There's a simple compilation error, see proposed fix. |
4a92f99
to
b08446a
Compare
Hmm, weirdly I can't seem to find this checkbox... |
Either way, thank you merged :) |
Huh interesting, didn't realize this limitation. |
Partial fix for #136: validate each classPath element by attempting to open it before starting VM. It would be more efficient to stat() it but SystemPackage lacks an abstraction for checking if a file exists, and we don't want to import Foundation.