You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by robpaveza <gi...@git.apache.org> on 2015/08/12 23:33:58 UTC
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
GitHub user robpaveza opened a pull request:
https://github.com/apache/cordova-windows/pull/113
CB-9482: Upon further investigation of CI failures, we saw that
AppDeployCmd also failed to install an app when:
- The emulator had been off previously
- It had been started with the AppDeployCmd /uninstall command
- The next command was to then install an app
This change detects the condition (only for emulator targets) and retries
once. This appears to correct the issue.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-windows CB-9482
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-windows/pull/113.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #113
----
commit 2edc0031dc6793fab9db81c7de03d1cae5a846d7
Author: Rob Paveza <ro...@microsoft.com>
Date: 2015-08-12T21:30:50Z
CB-9482: Upon further investigation of CI failures, we saw that
AppDeployCmd also failed to install an app when:
- The emulator had been off previously
- It had been started with the AppDeployCmd /uninstall command
- The next command was to then install an app
This change detects the condition (only for emulator targets) and retries
once. This appears to correct the issue.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
Posted by robpaveza <gi...@git.apache.org>.
Github user robpaveza commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/113#discussion_r36925808
--- Diff: template/cordova/lib/deployment.js ---
@@ -194,13 +194,28 @@ AppDeployCmdTool.prototype.enumerateDevices = function() {
});
};
+// Note: To account for CB-9482, we pass an extra parameter when retrying the call. Be forwarned to check for that
+// if additional parameters are added in the future.
AppDeployCmdTool.prototype.installAppPackage = function(pathToAppxPackage, targetDevice, shouldLaunch, shouldUpdate, pin) {
var command = shouldUpdate ? '/update' : '/install';
if (shouldLaunch) {
command += 'launch';
}
- return run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ var that = this;
+ var result = run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ if (targetDevice.type === 'emulator' && arguments.length < 6) {
--- End diff --
It's called out in the comment immediately preceding the function definition. There are 5 formal arguments specified to the function, so when the function is called normally, arguments.length = 5. In that case, we compose an error handler onto result; the composed error handler is only called if AppDeployCmd fails. That handler checks the code to see if it's `E_INVALIDARG`, and if so, re-calls the function, this time passing an additional parameter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
Posted by robpaveza <gi...@git.apache.org>.
Github user robpaveza commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/113#discussion_r36925416
--- Diff: template/cordova/lib/deployment.js ---
@@ -35,7 +35,7 @@ function run(cmd, args, opt_cwd) {
child.stderr.on('data', function(s) { stderr += s; });
child.on('exit', function(code) {
if (code) {
- d.reject(stderr);
+ d.reject({ message: stderr, code: code, toString: function() { return stderr; }});
--- End diff --
This isn't logging. This is providing an object instead of a string to the failure case of a promise.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/113#discussion_r37346680
--- Diff: template/cordova/lib/deployment.js ---
@@ -35,7 +35,7 @@ function run(cmd, args, opt_cwd) {
child.stderr.on('data', function(s) { stderr += s; });
child.on('exit', function(code) {
if (code) {
- d.reject(stderr);
+ d.reject({ message: stderr, code: code, toString: function() { return stderr; }});
--- End diff --
Whoops, I totally read `d.reject()` as `console.log`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
Posted by robpaveza <gi...@git.apache.org>.
Github user robpaveza commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/113#discussion_r36925333
--- Diff: template/cordova/lib/deployment.js ---
@@ -194,13 +194,28 @@ AppDeployCmdTool.prototype.enumerateDevices = function() {
});
};
+// Note: To account for CB-9482, we pass an extra parameter when retrying the call. Be forwarned to check for that
+// if additional parameters are added in the future.
AppDeployCmdTool.prototype.installAppPackage = function(pathToAppxPackage, targetDevice, shouldLaunch, shouldUpdate, pin) {
var command = shouldUpdate ? '/update' : '/install';
if (shouldLaunch) {
command += 'launch';
}
- return run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ var that = this;
+ var result = run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ if (targetDevice.type === 'emulator' && arguments.length < 6) {
+ result = result.then(null, function(e) {
+ // CB-9482: AppDeployCmd also reports E_INVALIDARG during this process. If so, try to repeat.
+ if (e.code === 2147942487) {
--- End diff --
OK, I'll do so in the updated PR, but I don't see that `var E_INVALIDARG = ...; if (e.code === E_INVALIDARG)` is necessarily better.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
Posted by robpaveza <gi...@git.apache.org>.
Github user robpaveza commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/113#discussion_r36926008
--- Diff: template/cordova/lib/deployment.js ---
@@ -194,13 +194,28 @@ AppDeployCmdTool.prototype.enumerateDevices = function() {
});
};
+// Note: To account for CB-9482, we pass an extra parameter when retrying the call. Be forwarned to check for that
+// if additional parameters are added in the future.
AppDeployCmdTool.prototype.installAppPackage = function(pathToAppxPackage, targetDevice, shouldLaunch, shouldUpdate, pin) {
var command = shouldUpdate ? '/update' : '/install';
if (shouldLaunch) {
command += 'launch';
}
- return run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ var that = this;
+ var result = run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ if (targetDevice.type === 'emulator' && arguments.length < 6) {
--- End diff --
In the update, I'll just re-call run().
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/113#discussion_r36921448
--- Diff: template/cordova/lib/deployment.js ---
@@ -35,7 +35,7 @@ function run(cmd, args, opt_cwd) {
child.stderr.on('data', function(s) { stderr += s; });
child.on('exit', function(code) {
if (code) {
- d.reject(stderr);
+ d.reject({ message: stderr, code: code, toString: function() { return stderr; }});
--- End diff --
Is this a better way to log, or was it only used in debugging?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-windows/pull/113
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/113#discussion_r36921320
--- Diff: template/cordova/lib/deployment.js ---
@@ -194,13 +194,28 @@ AppDeployCmdTool.prototype.enumerateDevices = function() {
});
};
+// Note: To account for CB-9482, we pass an extra parameter when retrying the call. Be forwarned to check for that
+// if additional parameters are added in the future.
AppDeployCmdTool.prototype.installAppPackage = function(pathToAppxPackage, targetDevice, shouldLaunch, shouldUpdate, pin) {
var command = shouldUpdate ? '/update' : '/install';
if (shouldLaunch) {
command += 'launch';
}
- return run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ var that = this;
+ var result = run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ if (targetDevice.type === 'emulator' && arguments.length < 6) {
+ result = result.then(null, function(e) {
+ // CB-9482: AppDeployCmd also reports E_INVALIDARG during this process. If so, try to repeat.
+ if (e.code === 2147942487) {
--- End diff --
Nitpick: Please define this as a constant.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-9482: Upon further investigation ...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/113#discussion_r36921360
--- Diff: template/cordova/lib/deployment.js ---
@@ -194,13 +194,28 @@ AppDeployCmdTool.prototype.enumerateDevices = function() {
});
};
+// Note: To account for CB-9482, we pass an extra parameter when retrying the call. Be forwarned to check for that
+// if additional parameters are added in the future.
AppDeployCmdTool.prototype.installAppPackage = function(pathToAppxPackage, targetDevice, shouldLaunch, shouldUpdate, pin) {
var command = shouldUpdate ? '/update' : '/install';
if (shouldLaunch) {
command += 'launch';
}
- return run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ var that = this;
+ var result = run(this.path, [command, pathToAppxPackage, '/targetdevice:' + targetDevice.__shorthand]);
+ if (targetDevice.type === 'emulator' && arguments.length < 6) {
--- End diff --
What's special about `arguments.length` and 6?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org