[TIMOB-24532] Use .aar handling from AndroidBuilder #206
Conversation
janvennemann
force-pushed
the
TIMOB-24532
branch
from
bb1facf
to
53a0eb9
Compare
| var libDir = path.join(module.modulePath, 'lib'); | ||
| fs.existsSync(libDir) && fs.readdirSync(libDir).forEach(function (name) { | ||
| var jarFile = path.join(libDir, name); | ||
| if (jarRegExp.test(name) && fs.existsSync(jarFile)) { |
There was a problem hiding this comment.
Do we really need an existence check here? We got the filename from readdirSync - I think it's pretty safe to assume it didn't get deleted immediately between the calls here...
There was a problem hiding this comment.
Yeah, i think it's safe to remove it.
|
|
||
| builder.androidLibraries.forEach(function (libraryInfo) { | ||
| libraryInfo.jars.forEach(function (libraryJarPathAndFilename) { | ||
| var jarHash = builder.hash(fs.readFileSync(libraryJarPathAndFilename).toString()); |
There was a problem hiding this comment.
The core of the loop is the exact same as above. Maybe gather all the JAR files together first, then use the same loop to generate hashes and push to the array or warn?
Additionally, I think it'd be beneficial to gather all the JARs and generate all the hashes in parallel using async, rather than do them all in series. I assume you'll get some performance bump, but obviously you'll have to change the collection a little to store full path => hash, and then the last step would be to iterate over the hashes in series and filter the duplicates.
There was a problem hiding this comment.
Sure, let me check that out
| jars.push(jarFile); | ||
| jarHashes[jarHash] = 1; | ||
| } else { | ||
| logger.debug('Excluding duplicate jar file %s from metabase generation', jarFile.cyan); |
There was a problem hiding this comment.
I think it's useful to point the user to the other JAR(s) that had the same hash, so they can determine the other duplicates and decide which one is "right". For example, some may be in our SDK/modules and they can't control it. Or maybe they're trying to double-add some library or support AAR or something?
There was a problem hiding this comment.
Well, technically there is no "right" JAR because if the hash is the same it doesn't matter which one is used. I adopted this from the Android builder which does the same check for duplicate JARs here.
The builder also has a check for conflicting JARs from modules (i.e. same filename but different hash), which stops the build and prints detailed information about the conflicting JARs. The same goes for AAR files. In this case the user has to check which one to use before he can continue the build, but this happens before the Hyperloop plugin gets executed.
| } | ||
|
|
||
| // 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')) { |
There was a problem hiding this comment.
I assume this stuff is irrelevant now because we uses hashes on JARs? Or maybe this is part of the Android build in general now that we support AARs?
There was a problem hiding this comment.
Yes, the build also does the hash check for JARs, so there is no need for this here anymore. I decided to also remove the special check for the android-support-v13.jar because it relies on a naming pattern and there is no proper way to guarantee the jar is always named like this. At least not when it's meant to be dropped in by a user. Do you remember the actual use case for that check?
|
@sgtcoolguy, hash generation is now done in parallel and the dupe check also works on any JARs from the android platform folder. |
JIRA: https://jira.appcelerator.org/browse/TIMOB-24532
Removes the AAR handling from Hyperloop plugin hook and instead relies on the AAR handling introduced with 6.1.0 in the SDK. With this change it is also possible to access any third-party library .jar from modules.