Open jvican opened 6 months ago
@Arthurm1 what do you think? Is this good to merge? My Gradle knowledge is a bit limited these days
@tgodzik Makes sense and, except for my last comment just now, it looks good to me.
Could be merged as is, I just think the other getXXXConfigurationName()
methods are superfluous.
I guess it's possible that artifacts would not be exported where they were before but I can't think of a case where that would be an issue.
Thanks for confirming! @jvican would you mind removing them or is it something you found actually necessary?
Hey folks - let me follow up on this soon. The gradle experts at work have raised more concern around some bloop internals that are violating some soon-to-be-deprecated behavior in Gradle 9, and it seems we need a bit of a broader discussion aside from this PR. I'd like to get this merged at some point, but let's first get that discussion with the Gradle experts going and seek their advice. I'll open a bloop ticket and redirect them there so the discussion is open.
Some Gradle experts at $WORK have told me that the way we were previously resolving artifacts wasn't strictly correct due to two reasons:
Project references in project configurations (like the ones we resolve in
getConfigurationArtifacts
) are allowed to be empty for now, but that will change, and it's a bug. https://github.com/gradle/gradle/issues/26155The way we currently resolve artifacts doesn't let Gradle know our intention (as we can resolve any arbitrary configuration) and it doesn't play well with Gradle transforms, which require all of the resolved configuration artifacts to be present, otherwise it fails with a
FileNotFoundException
.As a result, I changed the code to introduce a Gradle configuration
bloopConfig
that extends all those configurations we care about, and change the resolution code to only depend on that configuration.It seems that solution works and passes all tests, and it avoids the
FileNotFoundException
error I found out.Because we now have a proper
configuration
, it means that users that create their own configuration and want it to be exported to bloop need to dobloopConfig.extendsFrom(myConfig)
wheremyConfig
is a user-defined configuration. The reason for this is because when we declare that our configuration extends all the other existing configuration, we do it at evaluation time. User-defined configurations run after the bloop plugin, so they never get added unless the user wishes to. This is not a big deal as custom configuration are extremely rare, and this is a common practice in Gradle land when you define your own configuration.