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.

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

Adds new SPI Annotation #622

Closed
wants to merge 3 commits into from
Closed

Conversation

SentryMan
Copy link
Collaborator

Trying to auto-register services with avaje-spi is a dead end. So I suppose we need to make spi annotation for the generators.

@SentryMan SentryMan self-assigned this Jun 24, 2024
@SentryMan SentryMan added the enhancement New feature or request label Jun 24, 2024
@SentryMan SentryMan added this to the 10.0 milestone Jun 24, 2024
@SentryMan
Copy link
Collaborator Author

Feel free to change the names of the annotations to something else

@rob-bygrave
Copy link
Contributor

Trying to auto-register services with avaje-spi is a dead end.

Why? What has gone wrong?

@SentryMan
Copy link
Collaborator Author

Trying to auto-register services with avaje-spi is a dead end.

Why? What has gone wrong?

I thought it was good too, but it has a fatal flaw in that if multiple annotation processors try to use this, not all of the processor's spi interfaces will get registered. The reason is that the spi processor must be the last processor to execute after processing is over.

Trying to wrangle the apt order causes more work, not less.

@rob-bygrave
Copy link
Contributor

So I suppose we need to make spi annotation for the generators.

Or not? In that there are several options to consider now right?

What we know is that we need the inject-generator AP to write it's META-INF/services and not delegate that to avaje-spi-service (due to unknown ordering of the APs). Now if inject-generator AP appends to META-INF/services (and does not override) then this allows traditional manual registration of any InjectExtension. Does that not work?

@SentryMan
Copy link
Collaborator Author

SentryMan commented Jun 25, 2024

Does that not work?

that's what #621 is for. This one is because I don't like manually writing services and avaje-spi can't be used to automate this

@rbygrave
Copy link
Contributor

that's what #621 is for.

Yes, got that now. I saw the emails and saw the "Trying to auto-register services with avaje-spi is a dead end." ... so I came here first (so yeah I missed the previous part of the story there).

@SentryMan
Copy link
Collaborator Author

hmm, an alternate strategy would be to annotation process the avaje-spi @ServiceProvider and modify avaje-spi-service such that it'll abort writing any of the spis claimed by our generators

@rob-bygrave
Copy link
Contributor

It is generally the case for avaje-inject that the only folks registering a service will be InjectPlugin authors right? Like there might be better cases like avaje-validation but its not got good bang for buck here for avaje-inject as I see it.

@SentryMan
Copy link
Collaborator Author

It is generally the case for avaje-inject that the only folks registering a service will be InjectPlugin authors right?

Even so, writing services is tedious. In any case, don't forget those who may want to use the Property Plugin spi or fiddle with custom module ordering logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants