Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Update AndroidManifestFinder for Gradle builds with kapt #2030

Merged
WonderCsabo merged 2 commits into from
Jul 30, 2017
Merged

Update AndroidManifestFinder for Gradle builds with kapt #2030

WonderCsabo merged 2 commits into from
Jul 30, 2017

Conversation

@dodgex
Copy link
Member

@dodgex dodgex commented Jul 27, 2017

This should fix #2028

@WonderCsabo @ahornerr could you test this?

@dodgex dodgex requested a review from WonderCsabo July 27, 2017 20:31
@dodgex dodgex mentioned this pull request Jul 27, 2017
Merged
@dodgex
Copy link
Member Author

dodgex commented Jul 27, 2017

It even works for multiple flavor dimensions. :)

    flavorDimensions "flavor", "color"
    productFlavors {
        some_flavor {
            dimension "flavor"
        }
        some_flavor2 {
            dimension "flavor"
        }
        red {
            dimension "color"
        }
        green {
            dimension "color"
        }
    }

WonderCsabo
WonderCsabo suggested changes Jul 28, 2017
View reviewed changes
Copy link
Member

@WonderCsabo WonderCsabo left a comment

Choose a reason for hiding this comment

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

This works really well for kapt builds. Thanks!

However manifest finding for apt builds with multiple flavors is now broken. Can you fix it? I am wondering, maybe we should just skip listing all the directories in case of apt, and just use the old behaviour, this should also retain the performance.

Also, can you please add more tests? We should have tests when you have multiple flavors, for both kapt and apt.

...dannotations/src/main/java/org/androidannotations/internal/helper/AndroidManifestFinder.java
return Arrays.asList("build/intermediates/manifests/full" + gradleVariant, "build/intermediates/bundles" + gradleVariant,
"build/intermediates/manifests/aapt" + gradleVariant);
ArrayList<String> possibleLocations = new ArrayList<>();
for (String directory : Arrays.asList("build/intermediates/manifests/full", "build/intermediates/bundles", "build/intermediates/manifests/aapt")) {
Copy link
Member

@WonderCsabo WonderCsabo Jul 28, 2017

Choose a reason for hiding this comment

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

Add a new line.

Copy link
Member

@WonderCsabo WonderCsabo Jul 28, 2017

Choose a reason for hiding this comment

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

Before the for

...dannotations/src/main/java/org/androidannotations/internal/helper/AndroidManifestFinder.java

private void findPossibleLocations(String basePath, String targetPath, String variantPart, List<String> possibleLocations) {
String[] directories = new File(basePath + targetPath).list();
if (directories == null) {
Copy link
Member

@WonderCsabo WonderCsabo Jul 28, 2017

Choose a reason for hiding this comment

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

Add a new line.

Copy link
Member

@WonderCsabo WonderCsabo Jul 28, 2017

Choose a reason for hiding this comment

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

Before the if

@dodgex
Copy link
Member Author

dodgex commented Jul 28, 2017

Not sure where exaclty you want new lines.

@dodgex
Copy link
Member Author

dodgex commented Jul 28, 2017 •
edited
Loading

The new commit fixes APT builds. (I'll squash commits when its ready)

@WonderCsabo
Copy link
Member

WonderCsabo commented Jul 28, 2017

Thanks! Can you also add tests which use flavors?

@dodgex
Copy link
Member Author

dodgex commented Jul 28, 2017

I'm on it :)

@dodgex
Copy link
Member Author

dodgex commented Jul 28, 2017

@WonderCsabo tests added.

@ahornerr
Copy link

ahornerr commented Jul 28, 2017

What's the best way to test this?

@WonderCsabo
Copy link
Member

WonderCsabo commented Jul 28, 2017

@ahornerr what i did is creating flavors in the example projects.

@dodgex
Copy link
Member Author

dodgex commented Jul 30, 2017

merged commits and added requested new lines.

@WonderCsabo WonderCsabo merged commit ffbffe1 into androidannotations:develop Jul 30, 2017
@WonderCsabo WonderCsabo added this to the 4.4 milestone Jul 30, 2017
@WonderCsabo
Copy link
Member

WonderCsabo commented Jul 30, 2017

I tested it again, works well, thanks!

@dodgex dodgex deleted the 2028_improve_manifestfinder_for_kapt_with_variants branch July 30, 2017 09:32
@dodgex dodgex mentioned this pull request Aug 3, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@WonderCsabo WonderCsabo WonderCsabo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

'Could not find the AndroidManifest.xml file' error using AA 4.4.0-SNAPSHOT and Android Gradle plugin 3.0.0-alpha7 with productFlavors

3 participants

@dodgex @WonderCsabo @ahornerr