-
Notifications
You must be signed in to change notification settings - Fork 395
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
fix extractDuplicates & added resource transformers from maven-shade-plu... #614
Conversation
The case that the current extractDuplicates functionality fails is the same as in my integration test. There are 3 libraries: libA -> resourceA, The current functionality builds a map from filenames to the jars they are found in, so for this example that would be: resourceA --> libA, libB Then all duplicates resources are removed from each list of jars, except for the first jar (so one copy of the resource remains) The pro lem here is that libB appears both as the first element, for resourceB, and as the second element, for resourceA. This means it is in the list of jars from which duplicates are removed. The result is that resourceB is not found in the output apk. My extract duplicates method resolves this issue by removed duplicates from ALL the jars in the list, then adding them back into an additional duplicate-resources.jar that is created by the plugin. This is also where the resource transformers put their output. The additional jar is added to the list of jars from which resources are added to the apk. Hope this makes it clear. |
@@ -265,6 +265,12 @@ | |||
<version>2.2</version> | |||
</dependency> | |||
|
|||
<dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a whole other plugin as a dependency seems like a bad idea. Why is this needed? Just for the ResourceTransformer?
Yes just for the ResourceTransformer interface and implementations. My logic was to reuse the existing transformers instead of recreating them. So that any other 3rd party transformers could be reused. What do you suggest instead of adding the dependency? Copying the source files? I'm open to whatever you think is best. |
754612f
to
f1962bb
Compare
This fails on the IT tests for me... can you try a full build and see if you can fix this? |
Running com.jayway.maven.plugins.android.sample.TicTacToeSampleIT Results : Failed tests: Tests run: 12, Failures: 1, Errors: 0, Skipped: 1 [WARNING] File encoding has not been set, using platform encoding UTF-8, i.e. build is platform dependent! The file encoding for reports output files should be provided by the POM property ${project.reporting.outputEncoding}. |
@mosabua Fixed the tests. Seems like some transitive dependencies from maven-shade-plugin were interfering. I excluded them. There was another failure in the aar-child test project, which I fixed by changing the junit version. Finally, what is final word on the maven-shade-plugin dependency? Do you think it is better to include the ResourceTransformer source code? |
This change also fixes a bug in extractDuplicates. The jar files which are created during the duplicate extraction are created in the project's outputDirectory (i.e. /target/classes); which means they end up getting packaged in the APK artifact, which makes no sense. |
I am fine with pulling this in, but I have to have a closer look first. |
…plugin -extractDuplicates had cases where it removed all copies of duplicates. added test to verify behavior is fixed -support for using resource transformers from maven-shade-plugin to merge duplicate resources -added integration test for said behavior
@mosabua Rebased. |
its in ;-) |
Right after upgrading from 4.2.1 to 4.3.0 I get build failures when building my multi-module project. Getting the erros at the espresso instrumentation test module: Here's part of the build
|
I'll give that a go when I get some time. For now I moved back to the previous 4.2.1 version and everything works just fine. |
Yes if u can help me recreate it I will fix it.
|
@kedzie this is a sample project that has the issue described. Just "mvn clean install -DskipTests" on the parent pom.xml. It's pretty much empty, just dependencies in there. |
I'm taking a look at it if it is still an issue. sorry for the delay. @athkalia |
@athkalia I pulled this down and there are a bunch of missing dependencies so it does not build. I also noticed you didn't set extractDependencies in the application apk. Anyway I cannot run the build and therefore cannot recreate the issue.
|
@kedzie Sorry for that, I didnd't notice. I tried fixing the dependency problems while still re-creating the issue described above but it seems like I am stuck half way. If you try building it again there are some espresso dependencies missing. Those are
Those are needed to recreate the issue. I've added them separately in the folder, you just need to manually add them to your .m2 directory under .m2\repository\com\android\support\test\espresso Also, what's that "extractDependencies " that you mentioned? Cheers, |
extractDependencies is a plugin configuration parameter. If it isn't set Anyway did u think about adding a local maven repo to this example? Just
|
...gin
-extractDuplicates had cases where it removed all copies of duplicates. added test to verify behavior is fixed
-support for using resource transformers from maven-shade-plugin to merge duplicate resources
-added integration test for said behavior