Skip to content

BCV-MU requires access to ClassBinarySignature properties #178

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
aSemy opened this issue Jan 29, 2024 · 4 comments · Fixed by #188
Closed

BCV-MU requires access to ClassBinarySignature properties #178

aSemy opened this issue Jan 29, 2024 · 4 comments · Fixed by #188
Assignees

Comments

@aSemy
Copy link
Contributor

aSemy commented Jan 29, 2024

In version 0.14.0 several elements were marked as internal.

https://github.com/Kotlin/binary-compatibility-validator/pull/168/files#diff-8f30838a249b5077fe400072f0089da4daab28a873456622c54fdea121f9517fR13-R24

BCV-MU requires access to:

  • ClassBinarySignature
  • MEMBER_SORT_ORDER
  • MemberBinarySignature

for running the signature generator in a Gradle Worker.

https://github.com/adamko-dev/kotlin-binary-compatibility-validator-mu/blob/101ce0af1b66403ee2b7d32e7ad2b04e586c85cc/modules/bcv-gradle-plugin/src/main/kotlin/workers/BCVSignaturesWorker.kt#L115-L127

Please could these properties be made public again?

@fzhinkin
Copy link
Collaborator

fzhinkin commented Feb 5, 2024

@aSemy, first of all, sorry for the long time it took me to reply.

Looking at the affected BCV-MU's code, it seems like BCV's List<ClassBinarySignature>.dump extension function should be updated to sort class signatures by name. After that, both BCVSignaturesWorker::writeSignatures and KotlinApiBuildTask::generate could simply start using List<ClassBinarySignature>.dump instead of explicitly sorting and dumping signatures. In that case, no extra API should be exposed and BCV-MU continue to behave the same way it did.

On the BCV's side, it would look like this: https://github.com/Kotlin/binary-compatibility-validator/pull/188/files

Would that work for you?

@aSemy
Copy link
Contributor Author

aSemy commented Feb 5, 2024

Good idea! That sounds like a better solution. I'll experiment and see if your proposal is compatible with BCV-MU.

@JakeWharton
Copy link

I updated the task to use dump in #192

@fzhinkin
Copy link
Collaborator

Fixed in 0.15.0-Beta.1

# for free to join this conversation on GitHub. Already have an account? # to comment