-
Notifications
You must be signed in to change notification settings - Fork 72
Add quick switcher between iOS and macOS targets #381
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
Can one of the admins verify this patch? |
3 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
I was hoping that this would work in the context of a standalone Swift file (no SPM package present), but it doesn't seem to. I don't know if this is because my changes are wrong or because the extension doesn't currently support this. Single files seem to only work for the host's SDK. |
@complexspaces Thanks for your PR and I think it’s a good idea. As for cross-compiling a standalone file, I didn’t add the support because scripts are mostly intended to be run on the host machine directly, but it’s certainly useful to cross-compile a file without running it immediately. Perhaps we can add an action to generate a I initially designed the cross-compilation ability with |
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.
Thank you for the contribution. In general this looks good. A few comments
* | ||
* @param indirect whether to pass the flags by -Xswiftc | ||
*/ | ||
export function swiftDriverTargetFlags(indirect = false): 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.
Not sure about setting the target
command line parameter based on the sdk parameter. It seems a little fragile although I don't see a quick and easy way to set it otherwise.
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.
I agree that this is a bit fragile, but I wasn't sure of another way to do this without exposing SDK versioning information to the user as well, something that wouldn't necessary 95% of the time because XCode's SDK uses symlinks.
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.
What do you think about adding/removing the target parameter from the build arguments setting when you choose the platform? It would be clearer to the user what is happening. The current version is adding hidden arguments which might confuse the user. I don't have an issue with exposing the underlying details to the user
EDIT: forget this. It's just as clunky
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.
If we move to using destination files that'll make this a lot simpler
@@ -46,6 +46,11 @@ interface SwiftTargetInfo { | |||
[name: string]: string | object | undefined; | |||
} | |||
|
|||
export enum DarwinCompatibleTarget { |
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.
do we want to support building for the simulator as well as device.
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.
I wasn't sure about how to expose the simulator, hence why I didn't add it to start. Do you think it should be (in the switcher UI), labeled something like iOS (Simulator)
? I have not personally written any code that required simulator guards so I can't speak for how useful this is.
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.
We can give this a miss just now
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.
Sorry, I'm not familiar with that phrase 😓. Does "give this a miss" (here and other instances in this PR) mean to not worry about it for now?
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.
"give this a miss" means "lets not bother"
@stevapple My use case is probably fairly unique, but I have a large code repository where individual Swift files are compiled (mostly) on their own into standalone static libraries before being linked into larger Rust library builds. If it helps, the organization looks like this:
One of my goals with this PR was to support a structure like this where an IDE such as XCode is unable to work with individual files. It works OK with just macOS today, but we have the need to also have iOS-specific Swift code that uses things like |
e7c96bc
to
09862e4
Compare
* | ||
* @param indirect whether to pass the flags by -Xswiftc | ||
*/ | ||
export function swiftDriverTargetFlags(indirect = false): 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.
What do you think about adding/removing the target parameter from the build arguments setting when you choose the platform? It would be clearer to the user what is happening. The current version is adding hidden arguments which might confuse the user. I don't have an issue with exposing the underlying details to the user
EDIT: forget this. It's just as clunky
@@ -46,6 +46,11 @@ interface SwiftTargetInfo { | |||
[name: string]: string | object | undefined; | |||
} | |||
|
|||
export enum DarwinCompatibleTarget { |
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.
We can give this a miss just now
* | ||
* @param indirect whether to pass the flags by -Xswiftc | ||
*/ | ||
export function swiftDriverTargetFlags(indirect = false): 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.
If we move to using destination files that'll make this a lot simpler
src/toolchain/toolchain.ts
Outdated
break; | ||
} | ||
|
||
const { stdout } = await execFile("xcrun", ["--sdk", sdkType, "--show-sdk-path"]); |
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 you include the following as an argument in the execFile
call. Means we can pass DEVELOPER_DIR
to the xcrun
call and get the sdk from non-default Xcode installs
options: {
env: {...process.env, ...configuration.swiftEnvironmentVariables},
}
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.
Sure, no problem. That seems reasonable to me. I've added the environment variable forwarding to my changes.
09862e4
to
3102d30
Compare
@swift-server-bot test this please |
Thank you for working with me on this one, I'm happy to see it merged 🎉 |
@complexspaces Although I just noticed after merging, building doesn't work. I know you can't build a complete iOS package, but it is still useful to build. By the way thanks for your contribution. |
This PR adds a command palette action to the extension that allows a user to quickly switch between the macOS and iOS SDK targets. It's only available if the host is macOS because the iOS SDK is not (officially) available for distribution on other operating systems.
The goal of these changes are to make the extension a more reasonable replacement for XCode by getting closer to the cross-compiling convenience that XCode offers through its UI.
demo.mov