You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by nikhilkh <gi...@git.apache.org> on 2016/05/04 01:03:35 UTC

[GitHub] cordova-android pull request: CB-11198 Skip android target sdk che...

GitHub user nikhilkh opened a pull request:

    https://github.com/apache/cordova-android/pull/303

    CB-11198 Skip android target sdk check

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/MSOpenTech/cordova-android faster

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-android/pull/303.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 #303
    
----
commit 2df86f9f972d0e3149306b481ca5889f711c062c
Author: Nikhil Khandelwal <ni...@microsoft.com>
Date:   2016-05-03T23:02:02Z

    CB-11198 Skip android target sdk check

----


---
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-11198 Skip android target sdk che...

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/303#discussion_r61975678
  
    --- Diff: bin/lib/check_reqs.js ---
    @@ -254,18 +252,22 @@ module.exports.check_android_target = function(valid_target) {
             }
     
             var androidCmd = module.exports.getAbsoluteAndroidCmd();
    -        throw new CordovaError('Please install Android target: "' + valid_target + '".\n\n' +
    +        var msg = 'Please install Android target: "' + valid_target + '".\n\n' +
                 'Hint: Open the SDK manager by running: ' + androidCmd + '\n' +
                 'You will require:\n' +
                 '1. "SDK Platform" for ' + valid_target + '\n' +
                 '2. "Android SDK Platform-tools (latest)\n' +
    -            '3. "Android SDK Build-tools" (latest)');
    +            '3. "Android SDK Build-tools" (latest)'
    --- End diff --
    
    Missing semicolon.


---
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-11198 Skip android target sdk che...

Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/303#discussion_r62273198
  
    --- Diff: bin/templates/cordova/lib/builders/AntBuilder.js ---
    @@ -107,7 +107,18 @@ AntBuilder.prototype.build = function(opts) {
         var args = this.getArgs(opts.buildType == 'debug' ? 'debug' : 'release', opts);
         return check_reqs.check_ant()
         .then(function() {
    -        return spawn('ant', args, {stdio: 'inherit'});
    +        return spawn('ant', args, {stdio: 'pipe'});
    +    }).progress(function (stdio){
    --- End diff --
    
    Can you elaborate? As far as I can see superspawn only returns `stderr` output only if you 'pipe'. Piping implies that someone has to still write this to process.stdout & process.stderr.


---
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-11198 Skip android target sdk che...

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/303#discussion_r61976209
  
    --- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
    @@ -203,6 +203,11 @@ GradleBuilder.prototype.build = function(opts) {
             } else {
                 process.stdout.write(stdio.stdout);
             }
    +    }).catch(function (error) {
    +        if (error.toString().indexOf('failed to find target with hash string') >= 0) {
    +            return check_reqs.check_android_target(error);
    --- End diff --
    
    Would it be possible for check_android_target to actually find the target and then swallow this error?


---
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-11198 Skip android target sdk che...

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/303#discussion_r62002478
  
    --- Diff: bin/templates/cordova/lib/builders/AntBuilder.js ---
    @@ -107,7 +107,18 @@ AntBuilder.prototype.build = function(opts) {
         var args = this.getArgs(opts.buildType == 'debug' ? 'debug' : 'release', opts);
         return check_reqs.check_ant()
         .then(function() {
    -        return spawn('ant', args, {stdio: 'inherit'});
    +        return spawn('ant', args, {stdio: 'pipe'});
    +    }).progress(function (stdio){
    --- End diff --
    
    Should we handle stdio of `ant build` manually here?


---
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-11198 Skip android target sdk che...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cordova-android/pull/303


---
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-11198 Skip android target sdk che...

Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on the pull request:

    https://github.com/apache/cordova-android/pull/303#issuecomment-216705549
  
    :+1:


---
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-11198 Skip android target sdk che...

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/303#discussion_r61975714
  
    --- Diff: bin/lib/check_reqs.js ---
    @@ -238,13 +236,13 @@ module.exports.getAbsoluteAndroidCmd = function () {
         return cmd.replace(/(\s)/g, '\\$1');
     };
     
    -module.exports.check_android_target = function(valid_target) {
    +module.exports.check_android_target = function(originalError) {
    --- End diff --
    
    I don't see where any value for originalError is ever passed in by a caller.


---
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-11198 Skip android target sdk che...

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/303#discussion_r62303417
  
    --- Diff: bin/templates/cordova/lib/builders/AntBuilder.js ---
    @@ -107,7 +107,18 @@ AntBuilder.prototype.build = function(opts) {
         var args = this.getArgs(opts.buildType == 'debug' ? 'debug' : 'release', opts);
         return check_reqs.check_ant()
         .then(function() {
    -        return spawn('ant', args, {stdio: 'inherit'});
    +        return spawn('ant', args, {stdio: 'pipe'});
    +    }).progress(function (stdio){
    --- End diff --
    
    My bad, I somehow ignored the fact that you need to handle error output now


---
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