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.

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

Replace ios-deploy with node-ios-device #488

Closed
wants to merge 2 commits into from
Closed
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
1 change: 0 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ before_install:

install:
- npm install
- npm install ios-deploy
- npm install -g codecov

script:
Expand Down
3 changes: 1 addition & 2 deletions bin/templates/scripts/cordova/lib/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,8 @@ module.exports.run = function (buildOpts) {
// we also explicitly set device flag in options as we pass
// those parameters to other api (build as an example)
buildOpts.device = true;
return check_reqs.check_ios_deploy();
}
}).then(function () {

// CB-12287: Determine the device we should target when building for a simulator
if (!buildOpts.device) {
var newTarget = buildOpts.target || '';
Expand Down
17 changes: 1 addition & 16 deletions bin/templates/scripts/cordova/lib/check_reqs.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ const XCODEBUILD_MIN_VERSION = '9.0.0';
const XCODEBUILD_NOT_FOUND_MESSAGE =
'Please install version ' + XCODEBUILD_MIN_VERSION + ' or greater from App Store';

const IOS_DEPLOY_MIN_VERSION = '1.9.2';
const IOS_DEPLOY_NOT_FOUND_MESSAGE =
'Please download, build and install version ' + IOS_DEPLOY_MIN_VERSION + ' or greater' +
' from https://github.com/phonegap/ios-deploy into your path, or do \'npm install -g ios-deploy\'';

const COCOAPODS_MIN_VERSION = '1.0.1';
const COCOAPODS_NOT_FOUND_MESSAGE =
'Please install version ' + COCOAPODS_MIN_VERSION + ' or greater from https://cocoapods.org/';
Expand All @@ -53,14 +48,6 @@ module.exports.run = module.exports.check_xcodebuild = function () {
return checkTool('xcodebuild', XCODEBUILD_MIN_VERSION, XCODEBUILD_NOT_FOUND_MESSAGE);
};

/**
* Checks if ios-deploy util is available
* @return {Promise} Returns a promise either resolved with ios-deploy version or rejected
*/
module.exports.check_ios_deploy = function () {
return checkTool('ios-deploy', IOS_DEPLOY_MIN_VERSION, IOS_DEPLOY_NOT_FOUND_MESSAGE);
};

module.exports.check_os = function () {
// Build iOS apps available for OSX platform only, so we reject on others platforms
return os_platform_is_supported() ?
Expand Down Expand Up @@ -136,7 +123,7 @@ module.exports.check_cocoapods = function (toolChecker) {

/**
* Checks if specific tool is available.
* @param {String} tool Tool name to check. Known tools are 'xcodebuild' and 'ios-deploy'
* @param {String} tool Tool name to check. Known tools are 'xcodebuild'
* @param {Number} minVersion Min allowed tool version.
* @param {String} message Message that will be used to reject promise.
* @param {String} toolFriendlyName Friendly name of the tool, to report to the user. Optional.
Expand Down Expand Up @@ -187,7 +174,6 @@ module.exports.check_all = function () {
const requirements = [
new Requirement('os', 'Apple macOS', true),
new Requirement('xcode', 'Xcode'),
new Requirement('ios-deploy', 'ios-deploy'),
new Requirement('CocoaPods', 'CocoaPods')
];

Expand All @@ -197,7 +183,6 @@ module.exports.check_all = function () {
let checkFns = [
module.exports.check_os,
module.exports.check_xcodebuild,
module.exports.check_ios_deploy,
module.exports.check_cocoapods
];

Expand Down
36 changes: 26 additions & 10 deletions bin/templates/scripts/cordova/lib/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ var cp = require('child_process');
var build = require('./build');
var shell = require('shelljs');
var superspawn = require('cordova-common').superspawn;
var iosDevice = require('node-ios-device');
var check_reqs = require('./check_reqs');
var fs = require('fs-extra');

Expand Down Expand Up @@ -57,9 +58,8 @@ module.exports.run = function (runOptions) {
// we also explicitly set device flag in options as we pass
// those parameters to other api (build as an example)
runOptions.device = true;
return check_reqs.check_ios_deploy();
}
}).then(function () {

if (!runOptions.nobuild) {
return build.run(runOptions);
} else {
Expand Down Expand Up @@ -149,22 +149,38 @@ function filterSupportedArgs (args) {
* @return {Promise} Fullfilled when any device is connected, rejected otherwise
*/
function checkDeviceConnected () {
return superspawn.spawn('ios-deploy', ['-c', '-t', '1'], { printCommand: true, stdio: 'inherit' });
return Q.Promise(function (resolve, reject) {
return iosDevice.devices(function (err, devices) {
if (err || !devices.length) {
return reject(err || new Error('no device connected'));
}
return resolve(devices[0].udid);
});
});
}

/**
* Deploy specified app package to connected device
* using ios-deploy command
* using node-ios-device
* @param {String} appPath Path to application package
* @return {Promise} Resolves when deploy succeeds otherwise rejects
*/
function deployToDevice (appPath, target, extraArgs) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was extraArgs used for?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extraArgs contained the arguments not used by run.js.
It allowed to pass arguments to ios-deploy(ios-deploy usage).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so with your PR we loose this functionality. Which use case did it cover? What will not work any more for users that might be using this right now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think extraArgs is safe to ignore -- this would be any extra arguments to pass ios-deploy which we don't really officially support, but was "nice to have". If you do a cordova run --help you would see its the POPTS, which is not defined, thus we don't have any specific platform options.

We will have to remove any extraArgs code here as well:

var extraArgs = [];
if (runOptions.argv) {
// argv.slice(2) removes node and run.js, filterSupportedArgs removes the run.js args
extraArgs = module.exports.filterSupportedArgs(runOptions.argv.slice(2));
}
return module.exports.deployToDevice(appPath, runOptions.target, extraArgs);

function deployToDevice (appPath, target) {
// Deploying to device...
if (target) {
return superspawn.spawn('ios-deploy', ['--justlaunch', '-d', '-b', appPath, '-i', target].concat(extraArgs), { printCommand: true, stdio: 'inherit' });
} else {
return superspawn.spawn('ios-deploy', ['--justlaunch', '--no-wifi', '-d', '-b', appPath].concat(extraArgs), { printCommand: true, stdio: 'inherit' });
}

return Q.Promise((resolve) => {
if (target) {
return resolve(target);
}
return resolve(checkDeviceConnected());
}).then(device => Q.Promise((resolve, reject) => {
return iosDevice.installApp(device, appPath, err => {
if (err) {
return reject(err);
}
resolve();
});
}));
}

/**
Expand Down
22 changes: 2 additions & 20 deletions bin/templates/scripts/cordova/lib/versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,23 +91,6 @@ exports.get_apple_xcode_version = function () {
return d.promise;
};

/**
* Gets ios-deploy util version
* @return {Promise} Promise that either resolved with ios-deploy version
* or rejected in case of error
*/
exports.get_ios_deploy_version = function () {
var d = Q.defer();
child_process.exec('ios-deploy --version', function (error, stdout, stderr) {
if (error) {
d.reject(stderr);
} else {
d.resolve(stdout);
}
});
return d.promise;
};

/**
* Gets pod (CocoaPods) util version
* @return {Promise} Promise that either resolved with pod version
Expand Down Expand Up @@ -144,17 +127,16 @@ exports.get_ios_sim_version = function () {

/**
* Gets specific tool version
* @param {String} toolName Tool name to check. Known tools are 'xcodebuild', 'ios-sim' and 'ios-deploy'
* @param {String} toolName Tool name to check. Known tools are 'xcodebuild' and 'ios-sim'
* @return {Promise} Promise that either resolved with tool version
* or rejected in case of error
*/
exports.get_tool_version = function (toolName) {
switch (toolName) {
case 'xcodebuild': return exports.get_apple_xcode_version();
case 'ios-sim': return exports.get_ios_sim_version();
case 'ios-deploy': return exports.get_ios_deploy_version();
case 'pod': return exports.get_cocoapods_version();
default: return Q.reject(toolName + ' is not valid tool name. Valid names are: \'xcodebuild\', \'ios-sim\', \'ios-deploy\', and \'pod\'');
default: return Q.reject(toolName + ' is not valid tool name. Valid names are: \'xcodebuild\', \'ios-sim\', and \'pod\'');
}
};

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"dependencies": {
"cordova-common": "^3.0.0",
"ios-sim": "^6.1.2",
"node-ios-device": "^1.6.3",
"nopt": "^3.0.6",
"plist": "^1.2.0",
"q": "^1.4.1",
Expand Down
7 changes: 0 additions & 7 deletions tests/spec/component/versions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,6 @@ if (process.platform === 'darwin') {
}).catch(() => done.fail('expected promise resolution'));
});

it('should find ios-deploy version.', (done) => {
versions.get_tool_version('ios-deploy').then((version) => {
expect(version).not.toBe(undefined);
done();
}).catch(() => done.fail('expected promise resolution'));
});

it('should find pod version.', (done) => {
versions.get_tool_version('pod').then((version) => {
expect(version).not.toBe(undefined);
Expand Down