- 
                Notifications
    You must be signed in to change notification settings 
- Fork 213
[Macros] Set -external-plugin-path when the toolchain is not Xcode #1320
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
| fileSystem.exists(xcodePluginPath) | ||
| { | ||
| commandLine.appendFlag(.externalPluginPath) | ||
| commandLine.appendFlag(xcodePluginPath.pathString + "#" + xcodePluginPath.pathString) | 
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.
This is not safe because xcodePluginServerPath can contain #.
@artemcm I want to pass a pair of /path/to/usr/bin/swift-plugin-server and /path/to/usr/lib/swift/host/plugins paths.
Is squashedArgumentList usable for this purpose? Does swift-frontend have a way to correctly "unpack" those values?
faa70f5    to
    7c0946f      
    Compare
  
    | // default toolchain. | ||
| if | ||
| isFrontendArgSupported(.externalPluginPath), | ||
| let darwinToolchain = toolchain as? DarwinToolchain, | 
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.
Can we instead move this to addPlatformSpecificCommonFrontendOptions, please?
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.
Could we please add a macOS-only test for this?
Other than that and moving to code to a toolchain-specific place, LGTM!
7c0946f    to
    168d09b      
    Compare
  
    168d09b    to
    53ad3a9      
    Compare
  
    | @swift-ci Please test | 
rdar://108525615