You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by vladimir-kotikov <gi...@git.apache.org> on 2015/11/11 14:36:00 UTC
[GitHub] cordova-android pull request: CB-9971 Redirect gradlew stderr to c...
GitHub user vladimir-kotikov opened a pull request:
https://github.com/apache/cordova-android/pull/240
CB-9971 Redirect gradlew stderr to current process stdout
This is a fix for https://issues.apache.org/jira/browse/CB-9971
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-android CB-9971
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-android/pull/240.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 #240
----
commit 0c4f17de1da462e7d8677412bd99255f1e645eb0
Author: Vladimir Kotikov <ko...@gmail.com>
Date: 2015-11-11T13:10:12Z
CB-9971 Redirect gradlew stderr to current process stdout
----
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r45385875
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +212,54 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
+ * with java printing some necessary information to stderr instead of stdout.
+ * This function redirects all stderr messages to current process stdout. See
+ * https://issues.apache.org/jira/browse/CB-9971 for explanation.
+ *
+ * @param {String} cmd A command to spawn
+ * @param {String[]} args Command arguments. Note that on Windows arguments
+ * will be concatenated into string and passed to 'cmd.exe' along with '/s'
+ * and '/c' swithces for proper space-in-path handling
+ *
+ * @return {Promise} A promise, rejected with error message, if
+ * underlying command exits with nonzero exit code, fulfilled otherwise
+ */
+function spawnWithStderrRedirect(cmd, args) {
+ var opts = { stdio: 'pipe' };
+
+ if (process.platform === 'win32') {
+ // Work around spawn not being able to find .bat files.
+ var joinedArgs = [cmd]
+ .concat(args)
+ .map(function(a){
+ // Add quotes to arguments which contains whitespaces
+ if (/^[^"].* .*[^"]/.test(a)) return '"' + a + '"';
+ return a;
+ }).join(' ');
+
+ args = ['/s', '/c'].concat('"' + joinedArgs + '"');
+ cmd = 'cmd';
+ opts.windowsVerbatimArguments = true;
+ }
+
+ return Q.Promise(function (resolve, reject) {
+ var proc = child_process.spawn(cmd, args, opts);
+
+ proc.stdout.on('data', process.stdout.write.bind(process.stdout));
+ proc.stderr.on('data', function (data) {
+ var isRedirectNeeded = data.toString().match(/^Picked up _JAVA_OPTIONS: /i);
--- End diff --
/regex/.test(str) is faster when all you need is a boolean result, not the detailed info that match() returns.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r45386856
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +212,54 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
+ * with java printing some necessary information to stderr instead of stdout.
+ * This function redirects all stderr messages to current process stdout. See
+ * https://issues.apache.org/jira/browse/CB-9971 for explanation.
+ *
+ * @param {String} cmd A command to spawn
+ * @param {String[]} args Command arguments. Note that on Windows arguments
+ * will be concatenated into string and passed to 'cmd.exe' along with '/s'
+ * and '/c' swithces for proper space-in-path handling
+ *
+ * @return {Promise} A promise, rejected with error message, if
+ * underlying command exits with nonzero exit code, fulfilled otherwise
+ */
+function spawnWithStderrRedirect(cmd, args) {
+ var opts = { stdio: 'pipe' };
+
+ if (process.platform === 'win32') {
+ // Work around spawn not being able to find .bat files.
+ var joinedArgs = [cmd]
+ .concat(args)
+ .map(function(a){
+ // Add quotes to arguments which contains whitespaces
+ if (/^[^"].* .*[^"]/.test(a)) return '"' + a + '"';
+ return a;
+ }).join(' ');
+
+ args = ['/s', '/c'].concat('"' + joinedArgs + '"');
+ cmd = 'cmd';
+ opts.windowsVerbatimArguments = true;
+ }
+
+ return Q.Promise(function (resolve, reject) {
+ var proc = child_process.spawn(cmd, args, opts);
+
+ proc.stdout.on('data', process.stdout.write.bind(process.stdout));
+ proc.stderr.on('data', function (data) {
+ var isRedirectNeeded = data.toString().match(/^Picked up _JAVA_OPTIONS: /i);
+ process[isRedirectNeeded ? 'stdout' : 'stderr'].write(data);
--- End diff --
Are we sure that putting that line on stdout will not cause different problems? Would it be better to completely suppress it? I don't think anyone actually cares to see it.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-android/pull/240
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-android/pull/240#issuecomment-158453018
.It does, right, but there is two problems:
1. Superspawn returns output only _after_ the underlying process exits - if the build lasts long enough, this might be confusing for the user,
2. Superspawn either resolves with stdout or rejects with some error message that contains stderr output - there is no way to get both stdout and stderr after the promise completed
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r44595632
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +209,46 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
+ * with java printing some necessary information to stderr instead of stdout.
+ * This function redirects all stderr messages to current process stdout. See
+ * https://issues.apache.org/jira/browse/CB-9971 for explanation.
+ *
+ * @param {String} cmd A command to spawn
+ * @param {String[]} args Command arguments. Note that on Windows arguments
+ * will be concatenated into string and passed to 'cmd.exe' along with '/s'
+ * and '/c' swithces for proper space-in-path handling
+ *
+ * @return {Promise} A promise, rejected with error message, if
+ * underlying command exits with nonzero exit code, fulfilled otherwise
+ */
+function spawnWithStderrRedirect(cmd, args) {
+ return Q.Promise(function (resolve, reject) {
+ // Work around spawn not being able to find .bat files.
+ var opts = { stdio: [0,1,1] };
+ if (process.platform === 'win32') {
+ var joinedArgs = [cmd]
+ .concat(args)
+ .map(function(a){
+ if (/^[^"].* .*[^"]/.test(a)) return '"' + a + '"';
--- End diff --
Fair enough. However if `superspawn` code has a bug and is updated, then this code will be out of date. It's better not to duplicate code and instead modify and reuse `superspawn`.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on the pull request:
https://github.com/apache/cordova-android/pull/240#issuecomment-155854701
Does java.exe output compilation errors and warnings to stderr or stdout? If they go to stderr then we should NOT redirect those.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-android/pull/240#issuecomment-155912830
@jasongin, i will double check this
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-android/pull/240#issuecomment-155867938
Also :+1: on what @jasongin said.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r44586855
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +209,46 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
+ * with java printing some necessary information to stderr instead of stdout.
+ * This function redirects all stderr messages to current process stdout. See
+ * https://issues.apache.org/jira/browse/CB-9971 for explanation.
+ *
+ * @param {String} cmd A command to spawn
+ * @param {String[]} args Command arguments. Note that on Windows arguments
+ * will be concatenated into string and passed to 'cmd.exe' along with '/s'
+ * and '/c' swithces for proper space-in-path handling
+ *
+ * @return {Promise} A promise, rejected with error message, if
+ * underlying command exits with nonzero exit code, fulfilled otherwise
+ */
+function spawnWithStderrRedirect(cmd, args) {
+ return Q.Promise(function (resolve, reject) {
+ // Work around spawn not being able to find .bat files.
+ var opts = { stdio: [0,1,1] };
+ if (process.platform === 'win32') {
+ var joinedArgs = [cmd]
+ .concat(args)
+ .map(function(a){
+ if (/^[^"].* .*[^"]/.test(a)) return '"' + a + '"';
--- End diff --
This escaping works exactly the same way in `superspawn` and we hadn't had any problems with whitespaces, caused by this implementation, for a long time, so i'd prefer just not to change it.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-android/pull/240#issuecomment-158513479
I'm pretty sure we can modify this line: https://github.com/MarcDiethelm/superspawn/blob/master/lib/index.js#L166 to return both the stdout and stderr. To me it seems like writing a whole new function is going to result in maintenance bugs in the future.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-android/pull/240#issuecomment-158613976
I think current implementation is good enough so let's merge this PR.
:+1: to the idea of improving superspawn function so it can cover this case as well, but I think this is out of scope of this PR so let's continue discussion at CB-10052.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r44566005
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +209,46 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
+ * with java printing some necessary information to stderr instead of stdout.
+ * This function redirects all stderr messages to current process stdout. See
+ * https://issues.apache.org/jira/browse/CB-9971 for explanation.
+ *
+ * @param {String} cmd A command to spawn
+ * @param {String[]} args Command arguments. Note that on Windows arguments
+ * will be concatenated into string and passed to 'cmd.exe' along with '/s'
+ * and '/c' swithces for proper space-in-path handling
+ *
+ * @return {Promise} A promise, rejected with error message, if
+ * underlying command exits with nonzero exit code, fulfilled otherwise
+ */
+function spawnWithStderrRedirect(cmd, args) {
+ return Q.Promise(function (resolve, reject) {
+ // Work around spawn not being able to find .bat files.
+ var opts = { stdio: [0,1,1] };
+ if (process.platform === 'win32') {
+ var joinedArgs = [cmd]
+ .concat(args)
+ .map(function(a){
+ if (/^[^"].* .*[^"]/.test(a)) return '"' + a + '"';
--- End diff --
What if `a` contains `"` characters? For example, if `a` is `program" files "folder`? Is there a cross-platform shell-escaping library we can use instead?
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r44585568
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +209,46 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
--- End diff --
Not even an additional option, but an ability to accept any valid `stdio` option - `superspawn` now knows only `default`, `ignore` and `inherit`. I agree, this will be the most correct solution, but will require patching this in cordova-common and releasing it before next android release.
@jasongin, what do you think?
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r44566085
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +209,46 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
+ * with java printing some necessary information to stderr instead of stdout.
+ * This function redirects all stderr messages to current process stdout. See
+ * https://issues.apache.org/jira/browse/CB-9971 for explanation.
+ *
+ * @param {String} cmd A command to spawn
+ * @param {String[]} args Command arguments. Note that on Windows arguments
+ * will be concatenated into string and passed to 'cmd.exe' along with '/s'
+ * and '/c' swithces for proper space-in-path handling
+ *
+ * @return {Promise} A promise, rejected with error message, if
+ * underlying command exits with nonzero exit code, fulfilled otherwise
+ */
+function spawnWithStderrRedirect(cmd, args) {
+ return Q.Promise(function (resolve, reject) {
+ // Work around spawn not being able to find .bat files.
+ var opts = { stdio: [0,1,1] };
+ if (process.platform === 'win32') {
+ var joinedArgs = [cmd]
+ .concat(args)
+ .map(function(a){
+ if (/^[^"].* .*[^"]/.test(a)) return '"' + a + '"';
+ return a;
+ })
+ .join(' ');
+
+ args = ['/s', '/c'].concat('"' + joinedArgs + '"')/*, '"' + [cmd].concat(args).map(function(a){if (/^[^"].* .*[^"]/.test(a)) return '"' + a + '"'; return a;}).join(' ')+'"']*/;
--- End diff --
Is the comment at the end of the line necessary?
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-android/pull/240#issuecomment-158246192
Can't we still use superspawn and just add something like the `stderr.on('data')` filter to catch and discard the undesired line? Superspawn can return a promise that receives the stdout and stderr of the spawned command.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r44614185
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +209,46 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
--- End diff --
It looks like `superspawn` should be modified to take the same parameters as `child_process.spawn`. Refer [here](https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options) for Node 5.0.x, and [here](https://nodejs.org/docs/latest-v0.12.x/api/child_process.html#child_process_child_process_spawn_command_args_options) for Node 0.12.x.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r44566177
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +209,46 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
+ * with java printing some necessary information to stderr instead of stdout.
+ * This function redirects all stderr messages to current process stdout. See
+ * https://issues.apache.org/jira/browse/CB-9971 for explanation.
+ *
+ * @param {String} cmd A command to spawn
+ * @param {String[]} args Command arguments. Note that on Windows arguments
+ * will be concatenated into string and passed to 'cmd.exe' along with '/s'
+ * and '/c' swithces for proper space-in-path handling
+ *
+ * @return {Promise} A promise, rejected with error message, if
+ * underlying command exits with nonzero exit code, fulfilled otherwise
+ */
+function spawnWithStderrRedirect(cmd, args) {
+ return Q.Promise(function (resolve, reject) {
+ // Work around spawn not being able to find .bat files.
+ var opts = { stdio: [0,1,1] };
+ if (process.platform === 'win32') {
+ var joinedArgs = [cmd]
+ .concat(args)
+ .map(function(a){
+ if (/^[^"].* .*[^"]/.test(a)) return '"' + a + '"';
+ return a;
+ })
+ .join(' ');
+
+ args = ['/s', '/c'].concat('"' + joinedArgs + '"')/*, '"' + [cmd].concat(args).map(function(a){if (/^[^"].* .*[^"]/.test(a)) return '"' + a + '"'; return a;}).join(' ')+'"']*/;
+ cmd = 'cmd';
+ opts.windowsVerbatimArguments = true;
+ }
+ require('child_process')
--- End diff --
Please put the `require` at the top of the file.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-android/pull/240#issuecomment-158328421
@dblotsky, i propose to proceed with current approach for now, otherwise we'll have to release 'cordova-common' with updated superspawn before this fix.
I created [CB-10052](https://issues.apache.org/jira/browse/CB-10052) for `superspawn` improvement, and once it is implemented, we could remove this cusom logic and switch to superspawn again.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r45385577
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +212,54 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
+ * with java printing some necessary information to stderr instead of stdout.
+ * This function redirects all stderr messages to current process stdout. See
--- End diff --
Update the comments based on the revised code that only redirects the one line.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-android/pull/240#issuecomment-158336605
Doesn't superspawn already return a promise whose output can be processed? Superspawn doesn't need to be modified.
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r44567649
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +209,46 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
--- End diff --
Why not just add another option to the existing spawn() function?
---
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-android pull request: CB-9971 Redirect gradlew stderr to c...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/240#discussion_r45386261
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -211,3 +209,46 @@ module.exports = GradleBuilder;
function isAutoGenerated(file) {
return fs.existsSync(file) && fs.readFileSync(file, 'utf8').indexOf(MARKER) > 0;
}
+
+/**
+ * A special superspawn-like implementation, required to workaround the issue
--- End diff --
This is not really a consideration anymore now that the redirection has to be more specialized to this particular case
---
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