Skip to content
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.

Sign up for GitHub

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

fix/choosing_apple_device #1753

Merged
merged 3 commits into from
Oct 17, 2024
Merged

fix/choosing_apple_device #1753

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d89c2c
fix/choosing_apple_device
ElenaDiachenko Oct 9, 2024
c4fa6b0
fix
ElenaDiachenko Oct 12, 2024
e109320
fix
ElenaDiachenko Oct 16, 2024
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
152 changes: 97 additions & 55 deletions packages/sdk-apple/src/runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import child_process, { ExecFileOptions } from 'child_process';
import _ from 'lodash';
import path from 'path';
import {
fsExistsSync,
Expand All @@ -22,6 +23,7 @@
logRaw,
inquirerPrompt,
CoreEnvVars,
logInfo,
} from '@rnv/core';
import { getAppleDevices } from './deviceManager';

Expand Down Expand Up @@ -69,20 +71,33 @@

if (!c.platform) return;

const { device } = c.program.opts();
let devicesArr: AppleDevice[] = [];
if (device === true) {
devicesArr = await getAppleDevices(false, true);
} else {
devicesArr = await getAppleDevices(true, false);
}
const { device, target } = c.program.opts();
ElenaDiachenko marked this conversation as resolved.
Show resolved Hide resolved
const devicesArr = await getAppleDevices(false, !!device);
const run = (selectedDevice: AppleDevice) => {
logDebug(`Selected device: ${JSON.stringify(selectedDevice, null, 3)}`);
c.runtime.targetUDID = selectedDevice.udid;
if (selectedDevice.udid) {
p = `--udid ${selectedDevice.udid}`;
} else {
p = `--device ${selectedDevice.name}`;
}

return p;
};

let p;

