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

use source files as input for generating uuids and source bundle #634

Merged
merged 22 commits into from
Feb 5, 2024
Merged

use source files as input for generating uuids and source bundle #634

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
731fc78
use source files as input for generating uuids and source bundle
lbloder Jan 16, 2024
0c3c190
Allow UploadSourceBundleTask to be considered up to date, use mapping…
lbloder Jan 18, 2024
5655c9e
remove org config and auth
lbloder Jan 22, 2024
9f70a41
Merge branch 'main' into feat/source-context-build-cache
lbloder Jan 22, 2024
0dbd408
format
lbloder Jan 22, 2024
8cfad0e
null safe access to sentryVariant in AndroidComponentsConfig
lbloder Jan 23, 2024
1403b5a
Merge branch 'main' into feat/source-context-build-cache
lbloder Jan 24, 2024
8ebff03
add new tests
lbloder Jan 30, 2024
d1e403a
format
lbloder Jan 30, 2024
82c536f
Merge branch 'main' into feat/source-context-build-cache
lbloder Jan 30, 2024
5b91473
add tests for debugMetaProperties, bundleId generation and collectSou…
lbloder Jan 31, 2024
12f0f8c
Merge branch 'main' into feat/source-context-build-cache
lbloder Jan 31, 2024
fd2277f
rename inputFiles param for ProguardUuidTask
lbloder Jan 31, 2024
1475d0a
revert changes to PersonController
lbloder Feb 1, 2024
55d6c08
Merge branch 'main' into feat/source-context-build-cache
lbloder Feb 2, 2024
fa1714d
add clarifying comments for upToDateWhen
lbloder Feb 5, 2024
2c91edb
add additional SourceContext tests
lbloder Feb 5, 2024
4a6b42c
fix unused import
lbloder Feb 5, 2024
3577716
combine tests for generateSentryBundleId and sentryCollectSources tas…
lbloder Feb 5, 2024
aca3440
fix line length
lbloder Feb 5, 2024
8e8e2e8
add missing org, projectName and url
lbloder Feb 5, 2024
2ad414c
fix inverted test mixup
lbloder Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions examples/spring-boot-sample/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,10 @@ sentry {
debug.set(true)
includeSourceContext.set(true)
additionalSourceDirsForSourceContext.set(setOf("testsrc"))

org.set("lukas-bloder")
markushi marked this conversation as resolved.
Show resolved Hide resolved
projectName.set("java")
authToken.set(
"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"
)
}
1 change: 0 additions & 1 deletion ...ples/spring-boot-sample/src/main/java/io/sentry/samples/spring/boot/PersonController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public PersonController(PersonService personService) {

@GetMapping("{id}")
Person person(@PathVariable Long id) {
LOGGER.info("Loading person with id={}", id);
throw new IllegalArgumentException("Something went wrong [id=" + id + "]");
}

Expand Down
19 changes: 17 additions & 2 deletions plugin-build/src/main/kotlin/io/sentry/android/gradle/AndroidComponentsConfig.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import io.sentry.android.gradle.util.hookWithMinifyTasks
import io.sentry.android.gradle.util.info
import java.io.File
import org.gradle.api.Project
import org.gradle.api.file.Directory
import org.gradle.api.provider.Provider
import org.gradle.api.provider.SetProperty
import org.gradle.api.tasks.TaskProvider
Expand Down Expand Up @@ -73,13 +74,26 @@ fun AndroidComponentsExtension<*, *, *>.configure(

variant.configureDependenciesTask(project, extension, sentryTelemetryProvider)

// TODO: do this only once, and all other tasks should be SentryVariant.configureSomething
val sentryVariant = if (isAGP74) AndroidVariant74(variant) else null

val additionalSourcesProvider = project.provider {
extension.additionalSourceDirsForSourceContext.getOrElse(emptySet())
.map { project.layout.projectDirectory.dir(it) }
}
val sourceFiles = sentryVariant!!.sources(
project,
additionalSourcesProvider
)

val tasksGeneratingProperties =
mutableListOf<TaskProvider<out PropertiesFileOutputTask>>()
val sourceContextTasks = variant.configureSourceBundleTasks(
project,
extension,
sentryTelemetryProvider,
paths,
sourceFiles,
cliExecutable,
sentryOrg,
sentryProject
Expand All @@ -97,8 +111,6 @@ fun AndroidComponentsExtension<*, *, *>.configure(
)
generateProguardUuidTask?.let { tasksGeneratingProperties.add(it) }

// TODO: do this only once, and all other tasks should be SentryVariant.configureSomething
val sentryVariant = if (isAGP74) AndroidVariant74(variant) else null
sentryVariant?.configureNativeSymbolsTask(
project,
extension,
Expand Down Expand Up @@ -265,6 +277,7 @@ private fun Variant.configureSourceBundleTasks(
extension: SentryPluginExtension,
sentryTelemetryProvider: Provider<SentryTelemetryService>,
paths: OutputPaths,
sourceFiles: Provider<out Collection<Directory>>,
cliExecutable: Provider<String>,
sentryOrg: String?,
sentryProject: String?
Expand All @@ -280,6 +293,7 @@ private fun Variant.configureSourceBundleTasks(
sentryTelemetryProvider,
variant,
paths,
sourceFiles,
cliExecutable,
sentryOrg,
sentryProject,
Expand Down Expand Up @@ -354,6 +368,7 @@ private fun Variant.configureProguardMappingsTasks(
project = project,
extension,
sentryTelemetryProvider,
sourceFiles = getMappingFileProvider(project, variant, dexguardEnabled),
taskSuffix = name.capitalized,
output = paths.proguardUuidDir
)
Expand Down
40 changes: 29 additions & 11 deletions plugin-build/src/main/kotlin/io/sentry/android/gradle/AppConfig.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import io.sentry.android.gradle.util.hookWithPackageTasks
import io.sentry.android.gradle.util.info
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.file.Directory
import org.gradle.api.provider.Provider
import org.gradle.api.tasks.TaskProvider
import org.gradle.internal.build.event.BuildEventListenerRegistryInternal
Expand Down Expand Up @@ -68,11 +69,32 @@ fun AppExtension.configure(
buildEvents
)

// TODO: do this only once, and all other tasks should be SentryVariant.configureSomething
val sentryVariant = if (isAGP74) null else AndroidVariant70(variant)
sentryVariant?.configureNativeSymbolsTask(
project,
extension,
sentryTelemetryProvider,
cliExecutable,
sentryOrg,
sentryProject
)

val additionalSourcesProvider = project.provider {
extension.additionalSourceDirsForSourceContext.getOrElse(emptySet())
.map { project.layout.projectDirectory.dir(it) }
}
val sourceFiles = sentryVariant?.sources(
project,
additionalSourcesProvider
)

val tasksGeneratingProperties = mutableListOf<TaskProvider<out PropertiesFileOutputTask>>()
val sourceContextTasks = variant.configureSourceBundleTasks(
project,
extension,
sentryTelemetryProvider,
sourceFiles,
cliExecutable,
sentryOrg,
sentryProject
Expand All @@ -97,17 +119,6 @@ fun AppExtension.configure(
)
generateProguardUuidTask?.let { tasksGeneratingProperties.add(it) }

// TODO: do this only once, and all other tasks should be SentryVariant.configureSomething
val sentryVariant = if (isAGP74) null else AndroidVariant70(variant)
sentryVariant?.configureNativeSymbolsTask(
project,
extension,
sentryTelemetryProvider,
cliExecutable,
sentryOrg,
sentryProject
)

variant.configureDebugMetaPropertiesTask(
project,
extension,
Expand Down Expand Up @@ -187,6 +198,7 @@ private fun ApplicationVariant.configureSourceBundleTasks(
project: Project,
extension: SentryPluginExtension,
sentryTelemetryProvider: Provider<SentryTelemetryService>,
sourceFiles: Provider<out Collection<Directory>>?,
cliExecutable: Provider<String>,
sentryOrg: String?,
sentryProject: String?
Expand All @@ -208,6 +220,7 @@ private fun ApplicationVariant.configureSourceBundleTasks(
sentryTelemetryProvider,
variant,
paths,
sourceFiles,
cliExecutable,
sentryOrg,
sentryProject,
Expand Down Expand Up @@ -289,6 +302,11 @@ private fun ApplicationVariant.configureProguardMappingsTasks(
extension,
sentryTelemetryProvider,
output = outputDir,
SentryTasksProvider.getMappingFileProvider(
project,
variant,
dexguardEnabled
),
taskSuffix = name.capitalized
)

Expand Down
2 changes: 1 addition & 1 deletion plugin-build/src/main/kotlin/io/sentry/android/gradle/sourcecontext/CollectSourcesTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ abstract class CollectSourcesTask : DirectoryOutputTask() {
project: Project,
extension: SentryPluginExtension,
sentryTelemetryProvider: Provider<SentryTelemetryService>?,
sourceDirs: Provider<out Collection<Directory>>,
sourceDirs: Provider<out Collection<Directory>>?,
output: Provider<Directory>,
includeSourceContext: Property<Boolean>,
taskSuffix: String = ""
Expand Down
12 changes: 11 additions & 1 deletion plugin-build/src/main/kotlin/io/sentry/android/gradle/sourcecontext/GenerateBundleIdTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,23 @@ import io.sentry.android.gradle.util.info
import java.util.Properties
import java.util.UUID
import org.gradle.api.Project
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.Directory
import org.gradle.api.file.RegularFile
import org.gradle.api.provider.Property
import org.gradle.api.provider.Provider
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.PathSensitive
import org.gradle.api.tasks.PathSensitivity
import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.TaskProvider

abstract class GenerateBundleIdTask : PropertiesFileOutputTask() {

init {
outputs.upToDateWhen { false }
// outputs.upToDateWhen { false }
description = "Generates a unique build ID to be used " +
"when bundling sources for upload to Sentry"

Expand All @@ -33,6 +37,10 @@ abstract class GenerateBundleIdTask : PropertiesFileOutputTask() {
@get:Input
abstract val includeSourceContext: Property<Boolean>

@get:PathSensitive(PathSensitivity.RELATIVE)
@get:InputFiles
abstract val sourceDirs: ConfigurableFileCollection

@get:Internal
override val outputFile: Provider<RegularFile> get() = output.file(SENTRY_BUNDLE_ID_OUTPUT)

Expand Down Expand Up @@ -64,6 +72,7 @@ abstract class GenerateBundleIdTask : PropertiesFileOutputTask() {
project: Project,
extension: SentryPluginExtension,
sentryTelemetryProvider: Provider<SentryTelemetryService>?,
sourceDirs: Provider<out Collection<Directory>>?,
output: Provider<Directory>? = null,
includeSourceContext: Property<Boolean>,
taskSuffix: String = ""
Expand All @@ -75,6 +84,7 @@ abstract class GenerateBundleIdTask : PropertiesFileOutputTask() {
output?.let { task.output.set(it) }
task.includeSourceContext.set(includeSourceContext)
task.withSentryTelemetry(extension, sentryTelemetryProvider)
task.sourceDirs.setFrom(sourceDirs)
}
return generateBundleIdTask
}
Expand Down
11 changes: 3 additions & 8 deletions plugin-build/src/main/kotlin/io/sentry/android/gradle/sourcecontext/SourceContext.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.sentry.android.gradle.extensions.SentryPluginExtension
import io.sentry.android.gradle.telemetry.SentryTelemetryService
import io.sentry.gradle.common.SentryVariant
import org.gradle.api.Project
import org.gradle.api.file.Directory
import org.gradle.api.provider.Provider
import org.gradle.api.tasks.TaskProvider

Expand All @@ -15,23 +16,17 @@ class SourceContext {
sentryTelemetryProvider: Provider<SentryTelemetryService>?,
variant: SentryVariant,
paths: OutputPaths,
sourceFiles: Provider<out Collection<Directory>>?,
cliExecutable: Provider<String>,
sentryOrg: String?,
sentryProject: String?,
taskSuffix: String
): SourceContextTasks {
val additionalSourcesProvider = project.provider {
extension.additionalSourceDirsForSourceContext.getOrElse(emptySet())
.map { project.layout.projectDirectory.dir(it) }
}
val sourceFiles = variant.sources(
project,
additionalSourcesProvider
)
val generateBundleIdTask = GenerateBundleIdTask.register(
project,
extension,
sentryTelemetryProvider,
sourceFiles,
output = paths.bundleIdDir,
extension.includeSourceContext,
taskSuffix
Expand Down
3 changes: 3 additions & 0 deletions ...in-build/src/main/kotlin/io/sentry/android/gradle/sourcecontext/UploadSourceBundleTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ abstract class UploadSourceBundleTask : Exec() {
includeSourceContext.getOrElse(false) &&
!sourceBundleDir.asFileTree.isEmpty
}

// Allows gradle to consider this task up-to-date if the inputs haven't changed
Copy link

Choose a reason for hiding this comment

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

👋 just a fly on the wall. I'm a sentry user and a stakeholder in issue #616, but I'm reasonably certain this is incorrect. See the JavaDoc for the upToDateWhen method: https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/TaskOutputs.html#upToDateWhen-groovy.lang.Closure-

Adds a predicate to determine whether previous outputs of this task can be reused. The given closure is executed at task execution time. The closure is passed the task as a parameter. If the closure returns false, previous outputs of this task cannot be reused and the task will be executed. That means the task is out-of-date and no outputs will be loaded from the build cache.

You can add multiple such predicates. The task outputs cannot be reused when any predicate returns false.

See this discussion: https://discuss.gradle.org/t/outputs-uptodatewhen-confusion/17973/3

If you read “false” as “not”, then…
outputs.upToDateWhen { true } means “outputs are up to date”.
outputs.upToDateWhen { false } means “outputs are not up to date”.

If any upToDateWhen spec returns false, the task is considered out of date. If they return true, Gradle does the normal behavior of checking input/output files.

This should be removed to avoid confusion, but I don't think it will negatively impact anything

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @ryandens,
Thanks for having a look.
This particular task does not have any outputs. Gradle does up-to-date checks based on a tasks inputs and outputs. If a task does not have any outputs it will never be considered up-to-date (found that in the gradle docs somewhere, but don't have it at hand right know).
Therefore I added the outputs.upToDateWhen closure to let gradle know that it should always consider the tasks output to be up-to-date and only run the task if its inputs change.

I will double-check though.

Hope this makes things a bit less confusing.

outputs.upToDateWhen { true }
}

@get:Input
Expand Down
3 changes: 2 additions & 1 deletion ...d/src/main/kotlin/io/sentry/android/gradle/tasks/SentryGenerateDebugMetaPropertiesTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ import org.gradle.api.file.RegularFile
import org.gradle.api.provider.Provider
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.PathSensitive
import org.gradle.api.tasks.PathSensitivity
import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.TaskProvider

abstract class SentryGenerateDebugMetaPropertiesTask : DirectoryOutputTask() {

init {
outputs.upToDateWhen { false }
description = "Combines multiple properties files into sentry-debug-meta.properties"
}

Expand Down
12 changes: 11 additions & 1 deletion ...in-build/src/main/kotlin/io/sentry/android/gradle/tasks/SentryGenerateProguardUuidTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,32 @@ import io.sentry.android.gradle.util.info
import java.util.Properties
import java.util.UUID
import org.gradle.api.Project
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.Directory
import org.gradle.api.file.FileCollection
import org.gradle.api.file.RegularFile
import org.gradle.api.provider.Provider
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.PathSensitive
import org.gradle.api.tasks.PathSensitivity
import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.TaskProvider

abstract class SentryGenerateProguardUuidTask : PropertiesFileOutputTask() {

init {
outputs.upToDateWhen { false }
description = "Generates a unique build ID to be used " +
"when uploading the Sentry mapping file"
}

@get:Internal
override val outputFile: Provider<RegularFile> get() = output.file(SENTRY_UUID_OUTPUT)

@get:PathSensitive(PathSensitivity.RELATIVE)
@get:InputFiles
abstract val sourceDirs: ConfigurableFileCollection

@TaskAction
fun generateProperties() {
val outputDir = output.get().asFile
Expand Down Expand Up @@ -54,6 +62,7 @@ abstract class SentryGenerateProguardUuidTask : PropertiesFileOutputTask() {
extension: SentryPluginExtension,
sentryTelemetryProvider: Provider<SentryTelemetryService>?,
output: Provider<Directory>? = null,
sourceFiles: Provider<FileCollection>?,
taskSuffix: String = ""
): TaskProvider<SentryGenerateProguardUuidTask> {
val generateUuidTask = project.tasks.register(
Expand All @@ -62,6 +71,7 @@ abstract class SentryGenerateProguardUuidTask : PropertiesFileOutputTask() {
) { task ->
output?.let { task.output.set(it) }
task.withSentryTelemetry(extension, sentryTelemetryProvider)
task.sourceDirs.setFrom(sourceFiles)
}
return generateUuidTask
}
Expand Down
10 changes: 10 additions & 0 deletions plugin-build/src/main/kotlin/io/sentry/jvm/gradle/SentryJvmPlugin.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,22 @@ class SentryJvmPlugin @Inject constructor(
buildEvents.onOperationCompletion(sentryTelemetryProvider)
}

val additionalSourcesProvider = project.provider {
extension.additionalSourceDirsForSourceContext.getOrElse(emptySet())
.map { project.layout.projectDirectory.dir(it) }
}
val sourceFiles = javaVariant.sources(
project,
additionalSourcesProvider
)

val sourceContextTasks = SourceContext.register(
project,
extension,
sentryTelemetryProvider,
javaVariant,
outputPaths,
sourceFiles,
cliExecutable,
sentryOrgParameter,
sentryProjectParameter,
Expand Down
2 changes: 2 additions & 0 deletions plugin-build/src/test/kotlin/io/sentry/android/gradle/tasks/GenerateBundleIdTaskTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class GenerateBundleIdTaskTest {
project,
project.extensions.findByName("sentry") as SentryPluginExtension,
null,
null,
project.layout.buildDirectory.dir("dummy/folder/"),
project.objects.property(Boolean::class.java).convention(true),
"test"
Expand All @@ -46,6 +47,7 @@ class GenerateBundleIdTaskTest {
project,
project.extensions.findByName("sentry") as SentryPluginExtension,
null,
null,
project.layout.buildDirectory.dir("dummy/folder/"),
project.objects.property(Boolean::class.java).convention(true),
"test"
Expand Down
Loading
Loading