Fix binary name computation for default-package classes in the KSP processor - #2128
Closed
rootkiller6788 wants to merge 2 commits into
Closed
Conversation
…ocessor
The KSP processor built binary names by assuming the qualified name always
starts with '<package>.'. For a class in the default package the package is
empty, so the length+1 offset skipped the first character of the class name
and the generated service file ended up with a leading dot and a truncated
name ('.oo' instead of 'Foo'). The APT processor already handles this by
returning the plain (nested) class name for the unnamed package; mirror that
here.
Each META-INF/services/<interface> file is built from every @autoservice provider of that interface in the module, not just from the sources listed on the call. With aggregating=false KSP won't regenerate the file when a new provider source is added to an incremental build, so the new provider is missing from the output. This matches how the generated resource files are also aggregated for KAPT.
cpovirk
approved these changes
Sep 8, 2026
cpovirk
left a comment
cpovirk
left a comment
Member
There was a problem hiding this comment.
Thank you. I will get some more informed eyes on this internally, but it seems self-evidently correct to me.
copybara-service Bot
pushed a commit
that referenced
this pull request
Sep 8, 2026
…ocessor The KSP processor was added a couple of weeks ago, and I found two things in it while reading it against the APT version. 1. Binary names for classes in the default package are wrong. getBinaryName always strips '<package>.' from the qualified name, but for a root-package class there is no such prefix, so it drops the first character and leaves a stray dot: a provider Foo ends up written as .oo in the service file. The Java AutoServiceProcessor handles the unnamed package explicitly (it just returns the class name), so the KSP side should do the same. This only bites people who put providers in the default package, which is rare, but the generated file is just wrong when it happens. 2. The generated META-INF/services/<interface> file is an aggregating output but is declared isolating (aggregating=false). Each file collects every @autoservice provider of that interface anywhere in the module, so a provider in a brand-new source file changes it. With aggregating=false, KSP won't invalidate the file when an unrelated new source is added, so on an incremental build a newly added provider never shows up in the service file. Setting aggregating=true tells KSP to regenerate it whenever there's new input. No test added: the KSP test harness (room3) needs a full KSP/Kotlin compile that I couldn't run in my environment, and the existing tests only cover packaged classes. Both changes are behaviour-preserving for the common packaged case. Fixes #2128 FUTURE_COPYBARA_INTEGRATE_REVIEW=#2128 from rootkiller6788:fix-autoservice-ksp-default-package-aggregating cf6ade9 PiperOrigin-RevId: 977860974
Member
|
Thanks! I feel like the documentation for the |
copybara-service Bot
pushed a commit
that referenced
this pull request
Sep 8, 2026
…KspProcessor`. #2128 fixed an issue with them but wasn't able to add any tests, so I'm adding them here. RELNOTES=n/a PiperOrigin-RevId: 977982261
copybara-service Bot
pushed a commit
that referenced
this pull request
Sep 8, 2026
…KspProcessor`. #2128 fixed an issue with them but wasn't able to add any tests, so I'm adding them here. RELNOTES=n/a PiperOrigin-RevId: 978010933
Member
|
Added the default package tests in #2130. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The KSP processor was added a couple of weeks ago, and I found two things in it while reading it against the APT version.
Binary names for classes in the default package are wrong. getBinaryName always strips '.' from the qualified name, but for a root-package class there is no such prefix, so it drops the first character and leaves a stray dot: a provider Foo ends up written as .oo in the service file. The Java AutoServiceProcessor handles the unnamed package explicitly (it just returns the class name), so the KSP side should do the same. This only bites people who put providers in the default package, which is rare, but the generated file is just wrong when it happens.
The generated META-INF/services/ file is an aggregating output but is declared isolating (aggregating=false). Each file collects every @autoservice provider of that interface anywhere in the module, so a provider in a brand-new source file changes it. With aggregating=false, KSP won't invalidate the file when an unrelated new source is added, so on an incremental build a newly added provider never shows up in the service file. Setting aggregating=true tells KSP to regenerate it whenever there's new input.
No test added: the KSP test harness (room3) needs a full KSP/Kotlin compile that I couldn't run in my environment, and the existing tests only cover packaged classes. Both changes are behaviour-preserving for the common packaged case.