Restructure library group logic in buildSrc

[rlazo]

Library groups are now compute at the plugin, rather than being a singleton class using registrars. This fixes the previous bug that caused duplicate entries to be generated for the release generator because the singleton class was kept alive by the gradle daemon.

Additionally, it simplifies the verification of missing libraries due to library groups. Now, the build will fail if the release.json file does not include all the libraries in the library group.

[github-actions]

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

[github-actions]

Unit Test Results

   820 files  +   656     820 suites  +656   37m 59s ⏱️ + 35m 55s
5 008 tests +3 800  4 987 ✔️ +3 795  21 💤 +  5  0 ❌ ±0 
9 925 runs  +7 509  9 883 ✔️ +7 499  42 💤 +10  0 ❌ ±0 

Results for commit 6c68174. ± Comparison against base commit c5a894c.

♻️ This comment has been updated with latest results.

[google-oss-bot]

Coverage Report ¹

Affected Products

• firebase-messaging

  Overall coverage changed from 84.21% (c5a894c) to 84.10% (eecf3ec) by -0.11%.

  Filename	Base (c5a894c)	Merge (eecf3ec)	Diff
  Metadata.java	41.27%	36.51%	-4.76%

• firebase-firestore

  Filename	Base (c5a894c)	Merge (eecf3ec)	Diff
  PatchMutation.java	98.39%	100.00%	+1.61%
  SetMutation.java	97.22%	94.44%	-2.78%

Test Logs

• Base (c5a894c): https://github.com/firebase/firebase-android-sdk/actions/runs/6164473252
• Merge (eecf3ec): https://github.com/firebase/firebase-android-sdk/actions/runs/6177561141

1. https://storage.googleapis.com/firebase-sdk-metric-reports/W82vQxbGeN.html

[daymxn]

Left some comments on stuff that could be cleaned up- but nothing that should really prevent merging.

tbh, I very much dislike this way of fetching library groups, and would rather there be a more straightforward way instead of passing a function with the root project. although, this is something I don't think we could really solve until we get to migrating FirebaseLibraryExtension to Kotlin. and furthermore, we have more pressing matters- so deff a backlog issue.

All in all though, looking good!!

[rlazo]

Left some comments on stuff that could be cleaned up- but nothing that should really prevent merging.

tbh, I very much dislike this way of fetching library groups, and would rather there be a more straightforward way instead of passing a function with the root project. although, this is something I don't think we could really solve until we get to migrating FirebaseLibraryExtension to Kotlin. and furthermore, we have more pressing matters- so deff a backlog issue.

All in all though, looking good!!

Hey Daymon, curious to hear more about your ideas. When redesigning where the global library groups information should be, I discarted the FirebaseLibraryExtension since it has the view of a single Library, which is not enough information to compute a complete view of the whole universe of libraries and groups. It didn't seem to fit it as an extension function either, since we wouldn't be providing additionally functionality, but additional information, which wasn't there at creation time. Not a fan of that either.

I could have go with encapsulating "LibraryGroups" as a class, but wasn't fully convinced.

[daymxn]

Left some comments on stuff that could be cleaned up- but nothing that should really prevent merging.
tbh, I very much dislike this way of fetching library groups, and would rather there be a more straightforward way instead of passing a function with the root project. although, this is something I don't think we could really solve until we get to migrating FirebaseLibraryExtension to Kotlin. and furthermore, we have more pressing matters- so deff a backlog issue.
All in all though, looking good!!

Hey Daymon, curious to hear more about your ideas. When redesigning where the global library groups information should be, I discarted the FirebaseLibraryExtension since it has the view of a single Library, which is not enough information to compute a complete view of the whole universe of libraries and groups. It didn't seem to fit it as an extension function either, since we wouldn't be providing additionally functionality, but additional information, which wasn't there at creation time. Not a fan of that either.

I could have go with encapsulating "LibraryGroups" as a class, but wasn't fully convinced.

I agree. My main gripe here is the lack of global memoization (while also keeping things lazy)- something we could accomplish with a deferred property in FirebaseLibraryExtension- but it'd need to be written in Kotlin (so that it could be added directly to the class). You can't take advantage of delegates like that in extension properties unfortunately.

[rlazo]

Left some comments on stuff that could be cleaned up- but nothing that should really prevent merging.
tbh, I very much dislike this way of fetching library groups, and would rather there be a more straightforward way instead of passing a function with the root project. although, this is something I don't think we could really solve until we get to migrating FirebaseLibraryExtension to Kotlin. and furthermore, we have more pressing matters- so deff a backlog issue.
All in all though, looking good!!

