Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Reversed annotations order in KAPT and KT-51087 #74

Closed
Jeffset opened this issue Sep 4, 2023 · 1 comment
Closed

Reversed annotations order in KAPT and KT-51087 #74

Jeffset opened this issue Sep 4, 2023 · 1 comment
Labels
backend:kapt Related to KAPT backend
Milestone
1.4.0

Comments

@Jeffset
Copy link
Contributor

Jeffset commented Sep 4, 2023

As https://youtrack.jetbrains.com/issue/KT-51087 was fixed in 1.8.20, and Yatagan unconditionally reverses annotations order for Kotlin elements, it's high time to fix that properly.

The proposed solution is to read Kotlin metadata version and base the decision to reverse annotations' order on that.

Generally, it's not a big issue, however in Yatagan we have poorly designed Conditions API (1.0, 2.0 is much better and is not affected by this issue) which requires annotation order to be well-defined.
@Jeffset Jeffset added the backend:kapt Related to KAPT backend label Sep 4, 2023
@Jeffset Jeffset added this to the 1.4.0 milestone Sep 4, 2023
Jeffset pushed a commit that referenced this issue Sep 4, 2023
Kapt: Properly fix annotation ordering issue.
fdaec42 
As per KT-51087, kapt generated annotations in
 reverse order, which was important for Yatagan.
This CL adds logic to reverse annotations based
 on kotlin metadata version check.

The ultimate reason for the fix was that
`room-compiler-processing-testing` in `2.6.0-beta01`
migrated to 1.9.x Kotlin compiler and some tests were broken.

Closes #74
@Jeffset Jeffset mentioned this issue Sep 4, 2023
Closed
@Jeffset
Copy link
Contributor Author

Jeffset commented Sep 5, 2023

As I mentioned in #75 (comment)
this task seems impossible to solve correctly after all.

The only thing we can do is to quickly deprecate old Condition API and move on with our lives.
The rest of the APIs are not affected by the annotation ordering issues.

@Jeffset Jeffset closed this as not planned Won't fix, can't repro, duplicate, stale Sep 5, 2023
Jeffset pushed a commit that referenced this issue Sep 5, 2023
Chore: Update room-compiler-processing-testing to beta01.
7cdec2f 
This upgrades Kotlin to 1.9 in compilation tests,
 KAPT's annotation reversing bug is fixed, needed changes.
Proper fix, which would be Kotlin version dependent is not possible,
 so just remove reversing unconditionally.

See #74 for more info.
@Jeffset Jeffset mentioned this issue Sep 10, 2023
Closed
Jeffset added a commit that referenced this issue Oct 21, 2023
@Jeffset
Testing: Update room-compiler-processing-testing to 2.6.0
9076bdc 
This update bumps the internal Kotlin version used to compile test cases to 1.9.0,
which has KAPT annotation reversing fixed (see #74 for more info)
So, as nice version-dependent solution is almost impossible,
 we just unconditionally use normal annotation order.
@Jeffset Jeffset mentioned this issue Oct 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backend:kapt Related to KAPT backend
Projects
None yet
Milestone
1.4.0
Development

No branches or pull requests
1 participant
@Jeffset