-
Notifications
You must be signed in to change notification settings - Fork 461
Rework the Gradle attributes to make them integrate better with the ecosystem #4334
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.
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
base: master
Are you sure you want to change the base?
Conversation
… have to debug this
…martinbonnin/compatibility
f428119 to
3b56830
Compare
- Remove 'Bundling' from the 'Dokka JARs' attributes.
Dokka doesn't need to express an opinion on how the dependencies are distributed.
Fewer attributes means Gradle is less likely to get confused about the attributes.
- Remove modification of other configurations. It's risky, as other configurations might lazily add attributes. And with the removal of 'Bundling' it shouldn't be necessary.
- Refactoring to put things in better places.
- Update the tests.
- Register DokkaJavaRuntimeUsageCompatibilityRule in DokkaBasePlugin, so it's next to the other usage of `attributesSchema {}`, and it's always needed.
7b0393d to
bc6e40c
Compare
|
Wowowowow! That's a lot of tests 👀 |
|
Just in case you missed it on the copied PR. Before considering this PR, I'd like to suggest reviewing my analysis of the problem just posted to adamko-dev/dokkatoo#165 (comment) :-) Because I think the proper solution is not to do what is suggested here. |
I think we're good! There was a flaky test but it should be fixed now #4335 |
Thanks for the link. Unfortunately, it wouldn't help. It's not possible to modify the attributes of KGP Configurations. Additionally, any fix needs to work with older KGP versions (such as those bundled into older Gradle versions). Also, itt's not appropriate for DGP to use the KotlinPlatformType, which is only relevant for KMP. Adding it won't help with non-KMP consumers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in the PR are a bit out of my league, so you could safely ignore my review, and approval from the KBT side will be enough :)
In any case, thanks for the long description of what DokkaJavaRuntimeUsageCompatibilityRule is!
Continuation of #4286
Fixes KT-72019 and adamko-dev/dokkatoo#165
Credit goes to @martinbonnin!
Summary
Unfortunately, Gradle's matching algorithm prioritises the number of attributes matched over the actual attribute values. (Note that if a candidate does not have a requested attribute this still counts as a match!) This behaviour makes it impossible to strictly match Configurations, and leads to situations like this where two 'ecosystems' step on each other's toes.
The solution is in two parts:
Reduce the number of attributes DGP uses.
This prevents the matching algorithm from over-matching based on attributes alone. The Bundling attribute is removed, since this is not important for DGP classpaths.
(As suggested by @martinbonnin) DGP configurations use a custom value for the
Usageattribute, and use a one-wayAttributeCompatibilityRuleso DGP can still resolve dependencies.(A one-way rule should minimise ugly issues where rules can leak.)
See the KDoc of
DokkaJavaRuntimeUsageCompatibilityRulefor details.