milamberspace commented on code in PR #6846:
URL: https://github.com/apache/jmeter/pull/6846#discussion_r4415785885
##########
gradle.md:
##########
@@ -53,7 +53,8 @@ However, it might be useful to perform a "clean" build:
# Expected checksums are stored in /checksum.properties file
# Actual checksums are stored in
/build/checksum/computed.checksum.properties
- # Update expected dependencies after updating a dependency version
+ # Update src/dist/src/dist/expected_release_artifacts.txt after adding
or removing a dependency,
+ # or when the release outgrows total-size-limit. A version update that
keeps the same modules needs no change
Review Comment:
Since only the dist task reads the property, `gw
:src:dist:verifyReleaseDependencies -PupdateExpectedJars` is enough and much
faster than the full `check`. (nit)
##########
src/dist/build.gradle.kts:
##########
@@ -179,97 +178,132 @@ val populateLibs by tasks.registering {
}
}
-val updateExpectedJars by props(default =
gradle.startParameter.writeDependencyVerifications.isNotEmpty())
+val updateExpectedJars by props()
+
+// -PupdateExpectedJars sets total-size-limit to the current total size plus
this headroom,
+// so a dependency update can grow the release a little without touching the
expected file.
+val releaseJarsSizeHeadroom = 5L * 1024 * 1024
val verifyReleaseDependencies by tasks.registering {
- description = "Verifies if binary release archive contains the expected
set of external jars"
+ description = "Verifies the set of external modules and the total size of
external jars in the binary release"
group = LifecycleBasePlugin.VERIFICATION_GROUP
- dependsOn(configurations.runtimeClasspath)
- val expectedLibs = file("src/dist/expected_release_jars.csv")
- inputs.file(expectedLibs)
+ val expectedFile = file("src/dist/expected_release_artifacts.txt")
+ val projectPath = project.path
+ inputs.file(expectedFile)
+ inputs.files(configurations.runtimeClasspath)
inputs.property("updateExpectedJars", updateExpectedJars)
- val actualLibs =
layout.buildDirectory.file("dist/expected_release_jars.csv")
- outputs.file(actualLibs)
- val ignoreJarsMismatch = version.toString().endsWith("-SNAPSHOT")
- if (ignoreJarsMismatch || updateExpectedJars) {
- // The task does not fail in case of -SNAPSHOT version, so we make the
task never UP-TO-DATE
- // in that case. Otherwise, the task never executes on the second
request, even if the user runs with
- // -PupdateExpectedJars
+ val actualJars = layout.buildDirectory.file("dist/release_jars.csv")
+ outputs.file(actualJars)
+ if (updateExpectedJars) {
+ // Otherwise the update does nothing when the previous run passed with
the same inputs
outputs.upToDateWhen { false }
}
doLast {
- val caseInsensitive: Comparator<String> =
compareBy(String.CASE_INSENSITIVE_ORDER, { it })
-
- val deps =
configurations.runtimeClasspath.get().resolvedConfiguration.resolvedArtifacts
- val libs = deps.asSequence()
- .filter {
- val compId = it.id.componentIdentifier
- compId !is ProjectComponentIdentifier
- }
- .map { it.file.name to it.file.length() }
- .sortedWith(compareBy(caseInsensitive) { it.first })
- .associate { it }
-
- val expected = expectedLibs.readLines().asSequence()
- .filter { "," in it }
- .map {
- val (length, name) = it.split(",", limit = 2)
- name to length.toLong()
+ val caseInsensitive = String.CASE_INSENSITIVE_ORDER.thenBy { it }
+ val expectedPath = expectedFile.relativeTo(rootDir)
+ val actualFile = actualJars.get().asFile
+ val actualPath = actualFile.relativeTo(rootDir)
+
+ val artifacts =
configurations.runtimeClasspath.get().resolvedConfiguration.resolvedArtifacts
+ .filter { it.id.componentIdentifier !is ProjectComponentIdentifier
}
+ val jarsByModule = artifacts
+ .groupBy {
+ // Coordinates without the version, so a version update keeps
the expected file intact
+ val id = it.id.componentIdentifier as?
ModuleComponentIdentifier
+ ?: throw GradleException("${it.file.name} in
runtimeClasspath comes from ${it.id.componentIdentifier.displayName}, but only
modules from a repository and JMeter projects are expected")
+ buildString {
+ append(id.moduleIdentifier)
+ it.classifier?.let { classifier ->
append(':').append(classifier) }
+ if (it.extension != "jar") {
+ append('@').append(it.extension)
+ }
+ }
}
- .associate { it }
+ .toSortedMap(caseInsensitive)
+ val totalSize = artifacts.sumOf { it.file.length() }
- if (libs == expected) {
- return@doLast
+ actualFile.writeText(
+ artifacts.sortedWith(compareBy(caseInsensitive) { it.file.name })
+ .joinToString("\n", postfix = "\n") {
"${it.file.length()},${it.file.name}" }
+ )
+
+ // The expected file lists modules, not jars, so a second jar of a
listed module would go unnoticed
+ val ambiguous = jarsByModule.filterValues { it.size > 1 }
+ if (ambiguous.isNotEmpty()) {
+ throw GradleException(
+ "Each module in $expectedPath must resolve to a single jar,
but these resolve to several: " +
+ ambiguous.entries.joinToString("; ") { (module, files) ->
"$module (${files.joinToString { it.file.name }})" }
+ )
}
- val sb = StringBuilder()
- sb.append("External dependencies differ (you could update
${expectedLibs.relativeTo(rootDir)} if you run $path -PupdateExpectedJars):")
+ val sizeLimitPrefix = "total-size-limit="
+ val expectedLines = expectedFile.readLines()
+ .map { it.trim() }
+ .filter { it.isNotEmpty() && !it.startsWith("#") }
+ val existingSizeLimit = expectedLines.singleOrNull {
it.startsWith(sizeLimitPrefix) }
Review Comment:
`existingSizeLimit` is computed before the update branch but only used after
it; moving the lookup next to `val sizeLimit = existingSizeLimit ?: throw ...`
keeps the update path independent of the old file's contents. (nit)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
