Move global transforms from META-INF/services to META-INF/groovy

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Move global transforms from META-INF/services to META-INF/groovy

paulk_asert

I plan to move the default location to look for org.codehaus.groovy.transform.ASTTransformation from META-INF/services to META-INF/groovy since the class(es) mentioned in that file aren't service(s) in the normal Java sense.

We already did this for valid source file extensions.

Obviously, the old location will be kept for backwards compatibility for some time.

See:

Cheers, Paul.

Reply | Threaded
Open this post in threaded view
|

Re: Move global transforms from META-INF/services to META-INF/groovy

Daniel.Sun
+1



-----
Daniel Sun
Apache Groovy committer
Blog: http://blog.sunlan.me 
Twitter: @daniel_sun

--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html
Daniel Sun
Apache Groovy committer

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

Re: Move global transforms from META-INF/services to META-INF/groovy

Guillaume Laforge
Administrator
Sounds good!

On Sat, Sep 1, 2018 at 5:21 AM Daniel.Sun <[hidden email]> wrote:
+1



-----
Daniel Sun
Apache Groovy committer
Blog: http://blog.sunlan.me
Twitter: @daniel_sun

--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html


--
Guillaume Laforge
Apache Groovy committer & PMC Vice-President
Developer Advocate @ Google Cloud Platform

Reply | Threaded
Open this post in threaded view
|

Re: Move global transforms from META-INF/services to META-INF/groovy

Jochen Theodorou
In reply to this post by paulk_asert
On 01.09.2018 03:20, Paul King wrote:
>
> I plan to move the default location to look for
> org.codehaus.groovy.transform.ASTTransformation from META-INF/services
> to META-INF/groovy since the class(es) mentioned in that file aren't
> service(s) in the normal Java sense.

basically good to conform with the Java9 understanding of what should be
in services and what not... BUT... if we think about Java9 modules then
this might not be good enough I am afraid. If we really want a library X
in a different module than Groovy itself, then X cannot expose
transformation information files by putting them in META-INF/groovy. We
will not have access to that. Instead we have to go the full SPI
approach here

bye Jochen
Reply | Threaded
Open this post in threaded view
|

Re: Move global transforms from META-INF/services to META-INF/groovy

Andres Almiray
I’d rather keep the files where they are unless they stand in the way for Java9+ integration.

Sent from my primitive Tricorder

> On 1 Sep 2018, at 11:05, Jochen Theodorou <[hidden email]> wrote:
>
>> On 01.09.2018 03:20, Paul King wrote:
>> I plan to move the default location to look for org.codehaus.groovy.transform.ASTTransformation from META-INF/services to META-INF/groovy since the class(es) mentioned in that file aren't service(s) in the normal Java sense.
>
> basically good to conform with the Java9 understanding of what should be in services and what not... BUT... if we think about Java9 modules then this might not be good enough I am afraid. If we really want a library X in a different module than Groovy itself, then X cannot expose transformation information files by putting them in META-INF/groovy. We will not have access to that. Instead we have to go the full SPI approach here
>
> bye Jochen
Reply | Threaded
Open this post in threaded view
|

Re: Move global transforms from META-INF/services to META-INF/groovy

paulk_asert
It's not our own JDK9+ integration that I am concerned with in the first instance. It's actually Maven and OSGi where our non-standard location is currently problematic which is behind my current desire for this change.
The most recent was just our own fix in our build where we would have to step around the non-uniform location when creating the right OSGi MANIFEST information, see GROOVY-8766 and the related proposed PR.
It will subsequently make life easier for us on our JDK9+ journey I suspect but as Jochen says, we might have to make additional changes there anyway.

On Sat, Sep 1, 2018 at 10:52 PM Andres Almiray <[hidden email]> wrote:
I’d rather keep the files where they are unless they stand in the way for Java9+ integration.

Sent from my primitive Tricorder

> On 1 Sep 2018, at 11:05, Jochen Theodorou <[hidden email]> wrote:
>
>> On 01.09.2018 03:20, Paul King wrote:
>> I plan to move the default location to look for org.codehaus.groovy.transform.ASTTransformation from META-INF/services to META-INF/groovy since the class(es) mentioned in that file aren't service(s) in the normal Java sense.
>
> basically good to conform with the Java9 understanding of what should be in services and what not... BUT... if we think about Java9 modules then this might not be good enough I am afraid. If we really want a library X in a different module than Groovy itself, then X cannot expose transformation information files by putting them in META-INF/groovy. We will not have access to that. Instead we have to go the full SPI approach here
>
> bye Jochen
Reply | Threaded
Open this post in threaded view
|

Re: Move global transforms from META-INF/services to META-INF/groovy

