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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-25865] Android: Modified CLI to not select "build-tools" version newer than supported if a supported version is installed #28

Merged
merged 2 commits into from Jun 5, 2018
Merged

[TIMOB-25865] Android: Modified CLI to not select "build-tools" version newer than supported if a supported version is installed #28

merged 2 commits into from Jun 5, 2018

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented May 7, 2018 •
edited

JIRA:
https://jira.appcelerator.org/browse/TIMOB-25865

Summary:

  • This avoids an unnecessary "too new" warning from being logged after every build. Will now use the preferred build-tools version instead of always selecting newest version.
  • Modified to log which version(s) are supported (in parenthesis) if installed build-tools, platform-tools, and SDK is older than supported. This way the end-user has a clue on what to install.

Note:
Our appc.version.satisfies() function returns boolean values true and false... as well as the string 'maybe' if above supported version range (ie: too new). Doing an if-check on the 'maybe' string was what was biting us here.

Test:

  1. Open the Android SDK Manager.
  2. Install "build-tools" versions 27.x and 26.x, if not done already.
  3. Build a Titanium project for Android.
  4. Observe the build log and verify that a warning similar to the below is NOT logged.
[WARN] :   Android Build Tools 27.0.3 are too new and may or may not work with Titanium.
[WARN] :   If you encounter problems, select a supported version with:
[WARN] :      appc ti config android.buildTools.selectedVersion ##.##.##
[WARN] :    where ##.##.## is a version in /Users/user/Library/Android/sdk/build-tools that is 26.x
  1. Go to the Android SDK's "build-tools" directory in Finder or Explorer. On Mac, the default location is: ~/Library/Android/sdk/build-tools
  2. Under the "build-tools" directory, there should be multiple subdirectories named after the build-tools versions installed such as "26.0.1" and "27.0.3".
  3. Move all subdirectory build-tools versions older than version 27 to the desktop. (We'll move them back after the test.)
  4. Re-build the Titanium project for Android and observe the log.
  5. Verify that you do see the "too new" warning message mentioned in step 4 above.
  6. Move all of the remaining "build-tools" version subdirectories to you desktop (this includes v27 subdirectories).
  7. Re-build the Titanium project for Android and observe the log.
  8. Verify something similar to the following gets displayed. The "(Supported: version)" in parenthesis is something new this PR is logging.
[ERROR] :  The Android SDK located at /Users/user/Library/Android/sdk has incomplete or out-of-date packages.
[ERROR] :  
[ERROR] :  Current installed Android SDK tools:
[ERROR] :    Android SDK Tools:          25.2.5  (Supported: <=26.x)
[ERROR] :    Android SDK Platform Tools: 27.0.1  (Supported: 26.x)
[ERROR] :    Android SDK Build Tools:    not installed  (Supported: 26.x)
  1. Move the "build-tools" subdirectories back to their prior location to restore what you previously had installed.

@jquick-axway
Android: Modified CLI to not select "build-tools" version newer than …
f4c20fb 
…supported if a supported version is installed.

- This avoids an unecessary "too new" warning logged after every build.
- Also modified to log which version(s) are supported if installed build-tools is older than supported.
garymathews
garymathews reviewed May 8, 2018
View reviewed changes
lib/android.js Outdated
missing.forEach(function (m) {
msg += ' ' + commandPrefix + 'ti config android.executables.' + m + ' "' + path.join(dummyPath, m + requiredSdkTools[m]) + '"\n';
});
msg = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

single quotes?

@lokeshchdhry
Copy link
Contributor

FR Passed.

  1. If having supported version & higher : The supported version is selected.
  2. If only having version higher than supported : Warning is shown as below
[WARN] :   Android Build Tools 28.0.0 are too new and may or may not work with Titanium.
[WARN] :   If you encounter problems, select a supported version with:
[WARN] :      appc ti config android.buildTools.selectedVersion ##.##.##
[WARN] :    where ##.##.## is a version in /Users/user/Library/Android/sdk/build-tools that is 26.x
  1. If only having version lower than supported: Error is thrown as below
[ERROR] :  The Android SDK located at /Users/user/Library/Android/sdk has incomplete or out-of-date packages.
[ERROR] :  
[ERROR] :  Current installed Android SDK tools:
[ERROR] :    Android SDK Tools:          25.2.5  (Supported: <=26.x)
[ERROR] :    Android SDK Platform Tools: 27.0.1  (Supported: 26.x)
[ERROR] :    Android SDK Build Tools:    25.0.3  (Supported: 26.x)
  1. If no build tools present: Error is thrown
[ERROR] :  The Android SDK located at /Users/user/Library/Android/sdk has incomplete or out-of-date packages.
[ERROR] :  
[ERROR] :  Current installed Android SDK tools:
[ERROR] :    Android SDK Tools:          25.2.5  (Supported: <=26.x)
[ERROR] :    Android SDK Platform Tools: 27.0.1  (Supported: 26.x)
[ERROR] :    Android SDK Build Tools:    not installed  (Supported: 26.x)

Studio Ver: 5.1.0.201804230827
SDK Ver: 7.2.0 local build with updated node-titanium-sdk
OS Ver: 10.13.4
Xcode Ver: Xcode 9.3
Appc NPM: 4.2.13
Appc CLI: 7.0.3
Daemon Ver: 1.1.1
Ti CLI Ver: 5.1.0
Alloy Ver: 1.12.0
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 10

@lokeshchdhry
Copy link
Contributor

Waiting for CR to merge.

@hansemannn
Copy link
Contributor

@sgtcoolguy Can you do the CR?

garymathews
garymathews approved these changes May 31, 2018
View reviewed changes
Copy link
Contributor

@garymathews garymathews left a comment •
edited

Choose a reason for hiding this comment

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

CR: PASS

But the package.json version needs bumping

@lokeshchdhry
Copy link
Contributor

@garymathews, We will need a SDK PR for updating the package.json. So this can be merged. Have commented in the ticket to open a SDK PR for the same.

@lokeshchdhry lokeshchdhry merged commit dbdb4f7 into tidev:master Jun 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garymathews garymathews garymathews approved these changes

@sgtcoolguy sgtcoolguy Awaiting requested review from sgtcoolguy

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jquick-axway @lokeshchdhry @hansemannn @garymathews