dpogue commented on code in PR #1596:
URL: https://github.com/apache/cordova-ios/pull/1596#discussion_r2645172311
##########
lib/build.js:
##########
@@ -327,7 +334,7 @@ module.exports.run = function (buildOpts) {
* @param {Object} buildConfig The build configuration options
* @return {Array} Array of arguments that could be passed
directly to spawn method
*/
-function getXcodeBuildArgs (projectPath, configuration, emulatorTarget,
buildConfig = {}) {
+function getXcodeBuildArgs (projectPath, configuration, emulatorTarget,
buildConfig = {}, targetRuntime = null) {
Review Comment:
The doc comment block should be updated to include the new parameter
##########
lib/build.js:
##########
@@ -154,6 +154,12 @@ module.exports.run = function (buildOpts) {
}
const projectPath = this.root;
+ // When target includes a runtime (e.g., "iPhone-16-Pro, 18.5"), capture
it for xcodebuild destination
+ const targetRuntime = (function extractRuntime (target) {
+ if (!target || !target.includes(',')) return null;
+ const parts = target.split(',');
+ return (parts[1] || '').trim() || null;
+ })(buildOpts.target);
Review Comment:
You can probably simplify this with conditional chaining to avoid the need
for an IIFE:
```suggestion
const targetRuntime = buildOpts.target?.split(',')?.at(1)?.trim();
```
##########
lib/build.js:
##########
@@ -394,9 +401,10 @@ function getXcodeBuildArgs (projectPath, configuration,
emulatorTarget, buildCon
'-destination', customArgs.destination ||
'generic/platform=macOS,variant=Mac Catalyst'
]);
} else {
+ const osPart = targetRuntime ? `,OS=${targetRuntime}` : '';
Review Comment:
`version` might be a better name than `osPart` for this variable
##########
lib/simctlHelper.js:
##########
@@ -229,16 +229,40 @@ function getDeviceFromDeviceTypeId (deviceTypeId = null) {
runtimeRaw = null
] = deviceTypeId?.split(',') ?? [];
+ // Extract the segment after the SimDeviceType prefix (if present)
+ const prefix = 'com.apple.CoreSimulator.SimDeviceType.';
+ const rawWithoutPrefix = (deviceTypeRaw ||
'').trim().replace(/^com\.apple\.CoreSimulator\.SimDeviceType\./, '');
+
+ // If the target is a Simulator UDID (UUID), support selecting by UDID
directly
Review Comment:
Currently, the intention is for the `--target` option to be specified in the
same format that you get from `--list`. Providing a UDID is not (and never has
been) supported by Cordova tooling.
I'm not opposed to supporting it, but this probably needs a bit more
thinking to make sure that the same support works for deploying to a device and
not just to a simulator. We have some other work to do around device
deployments anyways, so this might be something that's better to revisit in the
future rather than partially supporting it now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