Andres Almiray
I see. Well. Given the short explanation found at GROOVY-8766 I’d say there is no need to move the files currently found at META-INF/services as OSGi is not the primary environment. 

This being said, additional manifest entries to make Groovy work better with OSGi are OK. 

What really causes trouble with ServiceLoader regardless of OSGi or not is the fact that we used META-INf/services for Groovy extension module definitions which do not adhere to the rules that all files under that directory must follow. We’ve discussed this topic before and I believe the agreement was to move only these files to a different location. 

AST xform files found under META-INF/services are indeed services, to the Groovy compiler. 

Cheers
Andres

Sent from my primitive Tricorder

On 1 Sep 2018, at 15:17, Paul King <[hidden email]> wrote:

It's not our own JDK9+ integration that I am concerned with in the first instance. It's actually Maven and OSGi where our non-standard location is currently problematic which is behind my current desire for this change.
The most recent was just our own fix in our build where we would have to step around the non-uniform location when creating the right OSGi MANIFEST information, see GROOVY-8766 and the related proposed PR.
It will subsequently make life easier for us on our JDK9+ journey I suspect but as Jochen says, we might have to make additional changes there anyway.

On Sat, Sep 1, 2018 at 10:52 PM Andres Almiray <[hidden email]> wrote:
I’d rather keep the files where they are unless they stand in the way for Java9+ integration.

Sent from my primitive Tricorder

> On 1 Sep 2018, at 11:05, Jochen Theodorou <[hidden email]> wrote:
>
>> On 01.09.2018 03:20, Paul King wrote:
>> I plan to move the default location to look for org.codehaus.groovy.transform.ASTTransformation from META-INF/services to META-INF/groovy since the class(es) mentioned in that file aren't service(s) in the normal Java sense.
>
> basically good to conform with the Java9 understanding of what should be in services and what not... BUT... if we think about Java9 modules then this might not be good enough I am afraid. If we really want a library X in a different module than Groovy itself, then X cannot expose transformation information files by putting them in META-INF/groovy. We will not have access to that. Instead we have to go the full SPI approach here
>
> bye Jochen
Reply | Threaded
Open this post in threaded view
|

Re: Move global transforms from META-INF/services to META-INF/groovy

paulk_asert
Okay, will leave things as they are for now. It will make the OSGi tweaks we do to the build somewhat uglier but we can live with that for now.

All other non-conforming files are already out of META-INF/services. These are the only files in that directory that we have that don't use a service loader.

I guess when we get back to re-working the JDK9+ changes we can look at this again if needed.

Cheers, Paul.

On Sun, Sep 2, 2018 at 1:12 AM Andres Almiray <[hidden email]> wrote:
I see. Well. Given the short explanation found at GROOVY-8766 I’d say there is no need to move the files currently found at META-INF/services as OSGi is not the primary environment. 

This being said, additional manifest entries to make Groovy work better with OSGi are OK. 

What really causes trouble with ServiceLoader regardless of OSGi or not is the fact that we used META-INf/services for Groovy extension module definitions which do not adhere to the rules that all files under that directory must follow. We’ve discussed this topic before and I believe the agreement was to move only these files to a different location. 

AST xform files found under META-INF/services are indeed services, to the Groovy compiler. 

Cheers
Andres

Sent from my primitive Tricorder

On 1 Sep 2018, at 15:17, Paul King <[hidden email]> wrote:

It's not our own JDK9+ integration that I am concerned with in the first instance. It's actually Maven and OSGi where our non-standard location is currently problematic which is behind my current desire for this change.
The most recent was just our own fix in our build where we would have to step around the non-uniform location when creating the right OSGi MANIFEST information, see GROOVY-8766 and the related proposed PR.
It will subsequently make life easier for us on our JDK9+ journey I suspect but as Jochen says, we might have to make additional changes there anyway.

On Sat, Sep 1, 2018 at 10:52 PM Andres Almiray <[hidden email]> wrote:
I’d rather keep the files where they are unless they stand in the way for Java9+ integration.

Sent from my primitive Tricorder

> On 1 Sep 2018, at 11:05, Jochen Theodorou <[hidden email]> wrote:
>
>> On 01.09.2018 03:20, Paul King wrote:
>> I plan to move the default location to look for org.codehaus.groovy.transform.ASTTransformation from META-INF/services to META-INF/groovy since the class(es) mentioned in that file aren't service(s) in the normal Java sense.
>
> basically good to conform with the Java9 understanding of what should be in services and what not... BUT... if we think about Java9 modules then this might not be good enough I am afraid. If we really want a library X in a different module than Groovy itself, then X cannot expose transformation information files by putting them in META-INF/groovy. We will not have access to that. Instead we have to go the full SPI approach here
>
> bye Jochen