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.

Sign up for GitHub

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

Jump to bottom

[GRADLE] Support AGP 3.4.0 #724

Closed
wants to merge 3 commits into from
Closed

[GRADLE] Support AGP 3.4.0 #724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5772747
[GRADLE] Support AGP 3.4.0
justacodefan Jun 5, 2019
e31bb01
[GRADLE] More updates for AGP 3.3+
justacodefan Jun 5, 2019
fbd82b9
Small rename
justacodefan Jun 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions sentry-android-gradle-plugin/build.gradle
View file
Open in desktop
Expand Up @@ -18,9 +18,9 @@ compileGroovy {
}

dependencies {
compile gradleApi()
compile localGroovy()
compile 'com.android.tools.build:gradle:2.3.3'
compileOnly gradleApi()
compileOnly localGroovy()
compileOnly 'com.android.tools.build:gradle:3.4.0'
}

publishing {
Expand Down
2 changes: 1 addition & 1 deletion sentry-android-gradle-plugin/gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Expand Up @@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-4.1-all.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-5.4.1-all.zip
66 changes: 47 additions & 19 deletions sentry-android-gradle-plugin/src/main/groovy/io/sentry/android/gradle/SentryPlugin.groovy
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@ package io.sentry.android.gradle
import com.android.build.gradle.AppPlugin
import com.android.build.gradle.LibraryPlugin
import com.android.build.gradle.api.ApplicationVariant
import com.android.build.gradle.api.BaseVariantOutput
import org.apache.commons.compress.utils.IOUtils
import org.gradle.api.Plugin
import org.gradle.api.Project
Expand Down Expand Up @@ -144,7 +145,12 @@ class SentryPlugin implements Plugin<Project> {
* @return
*/
static String getDebugMetaPropPath(Project project, ApplicationVariant variant) {
return "${variant.mergeAssets.outputDir}/sentry-debug-meta.properties"
try {
return "${variant.mergeAssetsProvider.outputDir}/sentry-debug-meta.properties"
Copy link

Choose a reason for hiding this comment

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

mergeAssetsProvider has type TaskProvider<MergeSourceSetFolders>. I beleive we need call get()on it to make it works.

} catch (Throwable ignored) {
return "${variant.mergeAssets.outputDir}/sentry-debug-meta.properties"
Copy link

@nesterov-n nesterov-n Jun 5, 2019 •
edited

Choose a reason for hiding this comment

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

outputDir has type Provider<Directory> at least at agp 3.4.
Therefore this function returns something like this property(interface org.gradle.api.file.Directory, map(property(interface org.gradle.api.file.Directory, fixed(class org.gradle.api.internal.file.DefaultFilePropertyFactory$FixedDirectory, /Users/user/projects/myAwesomeApp/app/build))))/sentry-debug-meta.properties

I believe it should be return variant.mergeAssets.outputDir.get().file("sentry-debug-meta.properties").getAsFile().path

Copy link

Choose a reason for hiding this comment

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

What @nesterov-n said. In general I'd favor an implementation where instead of try-catching possible exceptions you rather check for the expected type and then call the right method. This - together with a couple of invariants / assertions in the right places - will result in a much more consistent and stable implementation that even gives you proper diagnostics in case something will change (again). And if we learned one thing with the AGP then that change is a constant thing.

}

}

void apply(Project project) {
Expand All @@ -158,27 +164,27 @@ class SentryPlugin implements Plugin<Project> {
project.android.applicationVariants.all { ApplicationVariant variant ->
variant.outputs.each { variantOutput ->
def manifestPath = extension.manifestPath

if (manifestPath == null) {
try {
// Android Gradle Plugin < 3.0.0
manifestPath = variantOutput.processManifest.manifestOutputFile
} catch (Exception ignored) {
// Android Gradle Plugin >= 3.0.0
def outputDir = variantOutput.processManifest.manifestOutputDirectory
// Gradle 4.7 introduced the lazy task API and AGP 3.3+ adopts that,
// so we apparently have a Provider<File> here instead
// TODO: This will let us depend on the configuration of each flavor's
// manifest creation task and their transitive dependencies, which in
// turn prolongs the configuration time accordingly. Evaluate how Gradle's
// new Task Avoidance API can be used instead.
// (https://docs.gradle.org/current/userguide/task_configuration_avoidance.html)
if (!(outputDir instanceof File)) {
outputDir = outputDir.get().asFile
}
manifestPath = new File(outputDir, "AndroidManifest.xml")
}
def dir = findAndroidManifestFileDir(variantOutput)
manifestPath = new File(dir, "AndroidManifest.xml")
}

// if (manifestPath == null) {
Copy link
Member

Choose a reason for hiding this comment

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

I guess this dead code can go away before merging?

// try {
// // Android Gradle Plugin < 3.0.0
// manifestPath = variantOutput.processManifest.manifestOutputFile
// } catch (Exception ignored) {
// // Android Gradle Plugin >= 3.0.0
// def outputDir = variantOutput.processManifest.manifestOutputDirectory
//
// if (!(outputDir instanceof File)) {
// outputDir = outputDir.get().asFile
// }
// manifestPath = new File(outputDir, "AndroidManifest.xml")
// }
// }

def mappingFile = variant.getMappingFile()
def proguardTask = getProguardTask(project, variant)
def dexTask = getDexTask(project, variant)
Expand Down Expand Up @@ -282,4 +288,26 @@ class SentryPlugin implements Plugin<Project> {
}
}
}

static File findAndroidManifestFileDir(BaseVariantOutput variantOutput) {
// Gradle 4.7 introduced the lazy task API and AGP 3.3+ adopts that,
Copy link
Member

Choose a reason for hiding this comment

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

We should create a follow up issue to get this done once we merge this PR.

// so we apparently have a Provider<File> here instead
// TODO: This will let us depend on the configuration of each flavor's
// manifest creation task and their transitive dependencies, which in
// turn prolongs the configuration time accordingly. Evaluate how Gradle's
// new Task Avoidance API can be used instead.
// (https://docs.gradle.org/current/userguide/task_configuration_avoidance.html)

try { // Android Gradle Plugin >= 3.3.0
return variantOutput.processManifestProvider.get().manifestOutputDirectory.get().asFile
} catch (Exception ignored) {}

try { // Android Gradle Plugin >= 3.0.0
return variantOutput.processManifest.manifestOutputDirectory.get().asFile
} catch (Exception ignored) {}

try { // Android Gradle Plugin < 3.0.0
return new File(variantOutput.processManifest.manifestOutputFile).parentFile
} catch (Exception ignored) {}
}
}