Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings
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
androidannotations:developandroidannotations/androidannotations:developfrom
dodgex:2028_improve_manifestfinder_for_kapt_with_variantsCopy head branch name to clipboard
Jul 30, 2017
Merged

Update AndroidManifestFinder for Gradle builds with kapt#2030
WonderCsabo merged 2 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
dodgex:2028_improve_manifestfinder_for_kapt_with_variantsCopy head branch name to clipboard

Conversation

@dodgex

@dodgex dodgex commented Jul 27, 2017

Copy link
Copy Markdown
Member

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

dodgex commented Jul 27, 2017

Copy link
Copy Markdown
Member Author

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

@WonderCsabo WonderCsabo left a comment

Copy link
Copy Markdown
Member

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.

Comment thread ...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")) {

@WonderCsabo WonderCsabo Jul 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add a new line.

@WonderCsabo WonderCsabo Jul 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Before the for

Comment thread ...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) {

@WonderCsabo WonderCsabo Jul 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add a new line.

@WonderCsabo WonderCsabo Jul 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Before the if

@dodgex

dodgex commented Jul 28, 2017

Copy link
Copy Markdown
Member Author

Not sure where exaclty you want new lines.

@dodgex

dodgex commented Jul 28, 2017 •
edited
Loading

Copy link
Copy Markdown
Member Author

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

@WonderCsabo

WonderCsabo commented Jul 28, 2017

Copy link
Copy Markdown
Member

Thanks! Can you also add tests which use flavors?

@dodgex

dodgex commented Jul 28, 2017

Copy link
Copy Markdown
Member Author

I'm on it :)

@dodgex

dodgex commented Jul 28, 2017

Copy link
Copy Markdown
Member Author

@WonderCsabo tests added.

@ahornerr

ahornerr commented Jul 28, 2017

Copy link
Copy Markdown

What's the best way to test this?

@WonderCsabo

WonderCsabo commented Jul 28, 2017

Copy link
Copy Markdown
Member

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

@dodgex

dodgex commented Jul 30, 2017

Copy link
Copy Markdown
Member Author

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

WonderCsabo commented Jul 30, 2017

Copy link
Copy Markdown
Member

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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.