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.

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

[TIMOB-24275] (2_0_X) Hyperloop: Android - Class not found/Cannot read property 'newInstance' of null (Regression) #129

Merged
merged 1 commit into from Mar 9, 2017

Conversation

sgtcoolguy
Copy link
Contributor

The issue that caused this was our JAR duplication checks in the Hyperloop plugin hook for Android. Specifically we try to avoid a dexer "already added" error that occurs when the same class is included from multiple JAR files. See http://tools.android.com/recent/dealingwithdependenciesinandroidprojects

My first stab at checking for this was to simply check for conflicting jar base filenames. That's a big issue when using multiple AARs since the format of AARs it to contain a "classes.jar" file in them, so they'll all have the same base name.

This fix is to do a little more sophisticated checking, specifically:

  • if the file is a classes.jar, assume it came from an AAR and basically just retain it without duplication checks. (we could do better and track the AARs we've expanded to set the whitelist to skip here rather than just skipping all "classes.jar" files)
  • if the JAR has the same SHA1 as any other JAR we processed, skip it and log that we're doing so (at debug level)
  • if the JAR has a unique SHA1, but has the same base name as another JAR, error out and tell the user to delete one of them or rename one to resolve the conflict.

Please note that we already have very similar code in Titanium SDK's android CLI here: https://github.com/appcelerator/titanium_mobile/blob/master/android/cli/commands/_build.js#L1619

That code will check for conflicting JARs inside Titanium modules. The long-term solution is to re-use the same code if possible, and do the check before we run through dexer (and after hyperloop/plugins/hooks may have modified the list of JARs to dev)

data.args[1] = uniqueJars;

// Special case for android-support-v4.jar and android-support-v13.jar
if (basenames.hasOwnProperty('android-support-v4.jar') && basenames.hasOwnProperty('android-support-v13.jar')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We are currently updating the support library, so we will likely soon need a different way to handle those special cases or remind us to update this line every time we update the support library - which should become more frequent than it used to be.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. In my current PR tidev/titanium-sdk#8858 the libraries are now included with a version number, e.g. android-support-v4-23.4.0.jar. I'll change that back to omit the version number to avoid more unnecessary changes for now. We are also evaluating to allow users to supply their own version of the Android support library, i added a note in TIMOB-24446 so we won't forget about this special case in Hyperloop.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we do, we should still have some kind of registry to track different versions. Or find a solution that's more flexible, although that's easier said than done.

@hansemannn
Copy link
Contributor

Reminder to myself: Do a master "forwardport" 😁 for consistency.

Copy link
Contributor

@janvennemann janvennemann left a comment

Choose a reason for hiding this comment

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

CR looks good, approved! I also created TIMOB-24467 to track your suggested long term solution for the duplicate .jar checking in both Hyperloop and TiSDK

@hansemannn hansemannn changed the title Fix [TIMOB-24275] Hyperloop: Android - Class not found/Cannot read property 'newInstance' of null (Regression) [TIMOB-24275] (2_0_X) Hyperloop: Android - Class not found/Cannot read property 'newInstance' of null (Regression) Mar 8, 2017
@janvennemann
Copy link
Contributor

FT also passed!

@hansemannn hansemannn merged commit 842dae8 into tidev:2_0_X Mar 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants