Skip to content

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

Merged
merged 1 commit into from
Aug 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@
"title": "Clean Build",
"category": "Swift"
},
{
"command": "swift.switchPlatform",
"title": "Switch Target",
"category": "Swift"
},
{
"command": "swift.resetPackage",
"title": "Reset Package Dependencies",
Expand Down Expand Up @@ -245,6 +250,10 @@
"command": "swift.cleanBuild",
"when": "swift.hasPackage"
},
{
"command": "swift.switchPlatform",
"when": "isMac"
},
{
"command": "swift.resetPackage",
"when": "swift.hasPackage"
Expand Down
33 changes: 33 additions & 0 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ import { FolderEvent, WorkspaceContext } from "./WorkspaceContext";
import { createSwiftTask, SwiftTaskProvider } from "./SwiftTaskProvider";
import { FolderContext } from "./FolderContext";
import { PackageNode } from "./ui/PackageDependencyProvider";
import { withQuickPick } from "./ui/QuickPick";
import { execSwift } from "./utilities/utilities";
import { Version } from "./utilities/version";
import { DarwinCompatibleTarget, SwiftToolchain } from "./toolchain/toolchain";
import configuration from "./configuration";

/**
* References:
Expand Down Expand Up @@ -428,6 +431,34 @@ function openInExternalEditor(packageNode: PackageNode) {
}
}

interface DarwinQuickPickTarget extends vscode.QuickPickItem {
value: DarwinCompatibleTarget;
label: string;
}

/**
* Switches the target SDK to the platform selected in a QuickPick UI.
*/
async function switchPlatform() {
const onSelect = async (picked: DarwinQuickPickTarget) => {
const sdkForTarget = await SwiftToolchain.getSdkForTarget(picked.value);
if (sdkForTarget) {
configuration.sdk = sdkForTarget;
} else {
vscode.window.showErrorMessage("Unable to obtain requested SDK path");
}
};

await withQuickPick<DarwinQuickPickTarget>(
"Select a new target",
[
{ value: DarwinCompatibleTarget.macOS, label: "macOS" },
{ value: DarwinCompatibleTarget.iOS, label: "iOS" },
],
onSelect
);
}

function updateAfterError(result: boolean, folderContext: FolderContext) {
const triggerResolvedUpdatedEvent = folderContext.hasResolveErrors;
// set has resolve errors flag
Expand All @@ -450,6 +481,8 @@ export function register(ctx: WorkspaceContext) {
),
vscode.commands.registerCommand("swift.updateDependencies", () => updateDependencies(ctx)),
vscode.commands.registerCommand("swift.cleanBuild", () => cleanBuild(ctx)),
// Note: This is only available on macOS (gated in `package.json`) because its the only OS that has the iOS SDK available.
vscode.commands.registerCommand("swift.switchPlatform", () => switchPlatform()),
vscode.commands.registerCommand("swift.resetPackage", () => resetPackage(ctx)),
vscode.commands.registerCommand("swift.runSingle", () => runSingleFile(ctx)),
vscode.commands.registerCommand("swift.openPackage", () => openPackage(ctx)),
Expand Down
3 changes: 3 additions & 0 deletions src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ const configuration = {
get sdk(): string {
return vscode.workspace.getConfiguration("swift").get<string>("SDK", "");
},
set sdk(value: string) {
vscode.workspace.getConfiguration("swift").update("SDK", value);
},
/** swift build arguments */
get buildArguments(): string[] {
return vscode.workspace.getConfiguration("swift").get<string[]>("buildArguments", []);
Expand Down
2 changes: 2 additions & 0 deletions src/sourcekit-lsp/LanguageClientManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
swiftDriverSDKFlags,
buildPathFlags,
swiftRuntimeEnv,
swiftDriverTargetFlags,
} from "../utilities/utilities";
import { Version } from "../utilities/version";
import { FolderEvent, WorkspaceContext } from "../WorkspaceContext";
Expand Down Expand Up @@ -345,6 +346,7 @@ export class LanguageClientManager {
serverPathConfig.length > 0 ? serverPathConfig : getSwiftExecutable("sourcekit-lsp");
const sdkArguments = [
...swiftDriverSDKFlags(true),
...swiftDriverTargetFlags(true),
...filterArguments(
configuration.buildArguments.concat(buildPathFlags()),
LanguageClientManager.buildArgumentFilter
Expand Down
37 changes: 35 additions & 2 deletions src/toolchain/toolchain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ interface SwiftTargetInfo {
[name: string]: string | object | undefined;
}

/**
* A Swift compilation target that can be compiled to
* from macOS. These are similar to XCode's target list.
*/
export enum DarwinCompatibleTarget {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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

Copy link
Contributor Author

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?

Copy link
Contributor

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"

iOS,
macOS,
}

export class SwiftToolchain {
constructor(
public swiftFolderPath: string,
Expand Down Expand Up @@ -195,8 +204,8 @@ export class SwiftToolchain {
if (process.env.SDKROOT) {
return process.env.SDKROOT;
}
const { stdout } = await execFile("xcrun", ["--sdk", "macosx", "--show-sdk-path"]);
return path.join(stdout.trimEnd());

return this.getSdkForTarget(DarwinCompatibleTarget.macOS);
}
case "win32": {
return process.env.SDKROOT;
Expand All @@ -205,6 +214,30 @@ export class SwiftToolchain {
return undefined;
}

/**
* @param target Target to obtain the SDK path for
* @returns path to the SDK for the target
*/
public static async getSdkForTarget(
target: DarwinCompatibleTarget
): Promise<string | undefined> {
let sdkType: string;
switch (target) {
case DarwinCompatibleTarget.macOS:
sdkType = "macosx";
break;
case DarwinCompatibleTarget.iOS:
sdkType = "iphoneos";
break;
}

// Include custom variables so that non-standard XCode installs can be better supported.
const { stdout } = await execFile("xcrun", ["--sdk", sdkType, "--show-sdk-path"], {
env: { ...process.env, ...configuration.swiftEnvironmentVariables },
});
return path.join(stdout.trimEnd());
}

/**
* @returns path to custom SDK
*/
Expand Down
46 changes: 46 additions & 0 deletions src/ui/QuickPick.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the VSCode Swift open source project
//
// Copyright (c) 2022 the VSCode Swift project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of VSCode Swift project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

import * as vscode from "vscode";

/**
* Displays a QuickPick UI with the parameters and passes the result
* to the provided closure.
*/
export async function withQuickPick<T extends vscode.QuickPickItem>(
placeholder: string,
options: T[],
onSelect: (picked: T) => Promise<void>
) {
const picker = vscode.window.createQuickPick<T>();

picker.placeholder = placeholder;
picker.items = options;

picker.show();

const pickedItem = await new Promise<T | undefined>(resolve => {
picker.onDidAccept(() => resolve(picker.selectedItems[0]));
picker.onDidHide(() => resolve(undefined));
});

picker.busy = true;

if (pickedItem) {
await onSelect(pickedItem);
picker.busy = false;
}

picker.dispose();
}
30 changes: 30 additions & 0 deletions src/utilities/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,36 @@ export function swiftDriverSDKFlags(indirect = false): string[] {
return indirect ? args.flatMap(arg => ["-Xswiftc", arg]) : args;
}

/**
* Get target flags for swiftc
*
* @param indirect whether to pass the flags by -Xswiftc
*/
export function swiftDriverTargetFlags(indirect = false): string[] {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

@adam-fowler adam-fowler Aug 6, 2022

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

Copy link
Contributor

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

const IPHONE_SDK_KIND = "iPhoneOS";

let args: string[] = [];

if (configuration.sdk === "") {
return args;
}

const sdkKindParts = configuration.sdk.split("/");
const sdkKind = sdkKindParts[sdkKindParts.length - 1];
if (sdkKind.includes(IPHONE_SDK_KIND)) {
// Obtain the iOS version of the SDK.
const version = sdkKind.substring(
// Trim the prefix
sdkKind.length - IPHONE_SDK_KIND.length,
// Trim the `.sdk` suffix
sdkKind.length - 4
);
args = ["-target", `arm64-apple-ios${version}`];
}

return indirect ? args.flatMap(arg => ["-Xswiftc", arg]) : args;
}

/**
* Get the file name of executable
*
Expand Down