Skip to content

Commit f98c035

Browse files
daymxndaymxn
authored
Fix ModuleVersion bumping (#6679)
Per [b/394908865](https://b.corp.google.com/issues/394908865), This fixes an issue where `ModuleVersion.bump()` was not properly resetting the smaller version types. Additionally, this fixes some other minor issues with bom generation. Namely, this PR also fixes: - [b/394908773](https://b.corp.google.com/issues/394908773) -> Fix bom release note ordering - [b/394909103](https://b.corp.google.com/issues/394909103) -> Separate published bom artifacts
1 parent b8803fc commit f98c035

File tree

10 files changed

+176
-38
lines changed

10 files changed

+176
-38
lines changed

‎.github/workflows/make-bom.yml‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ jobs:
1111
uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3
1212
with:
1313
python-version: '3.10'
14+
1415
- uses: actions/[email protected]
16+
1517
- name: Set up JDK 17
1618
uses: actions/[email protected]
1719
with:
@@ -21,19 +23,25 @@ jobs:
2123

2224
- name: Build
2325
run: |
24-
./ci/run.sh \
25-
--artifact-target-dir=./logs/artifacts \
26-
--artifact-patterns=bom.zip \
27-
--artifact-patterns=bomReleaseNotes.md \
28-
--artifact-patterns=recipeVersionUpdate.txt \
29-
gradle \
30-
-- \
31-
--build-cache \
32-
buildBomZip
33-
34-
- name: Upload generated artifacts
26+
./gradlew buildBomBundleZip
27+
28+
- name: Upload bom
29+
uses: actions/[email protected]
30+
with:
31+
name: bom
32+
path: build/bom/
33+
retention-days: 15
34+
35+
- name: Upload release notes
36+
uses: actions/[email protected]
37+
with:
38+
name: bom_release_notes
39+
path: build/bomReleaseNotes.md
40+
retention-days: 15
41+
42+
- name: Upload recipe version update
3543
uses: actions/[email protected]
3644
with:
37-
name: artifacts
38-
path: ./logs/artifacts/
39-
retention-days: 5
45+
name: recipe_version
46+
path: build/recipeVersionUpdate.txt
47+
retention-days: 15

‎plugins/src/main/java/com/google/firebase/gradle/bomgenerator/GenerateBomReleaseNotesTask.kt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ abstract class GenerateBomReleaseNotesTask : DefaultTask() {
5151
val previousDeps = previousBom.get().dependencyManagement?.dependencies.orEmpty()
5252
previousBomVersions.set(previousDeps.associate { it.fullArtifactName to it.version })
5353

54-
val sortedDependencies = currentDeps.sortedBy { it.version }
54+
val sortedDependencies = currentDeps.sortedBy { it.toString() }
5555

5656
val headingId = "{: #bom_v${bom.version.replace(".", "-")}}"
5757

@@ -71,8 +71,9 @@ abstract class GenerateBomReleaseNotesTask : DefaultTask() {
7171
| Firebase Android SDKs mapped to this {{bom}} version
7272
| </p>
7373
| <p>
74-
| Libraries that were versioned with this release are in highlighted rows.<br>
75-
| Refer to a library's release notes (on this page) for details about its changes.
74+
| Libraries that were versioned with this release are in highlighted rows.
75+
| <br>Refer to a library's release notes (on this page) for details about its
76+
| changes.
7677
| </p>
7778
| <table>
7879
| <thead>

‎plugins/src/main/java/com/google/firebase/gradle/bomgenerator/GenerateBomTask.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import com.google.firebase.gradle.plugins.datamodels.LicenseElement
2525
import com.google.firebase.gradle.plugins.datamodels.PomElement
2626
import com.google.firebase.gradle.plugins.datamodels.fullArtifactName
2727
import com.google.firebase.gradle.plugins.datamodels.moduleVersion
28-
import com.google.firebase.gradle.plugins.diff
2928
import com.google.firebase.gradle.plugins.orEmpty
29+
import com.google.firebase.gradle.plugins.pairBy
3030
import com.google.firebase.gradle.plugins.partitionNotNull
3131
import com.google.firebase.gradle.plugins.services.GMavenService
3232
import org.gradle.api.DefaultTask
@@ -144,7 +144,7 @@ abstract class GenerateBomTask : DefaultTask() {
144144
val oldBomVersion = ModuleVersion.fromString(oldBom.artifactId, oldBom.version)
145145

146146
val oldBomDependencies = oldBom.dependencyManagement?.dependencies.orEmpty()
147-
val changedDependencies = oldBomDependencies.diff(releasingDependencies)
147+
val changedDependencies = oldBomDependencies.pairBy(releasingDependencies) { it.artifactId }
148148

149149
val versionBumps =
150150
changedDependencies.mapNotNull { (old, new) ->

‎plugins/src/main/java/com/google/firebase/gradle/plugins/KotlinExtensions.kt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,43 @@ infix fun <T> List<T>.diff(other: List<T>): List<Pair<T?, T?>> {
296296
return firstList.zip(secondList).filter { it.first != it.second }
297297
}
298298

299+
/**
300+
* Creates a list of pairs between two lists, matching according to the provided [mapper].
301+
*
302+
* ```kotlin
303+
* data class Person(name: String, age: Int)
304+
*
305+
* val firstList = listOf(
306+
* Person("Mike", 5),
307+
* Person("Rachel", 6)
308+
* )
309+
*
310+
* val secondList = listOf(
311+
* Person("Michael", 4),
312+
* Person("Mike", 1)
313+
* )
314+
*
315+
* val diffList = firstList.pairBy(secondList) {
316+
* it.name
317+
* }
318+
*
319+
* diffList shouldBeEqualTo listOf(
320+
* Person("Mike", 5) to Person("Mike", 1)
321+
* Person("Rachel", 6) to null
322+
* null to Person("Mike", 1)
323+
* )
324+
* ```
325+
*/
326+
inline fun <T, R> List<T>.pairBy(other: List<T>, mapper: (T) -> R): List<Pair<T?, T?>> {
327+
val firstMap = associateBy { mapper(it) }
328+
val secondMap = other.associateBy { mapper(it) }
329+
330+
val changedOrRemoved = firstMap.map { it.value to secondMap[it.key] }
331+
val added = secondMap.filterKeys { it !in firstMap }.map { null to it.value }
332+
333+
return changedOrRemoved + added
334+
}
335+
299336
/**
300337
* Creates a list that is forced to certain size.
301338
*

‎plugins/src/main/java/com/google/firebase/gradle/plugins/ModuleVersion.kt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,10 @@ data class ModuleVersion(
269269
.let { it ?: if (pre != null) VersionType.PRE else VersionType.PATCH }
270270
.let {
271271
when (it) {
272-
VersionType.MAJOR -> copy(major = major + 1)
273-
VersionType.MINOR -> copy(minor = minor + 1)
274-
VersionType.PATCH -> copy(patch = patch + 1)
272+
VersionType.MAJOR ->
273+
copy(major = major + 1, minor = 0, patch = 0, pre = pre?.copy(build = 1))
274+
VersionType.MINOR -> copy(minor = minor + 1, patch = 0, pre = pre?.copy(build = 1))
275+
VersionType.PATCH -> copy(patch = patch + 1, pre = pre?.copy(build = 1))
275276
VersionType.PRE -> copy(pre = pre?.bump())
276277
}
277278
}

‎plugins/src/main/java/com/google/firebase/gradle/plugins/PublishingPlugin.kt‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ import org.gradle.kotlin.dsl.register
6161
* outside of the standard [FIREBASE_PUBLISH_TASK] workflow (possibly at a later time in the release
6262
* cycle):
6363
* - [BUILD_BOM_ZIP_TASK] -> Creates a zip file of the contents of [GENERATE_BOM_TASK]
64+
* [registerGenerateBomTask]
65+
* - [BUILD_BOM_BUNDLE_ZIP_TASK] -> Creates a zip file of the contents of [BUILD_BOM_ZIP_TASK]
6466
* [registerGenerateBomTask],
6567
* [GENERATE_BOM_RELEASE_NOTES_TASK][registerGenerateBomReleaseNotesTask] and
6668
* [GENERATE_TUTORIAL_BUNDLE_TASK][registerGenerateTutorialBundleTask]
@@ -140,9 +142,16 @@ abstract class PublishingPlugin : Plugin<Project> {
140142
destinationDirectory.set(project.layout.buildDirectory)
141143
}
142144

143-
project.tasks.register<Zip>(BUILD_BOM_ZIP_TASK) {
144-
from(generateBom, generateBomReleaseNotes, generateTutorialBundle)
145-
archiveFileName.set("bom.zip")
145+
val buildBomZip =
146+
project.tasks.register<Zip>(BUILD_BOM_ZIP_TASK) {
147+
from(generateBom)
148+
archiveFileName.set("bom.zip")
149+
destinationDirectory.set(project.layout.buildDirectory)
150+
}
151+
152+
project.tasks.register<Zip>(BUILD_BOM_BUNDLE_ZIP_TASK) {
153+
from(buildBomZip, generateBomReleaseNotes, generateTutorialBundle)
154+
archiveFileName.set("bomBundle.zip")
146155
destinationDirectory.set(project.layout.projectDirectory)
147156
}
148157

@@ -757,6 +766,7 @@ abstract class PublishingPlugin : Plugin<Project> {
757766
const val BUILD_KOTLINDOC_ZIP_TASK = "buildKotlindocZip"
758767
const val BUILD_RELEASE_NOTES_ZIP_TASK = "buildReleaseNotesZip"
759768
const val BUILD_BOM_ZIP_TASK = "buildBomZip"
769+
const val BUILD_BOM_BUNDLE_ZIP_TASK = "buildBomBundleZip"
760770
const val FIREBASE_PUBLISH_TASK = "firebasePublish"
761771
const val PUBLISH_ALL_TO_BUILD_TASK = "publishAllToBuildDir"
762772

‎plugins/src/main/java/com/google/firebase/gradle/plugins/datamodels/PomElement.kt‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ data class PomElement(
158158
@XmlElement val artifactId: String,
159159
@XmlElement val version: String,
160160
@XmlElement val packaging: String? = null,
161-
@XmlChildrenName("licenses") val licenses: List<LicenseElement>? = null,
161+
@XmlChildrenName("license") val licenses: List<LicenseElement>? = null,
162162
@XmlElement val scm: SourceControlManagement? = null,
163163
@XmlElement val dependencyManagement: DependencyManagementElement? = null,
164164
@XmlChildrenName("dependency") val dependencies: List<ArtifactDependency>? = null,
@@ -171,15 +171,25 @@ data class PomElement(
171171
* @see fromFile
172172
*/
173173
fun toFile(file: File): File {
174-
val xmlWriter = XML {
175-
indent = 2
176-
xmlDeclMode = XmlDeclMode.None
177-
}
178-
file.writeText(xmlWriter.encodeToString(this))
174+
file.writeText(toString())
179175
return file
180176
}
181177

178+
/**
179+
* Serializes this pom element into a valid XML element.
180+
*
181+
* @see toFile
182+
*/
183+
override fun toString(): String {
184+
return xml.encodeToString(this)
185+
}
186+
182187
companion object {
188+
private val xml = XML {
189+
indent = 2
190+
xmlDeclMode = XmlDeclMode.None
191+
}
192+
183193
/**
184194
* Deserializes a [PomElement] from a `pom.xml` file.
185195
*
@@ -201,6 +211,6 @@ data class PomElement(
201211
* @see fromFile
202212
*/
203213
fun fromElement(element: Element): PomElement =
204-
XML.decodeFromReader(xmlStreaming.newReader(element))
214+
xml.decodeFromReader(xmlStreaming.newReader(element))
205215
}
206216
}

‎plugins/src/test/kotlin/com/google/firebase/gradle/plugins/GenerateBomReleaseNotesTests.kt‎

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,68 @@ class GenerateBomReleaseNotesTests : FunSpec() {
6565
Firebase Android SDKs mapped to this {{bom}} version
6666
</p>
6767
<p>
68-
Libraries that were versioned with this release are in highlighted rows.<br>
69-
Refer to a library's release notes (on this page) for details about its changes.
68+
Libraries that were versioned with this release are in highlighted rows.
69+
<br>Refer to a library's release notes (on this page) for details about its
70+
changes.
71+
</p>
72+
<table>
73+
<thead>
74+
<th>Artifact name</th>
75+
<th>Version mapped<br>to previous {{bom}} v1.0.0</th>
76+
<th>Version mapped<br>to this {{bom}} v1.0.0</th>
77+
</thead>
78+
<tbody>
79+
<tr>
80+
<td>com.google.firebase:firebase-auth</td>
81+
<td>10.0.0</td>
82+
<td>10.0.0</td>
83+
</tr>
84+
<tr>
85+
<td>com.google.firebase:firebase-firestore</td>
86+
<td>10.0.0</td>
87+
<td>10.0.0</td>
88+
</tr>
89+
</tbody>
90+
</table>
91+
</section>
92+
"""
93+
.trimIndent()
94+
}
95+
96+
@Test
97+
fun `sorts the entries alphabetically`() {
98+
val dependencies =
99+
listOf(
100+
ArtifactDependency(
101+
groupId = "com.google.firebase",
102+
artifactId = "firebase-firestore",
103+
version = "10.0.0",
104+
),
105+
ArtifactDependency(
106+
groupId = "com.google.firebase",
107+
artifactId = "firebase-auth",
108+
version = "10.0.0",
109+
),
110+
)
111+
val bom = makeBom("1.0.0", dependencies)
112+
val file = makeReleaseNotes(bom, bom)
113+
114+
file.readText().trim() shouldBeText
115+
"""
116+
### {{firebase_bom_long}} ({{bill_of_materials}}) version 1.0.0 {: #bom_v1-0-0}
117+
{% comment %}
118+
These library versions must be flat-typed, do not use variables.
119+
The release note for this BoM version is a library-version snapshot.
120+
{% endcomment %}
121+
122+
<section class="expandable">
123+
<p class="showalways">
124+
Firebase Android SDKs mapped to this {{bom}} version
125+
</p>
126+
<p>
127+
Libraries that were versioned with this release are in highlighted rows.
128+
<br>Refer to a library's release notes (on this page) for details about its
129+
changes.
70130
</p>
71131
<table>
72132
<thead>
@@ -147,8 +207,9 @@ class GenerateBomReleaseNotesTests : FunSpec() {
147207
Firebase Android SDKs mapped to this {{bom}} version
148208
</p>
149209
<p>
150-
Libraries that were versioned with this release are in highlighted rows.<br>
151-
Refer to a library's release notes (on this page) for details about its changes.
210+
Libraries that were versioned with this release are in highlighted rows.
211+
<br>Refer to a library's release notes (on this page) for details about its
212+
changes.
152213
</p>
153214
<table>
154215
<thead>

‎plugins/src/test/kotlin/com/google/firebase/gradle/plugins/GenerateBomTests.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,11 +243,11 @@ class GenerateBomTests : FunSpec() {
243243
<version>1.0.1</version>
244244
<packaging>pom</packaging>
245245
<licenses>
246-
<licenses>
246+
<license>
247247
<name>The Apache Software License, Version 2.0</name>
248248
<url>http://www.apache.org/licenses/LICENSE-2.0.txt</url>
249249
<distribution>repo</distribution>
250-
</licenses>
250+
</license>
251251
</licenses>
252252
<dependencyManagement>
253253
<dependencies>

‎plugins/src/test/kotlin/com/google/firebase/gradle/plugins/ModuleVersionTests.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,16 @@ class ModuleVersionTests : FunSpec() {
9595
ModuleVersion(1, 1, 1).apply { bump(PRE) shouldBe this }
9696
}
9797

98+
@Test
99+
fun `Bump resets the smaller version types`() {
100+
val version = ModuleVersion(1, 1, 1, PreReleaseVersion(ALPHA, 2))
101+
102+
version.bump(PRE) shouldBe ModuleVersion(1, 1, 1, PreReleaseVersion(ALPHA, 3))
103+
version.bump(PATCH) shouldBe ModuleVersion(1, 1, 2, PreReleaseVersion(ALPHA, 1))
104+
version.bump(MINOR) shouldBe ModuleVersion(1, 2, 0, PreReleaseVersion(ALPHA, 1))
105+
version.bump(MAJOR) shouldBe ModuleVersion(2, 0, 0, PreReleaseVersion(ALPHA, 1))
106+
}
107+
98108
@Test
99109
fun `Bump correctly chooses the smallest by default`() {
100110
ModuleVersion(1, 1, 1).bump().patch shouldBe 2

0 commit comments

Comments
 (0)