if (device === true) {
if (devicesArr.length === 1) {
const { isValidTarget, isValidDevice } = await _getAvailableTarget({
target,
device,
devicesArr,
});

if (device || isValidTarget) {
const availableDevices = devicesArr.filter((d) => d.isDevice);
if (availableDevices.length === 1 && ((device && !target && isValidDevice) || isValidTarget)) {
logSuccess(
`Found one device connected! Device name: ${chalk().bold.white(
`Found device connected! Device name: ${chalk().bold.white(
devicesArr[0].name
)} udid: ${chalk().bold.white(devicesArr[0].udid)}`
);
Expand All @@ -92,19 +107,7 @@
} else {
p = `--device ${devicesArr[0].name}`;
}
} else if (devicesArr.length > 1) {
const run = (selectedDevice: AppleDevice) => {
logDebug(`Selected device: ${JSON.stringify(selectedDevice, null, 3)}`);
c.runtime.targetUDID = selectedDevice.udid;
if (selectedDevice.udid) {
p = `--udid ${selectedDevice.udid}`;
} else {
p = `--device ${selectedDevice.name}`;
}

return p;
};

} else if (availableDevices.length) {
if (c.runtime.target) {
const selectedDevice = devicesArr.find((d) => d.name === c.runtime.target);
if (selectedDevice) {
Expand All @@ -113,7 +116,7 @@
logWarning(`Could not find device ${c.runtime.target}`);
}

const devices = devicesArr.map((v) => ({
const devices = availableDevices.map((v) => ({
name: `${v.name} | ${v.icon} | v: ${chalk().green(v.version)} | udid: ${chalk().grey(v.udid)}${
v.isDevice ? chalk().red(' (device)') : ''
}`,
Expand All @@ -133,8 +136,6 @@
} else {
return Promise.reject(`No ${c.platform} devices connected!`);
}
} else if (device) {
p = `--device ${device}`;
} else if (c.runtime.isTargetTrue) {
const devices = devicesArr.map((v) => ({
name: `${v.name} | ${v.icon} | v: ${chalk().green(v.version)} | udid: ${chalk().grey(v.udid)}${
Expand All @@ -143,38 +144,38 @@
value: v,
}));

const { sim } = await inquirerPrompt({
name: 'sim',
message: 'Select the simulator you want to launch on',
const { chosenTarget } = await inquirerPrompt({
name: 'chosenTarget',
message: 'Select the simulator or device you want to launch on',
type: 'list',
choices: devices,
});

c.runtime.target = sim.name;
if (c.runtime.target) {
p = `--simulator ${c.runtime.target.replace(/(\s+)/g, '\\$1')}`;
if (!chosenTarget.isDevice) {
c.runtime.target = chosenTarget.name;
if (c.runtime.target) {
p = `--simulator ${c.runtime.target.replace(/(\s+)/g, '\\$1')}`;
}
} else {
return run(chosenTarget);
}
} else if (c.runtime.target || devicesArr.length > 0) {
// check if the default sim is available
const desiredSim = devicesArr.find((d) => d.name === c.runtime.target && !d.isDevice);
let desiredSim = devicesArr.find((d) => d.name === c.runtime.target);

if (!desiredSim) {
const { sim } = await inquirerPrompt({
name: 'sim',
const { currentTarget } = await inquirerPrompt({
name: 'currentTarget',
message: !c.runtime.target
? `No global or project default simulator defined. Please select a supported simulator to use`
: `We couldn't find ${c.runtime.target} as a simulator supported by the current version of your Xcode. Please select another sim`,
? `No global or project default simulator or device defined. Please select a supported simulator to use`
: `We couldn't find ${c.runtime.target} as a target supported by the current version of your Xcode or as a connected device. Please select another simulator or device`,
type: 'list',
choices: devicesArr
.filter((d) => !d.isDevice)
.map((v) => ({
name: `${v.name} | ${v.icon} | v: ${chalk().green(v.version)} | udid: ${chalk().grey(v.udid)}${
v.isDevice ? chalk().red(' (device)') : ''
}`,
value: v,
})),
choices: devicesArr.map((v) => ({
name: `${v.name} | ${v.icon} | v: ${chalk().green(v.version)} | udid: ${chalk().grey(v.udid)}${
v.isDevice ? chalk().red(' (device)') : ''
}`,
value: v,
})),
});

desiredSim = currentTarget;
const localOverridden = !!c.files.project.configLocal?.defaultTargets?.[c.platform];

const actionLocalUpdate = `Update ${chalk().green('project')} default target for platform ${c.platform}`;
Expand All @@ -188,15 +189,15 @@
type: 'list',
name: 'chosenAction',
choices: [actionLocalUpdate, actionGlobalUpdate, actionNoUpdate],
warningMessage: `Your default target for platform ${c.platform} is set to ${c.runtime.target}. This seems to not be supported by Xcode anymore`,
warningMessage: `Your default target for platform ${c.platform} is set to ${c.runtime.target}.`,
});

c.runtime.target = sim.name;
c.runtime.target = currentTarget.name;

if (chosenAction === actionLocalUpdate || (chosenAction === actionGlobalUpdate && localOverridden)) {
const configLocal = c.files.project.configLocal || {};
if (!configLocal.defaultTargets) configLocal.defaultTargets = {};
configLocal.defaultTargets[c.platform] = sim.name;
configLocal.defaultTargets[c.platform] = currentTarget.name;

c.files.project.configLocal = configLocal;
writeFileSync(c.paths.project.configLocal, configLocal);
Expand All @@ -206,22 +207,63 @@
const configGlobal = c.files.workspace.config;
if (configGlobal) {
if (!configGlobal.defaultTargets) configGlobal.defaultTargets = {};
configGlobal.defaultTargets[c.platform] = sim.name;
configGlobal.defaultTargets[c.platform] = currentTarget.name;

c.files.workspace.config = configGlobal;
writeFileSync(c.paths.workspace.config, configGlobal);
}
}
}
if (!desiredSim?.isDevice) {
const target = c.runtime.target?.replace(/(\s+)/g, '\\$1');

const target = c.runtime.target?.replace(/(\s+)/g, '\\$1');

p = `--simulator ${target}`;
p = `--simulator ${target}`;

Check warning

Code scanning / CodeQL

Unsafe shell command constructed from library input Medium

This string concatenation which depends on
library input
Loading
is later used in a
shell command
Loading
.

Copilot Autofix AI 2 months ago

To fix the problem, we should avoid using string concatenation to construct the shell command. Instead, we can use child_process.execFile to safely pass the arguments to the command without risking shell injection. This approach ensures that the input is treated as an argument rather than part of the command string.

Suggested changeset 1
packages/sdk-apple/src/runner.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/sdk-apple/src/runner.ts b/packages/sdk-apple/src/runner.ts
--- a/packages/sdk-apple/src/runner.ts
+++ b/packages/sdk-apple/src/runner.ts
@@ -79,5 +79,5 @@
         if (selectedDevice.udid) {
-            p = `--udid ${selectedDevice.udid}`;
+            p = ['--udid', selectedDevice.udid];
         } else {
-            p = `--device ${selectedDevice.name}`;
+            p = ['--device', selectedDevice.name];
         }
@@ -217,5 +217,5 @@
         if (!desiredSim?.isDevice) {
-            const target = c.runtime.target?.replace(/(\s+)/g, '\\$1');
+            const target = c.runtime.target;
 
-            p = `--simulator ${target}`;
+            p = ['--simulator', target];
         } else {
EOF
@@ -79,5 +79,5 @@
if (selectedDevice.udid) {
p = `--udid ${selectedDevice.udid}`;
p = ['--udid', selectedDevice.udid];
} else {
p = `--device ${selectedDevice.name}`;
p = ['--device', selectedDevice.name];
}
@@ -217,5 +217,5 @@
if (!desiredSim?.isDevice) {
const target = c.runtime.target?.replace(/(\s+)/g, '\\$1');
const target = c.runtime.target;

p = `--simulator ${target}`;
p = ['--simulator', target];
} else {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
} else {
return run(desiredSim);
}
}

return p;
};

const _getAvailableTarget = async ({
target,
device,
devicesArr,
}: {
target: string | undefined | boolean;
device: string | undefined | boolean;
devicesArr: AppleDevice[];
}) => {
let isValidTarget = false;
let isValidDevice = false;

if (_.isString(target)) {
const targetMatch = devicesArr.filter((d) => d.isDevice).find((d) => d.name === target);
isValidTarget = !!targetMatch || (await _isValidIP(target));
}
if (_.isString(device)) {
const deviceMatch = devicesArr.find((d) => d.isDevice && d.name === device);
isValidDevice = !!deviceMatch || (await _isValidIP(device));
}
if (device === true) {
isValidDevice = true;
}

return { isValidTarget, isValidDevice };
};

const _isValidIP = async (ip: string) => {
try {
await executeAsync(`ping -c 1 ${ip}`, { silent: true });
logInfo(`Target IP ${ip} is a valid IP address`);
return true;
} catch (error) {
logInfo(`Target IP ${ip} is not a valid IP address`);
return false;
}
};

export const runXcodeProject = async (runDeviceArguments?: string) => {
const c = getContext();
logDefault('runXcodeProject', `targetArgs:${runDeviceArguments}`);
Expand Down
Loading