Hey Daymon, curious to hear more about your ideas. When redesigning where the global library groups information should be, I discarted the FirebaseLibraryExtension since it has the view of a single Library, which is not enough information to compute a complete view of the whole universe of libraries and groups. It didn't seem to fit it as an extension function either, since we wouldn't be providing additionally functionality, but additional information, which wasn't there at creation time. Not a fan of that either.
I could have go with encapsulating "LibraryGroups" as a class, but wasn't fully convinced.

I agree. My main gripe here is the lack of global memoization (while also keeping things lazy)- something we could accomplish with a deferred property in FirebaseLibraryExtension- but it'd need to be written in Kotlin (so that it could be added directly to the class). You can't take advantage of delegates like that in extension properties unfortunately.

I still don't think that this info belongs to FirebaseLibraryExtension. I fail to see an scenario in which knowing about library groups is useful at a subproject level. That's why I think this should live at the plugin level

[daymxn]

I still don't think that this info belongs to FirebaseLibraryExtension. I fail to see an scenario in which knowing about library groups is useful at a subproject level. That's why I think this should live at the plugin level

Actually, about half of our LG usage happens at the subproject level. For example, both UpdatePinnedDependenciesTask and ReleaseGenerator get the sibling libraries on a per library basis.

[github-actions]

buildSrc Test Results

36 tests   36 ✔️  2m 6s ⏱️
  8 suites    0 💤
  8 files      0 ❌

Results for commit 6c68174.

♻️ This comment has been updated with latest results.

[rlazo]

I still don't think that this info belongs to FirebaseLibraryExtension. I fail to see an scenario in which knowing about library groups is useful at a subproject level. That's why I think this should live at the plugin level

Actually, about half of our LG usage happens at the subproject level. For example, both UpdatePinnedDependenciesTask and ReleaseGenerator get the sibling libraries on a per library basis.

I could be wrong, but I see tasks in two groups:

• Subproject level tasks, which are tasks that do not need to know anything about the world outside their suproject to work. Dackka is a good example, since generating the refdocs for one subproject doesn't depend on the others. What about referencing methos across SDKs? That's a dependency declared in the build file that could point to an artifact in maven.
• Root project level tasks, are tasks that require cross subproject knowledge to work. UpdatePinnedDependenciesTask needs to know whether two libraries connected by a project level dependency belong to the same library group, which is a global concept. Knowing your own library group tells you nothing about the other libs library group, you have to go and check. As for ReleaseGenerator, we can know at a subproject level whether a particular sdk should be included in the release on its own merit, but that's not enough because we can include them for other reasons. Also, the release generator has to accumulate which sdks to release to create a single config.

[google-oss-bot]

Size Report ¹

Affected Products

No changes between base commit (c5a894c) and merge commit (eecf3ec).

Test Logs

• Base (c5a894c): https://github.com/firebase/firebase-android-sdk/actions/runs/6164473252
• Merge (eecf3ec): https://github.com/firebase/firebase-android-sdk/actions/runs/6177561141

1. https://storage.googleapis.com/firebase-sdk-metric-reports/r575o985hh.html

[daymxn]

I still don't think that this info belongs to FirebaseLibraryExtension. I fail to see an scenario in which knowing about library groups is useful at a subproject level. That's why I think this should live at the plugin level

Actually, about half of our LG usage happens at the subproject level. For example, both UpdatePinnedDependenciesTask and ReleaseGenerator get the sibling libraries on a per library basis.

I could be wrong, but I see tasks in two groups:

• Subproject level tasks, which are tasks that do not need to know anything about the world outside their suproject to work. Dackka is a good example, since generating the refdocs for one subproject doesn't depend on the others. What about referencing methos across SDKs? That's a dependency declared in the build file that could point to an artifact in maven.
• Root project level tasks, are tasks that require cross subproject knowledge to work. UpdatePinnedDependenciesTask needs to know whether two libraries connected by a project level dependency belong to the same library group, which is a global concept. Knowing your own library group tells you nothing about the other libs library group, you have to go and check. As for ReleaseGenerator, we can know at a subproject level whether a particular sdk should be included in the release on its own merit, but that's not enough because we can include them for other reasons. Also, the release generator has to accumulate which sdks to release to create a single config.

Yeah that's a good point. Maybe just an easier way of fetching the data relevant to a given group then? Ideally, I think something like this would be the cleanest and most straightforward:

// straightforward, easy identification of point of truths; but a bit verbose
LibraryGroups.projectsForGroup(library.libraryGroup) 

or

library.findSiblings() // a list of libraries in the same group ("sibling" projects)

wdyt?

[daymxn]

Left some comments regarding alignment with other task designs and documentation, but otherwise looks good to me!

e: Also, make sure the smoke test failure isn't legit

[google-oss-bot]

	1 Warning
⚠️	Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger