[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[filmaj]

Github user filmaj commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r106050913
  
--- Diff: bin/lib/check_reqs.js ---
@@ -246,7 +273,11 @@ module.exports.check_android_target = 
function(originalError) {
 //   Google Inc.:Glass Development Kit Preview:20
 var valid_target = module.exports.get_target();
 var msg = 'Android SDK not found. Make sure that it is installed. If 
it is not at the default location, set the ANDROID_HOME environment variable.';
-return tryCommand('android list targets --compact', msg)
+//   Changing "targets" to "target" is stupid and makes more code.  
Thanks Google!
+var cmd = 'android list targets --compact';
+if(forgivingWhichSync('avdmanager').length > 0)
+  cmd = 'avdmanager list target --compact';
--- End diff --

Actually, don't worry about it, I already have a fix incoming anyways.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[filmaj]

Github user filmaj commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r106043706
  
--- Diff: bin/lib/check_reqs.js ---
@@ -246,7 +273,11 @@ module.exports.check_android_target = 
function(originalError) {
 //   Google Inc.:Glass Development Kit Preview:20
 var valid_target = module.exports.get_target();
 var msg = 'Android SDK not found. Make sure that it is installed. If 
it is not at the default location, set the ANDROID_HOME environment variable.';
-return tryCommand('android list targets --compact', msg)
+//   Changing "targets" to "target" is stupid and makes more code.  
Thanks Google!
+var cmd = 'android list targets --compact';
+if(forgivingWhichSync('avdmanager').length > 0)
+  cmd = 'avdmanager list target --compact';
--- End diff --

I just realized this but I think what we want here is to use `sdkmanager` 
and not `avdmanager`, right @infil00p ? `avdmanager list targets` returns 
either AVD device definitions or existing AVD IDs. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[infil00p]

Github user infil00p commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r105255990
  
--- Diff: bin/lib/check_reqs.js ---
@@ -78,21 +79,46 @@ module.exports.check_ant = function() {
 });
 };
 
+module.exports.get_gradle_wrapper = function() {
+var androidStudioPath;
+if(os.platform() == "darwin") {
--- End diff --

@dpogue Honestly, we're just installing the wrapper, so any will work.  I 
wanted to delete 
https://github.com/apache/cordova-android/blob/master/bin/templates/cordova/lib/builders/GradleBuilder.js#L187
 for a while, since it replaces everything with an old version of Gradle that 
just gets updated anyway, but we might just have to keep it in and update it to 
the latest Gradle supported.

FYI: My homebrew keeps fetching 2.2, and Android Studio 2.3 is packaged 
with 3.2 on my Windows and Mac partitions.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[dpogue]

Github user dpogue commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r105253370
  
--- Diff: bin/lib/check_reqs.js ---
@@ -78,21 +79,46 @@ module.exports.check_ant = function() {
 });
 };
 
+module.exports.get_gradle_wrapper = function() {
+var androidStudioPath;
+if(os.platform() == "darwin") {
--- End diff --

A particular version of gradle, or just gradle in general?

Ubuntu gradle versions vary from release to release: 
http://packages.ubuntu.com/search?keywords=gradle
ArchLinux and homebrew (macOS) are both shipping the latest version (3.4)
Latest Android Studio ships 3.3


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[infil00p]

Github user infil00p commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r105252250
  
--- Diff: bin/lib/check_reqs.js ---
@@ -78,21 +79,46 @@ module.exports.check_ant = function() {
 });
 };
 
+module.exports.get_gradle_wrapper = function() {
+var androidStudioPath;
+if(os.platform() == "darwin") {
--- End diff --

I have no idea where Android Studio is installed on a Linux box.  It 
literally could be anywhere!  If you're not using Android Studio, you HAVE to 
have Gradle installed.  It's either one or the other from here on out.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[filmaj]

Github user filmaj commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r105246727
  
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -65,6 +65,20 @@ GradleBuilder.prototype.getArgs = function(cmd, opts) {
 return args;
 };
 
+/*
+ * This returns a promise
+ */
+
+GradleBuilder.prototype.runGradleWrapper = function(gradle_cmd) {
+   var gradlePath = path.join(this.root, 'gradlew');
+   if(fs.existsSync(gradlePath)) {
+   ;
--- End diff --

I think the spacing is off, too, yeah?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[filmaj]

Github user filmaj commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r105240916
  
--- Diff: bin/lib/check_reqs.js ---
@@ -78,21 +79,46 @@ module.exports.check_ant = function() {
 });
 };
 
+module.exports.get_gradle_wrapper = function() {
+var androidStudioPath;
+if(os.platform() == "darwin") {
--- End diff --

What about not-Mac and not-Windows?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[filmaj]

Github user filmaj commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r105241332
  
--- Diff: bin/lib/check_reqs.js ---
@@ -78,21 +79,46 @@ module.exports.check_ant = function() {
 });
 };
 
+module.exports.get_gradle_wrapper = function() {
+var androidStudioPath;
+if(os.platform() == "darwin") {
--- End diff --

K, based on the rest of the flow, it looks like for not-Mac and 
not-Windows, we're hoping that `gradle` is on the PATH. Is that acceptable? I 
don't have an opinion either way but I think it's worth posing the question.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[filmaj]

Github user filmaj commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r105241092
  
--- Diff: bin/lib/check_reqs.js ---
@@ -78,21 +79,46 @@ module.exports.check_ant = function() {
 });
 };
 
+module.exports.get_gradle_wrapper = function() {
+var androidStudioPath;
+if(os.platform() == "darwin") {
+  androidStudioPath = path.join('/Applications', 'Android Studio.app', 
'Contents', 'gradle');
+} else if (os.platform() == "win32") {
+  androidStudioPath = path.join(process.env["ProgramFiles"],'Android', 
'Android Studio', 'gradle');
+}
+
+if(androidStudioPath != null && fs.existsSync(androidStudioPath)) {
+  var dirs = fs.readdirSync(androidStudioPath);
+  if(dirs[0].split('-')[0] == "gradle")
+  {
+return path.join(androidStudioPath, dirs[0], "bin", "gradle");
+  }
+} else {
+  //OK, let's try to check for Gradle!
+  return forgivingWhichSync('gradle');
+}
+};
+
 // Returns a promise. Called only by build and clean commands.
 module.exports.check_gradle = function() {
 var sdkDir = process.env['ANDROID_HOME'];
+var d = Q.defer();
 if (!sdkDir)
 return Q.reject(new CordovaError('Could not find gradle wrapper 
within Android SDK. Could not find Android SDK directory.\n' +
 'Might need to install Android SDK or set up \'ANDROID_HOME\' 
env variable.'));
 
-var wrapperDir = path.join(sdkDir, 'tools', 'templates', 'gradle', 
'wrapper');
-if (!fs.existsSync(wrapperDir)) {
-return Q.reject(new CordovaError('Could not find gradle wrapper 
within Android SDK. Might need to update your Android SDK.\n' +
-'Looked here: ' + wrapperDir));
-}
-return Q.when();
+var path = this.get_gradle_wrapper();
+console.log(path);
--- End diff --

Probably want to remove this 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[filmaj]

Github user filmaj commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/367#discussion_r105241573
  
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -65,6 +65,20 @@ GradleBuilder.prototype.getArgs = function(cmd, opts) {
 return args;
 };
 
+/*
+ * This returns a promise
+ */
+
+GradleBuilder.prototype.runGradleWrapper = function(gradle_cmd) {
+   var gradlePath = path.join(this.root, 'gradlew');
+   if(fs.existsSync(gradlePath)) {
+   ;
--- End diff --

What's going on in 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android pull request #367: CB-12524: Working around Android SDK upda...

[infil00p]

GitHub user infil00p opened a pull request:

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

CB-12524: Working around Android SDK update by finding Gradle in Android 
Studio itself, if that fails, we look for a locally installed Gradle



### Platforms affected
Android

### What does this PR do?
This PR allows Cordova users to be able to build Android projects again 
using the platform CLI tools.  

### What testing has been done on this change?
Testing has been done on the build scripts to make sure things work, but 
testing on the CLI integration still needs to be done.

### Checklist
- [ x] [Reported an 
issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
- [ x] Commit message follows the format: "CB-3232: (android) Fix bug with 
resolving file paths", where CB- is the JIRA ID & "android" is the platform 
affected.
- [ ] Added automated test coverage as appropriate for this change.


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

$ git pull https://github.com/infil00p/cordova-android android_sdk_fixes

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

https://github.com/apache/cordova-android/pull/367.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 #367


commit 78c5a54e5f255aab205ecbc72047b5cce9f18a66
Author: Joe Bowser 
Date:   2017-03-06T23:03:32Z

We're distributing a wrapper in the framework directory

commit 9469d11669fcf6ef4342954d46722144c9f36731
Author: Joe Bowser 
Date:   2017-03-06T23:55:22Z

CB-12524: Updating gradle to just grab the wrapper from the framework 
directory for building

commit 4502e3253783425eda5cff881e48f4d58de88e3e
Author: Joe Bowser 
Date:   2017-03-07T22:29:02Z

CB-12524: Create script needs to copy the wrapper from the framework 
directory to the CordovaLib directory.  This means you can compile an AAR out 
of any of the projects if you wanted.

commit 8c01438e16ad1403e9eb54d1870bbc8371458343
Author: Joe Bowser 
Date:   2017-03-09T02:09:49Z

CB-12524: This now fetches the template from  inside of the Android Studio 
directory, and falls back to a locally installed Gradle instance

commit a4fa4b79fc6a8ad8afe9249aa591c62e1742d666
Author: Bharath Hariharan 
Date:   2017-02-23T19:13:10Z

Updating version

 This closes #364

commit f3408169735e01817bdde113547ca40375529ebb
Author: Joe Bowser 
Date:   2017-03-09T18:26:48Z

CB-12524: Windows functionality for Gradle execution

commit 45e3c6983470d47628e8deba23ab41498638a8a9
Author: Joe Bowser 
Date:   2017-03-09T18:32:07Z

Accidentally committed temp files in Windows




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org