jakartaee / rest

Jakarta RESTful Web Services
Other
353 stars 114 forks source link

Rename the TCK packages #1082

Closed jamezp closed 2 years ago

jamezp commented 2 years ago

Resolves #1081

I'm creating this as a draft since we don't really know what the package name should be. This would be my personal vote though.

alwin-joseph commented 2 years ago

Just to clarify that the current changes in this PR will not be enough for package rename. Once the package name is confirmed I am sure below will also be taken care of.

jamezp commented 2 years ago

@alwin-joseph You're correct. I assumed my IDE was smarter than it was :) I'll see if I can get this fixed with a script or something.

jamezp commented 2 years ago

@alwin-joseph I think I've got it all fixed now. I did see some TCK failures for SeBootstrapIT, however they don't seem related.

There is some other weirdness with the jersey-tck too. It's got a dependency on jakarta.ws.rs:jakarta.ws.rs-tck. The "jaxrs-tck" project has an artifact id of <artifactId>${tck.artifactId}</artifactId>. The tck.artifactId property is set to <tck.artifactId>jakarta-restful-ws-tck</tck.artifactId> which seems very strange.

There is also a tckbundle.sh which overrides the property too with mvn clean install -Dtck.artifactId=restful-ws-tck. It seems there should be some consistency there and likely not use a property at all.

alwin-joseph commented 2 years ago

@alwin-joseph I think I've got it all fixed now. I did see some TCK failures for SeBootstrapIT, however they don't seem related.

Thanks!

There is some other weirdness with the jersey-tck too. It's got a dependency on jakarta.ws.rs:jakarta.ws.rs-tck. The "jaxrs-tck" project has an artifact id of <artifactId>${tck.artifactId}</artifactId>. The tck.artifactId property is set to <tck.artifactId>jakarta-restful-ws-tck</tck.artifactId> which seems very strange.

Agree. I see it now. It was missed from PR https://github.com/eclipse-ee4j/jaxrs-api/pull/1076. The jersey-tck/pom.xml need to have the same property jakarta-restful-ws-tck</tck.artifactId> set as default and use the same for tck name instead of jakarta.ws.rs-tck. The tck name was kept in consistent to the previous releases of the REST TCK. I will create a separate PR to correct this.

There is also a tckbundle.sh which overrides the property too with mvn clean install -Dtck.artifactId=restful-ws-tck. It seems there should be some consistency there and likely not use a property at all.

We need 2 TCK bundles differentiated by the LICENSE (EFTL or EPL) file used in them. The EFTL TCK bundle will be named as jakarta-restful-ws-tck.zip and EPL TCK bundle will be named as restful-ws-tck.zip. I was expecting to generate and run the EFTL bundle by default which will be used for ballot review and certification. The EPL TCK bundle will also need to be generated on a need basis hence the property override.

alwin-joseph commented 2 years ago

@alwin-joseph I think I've got it all fixed now. I did see some TCK failures for SeBootstrapIT, however they don't seem related.

Thanks!

I was able to test this change. It works and passes the tests. There were 2 failures for SeBootstrapIT but it was unrelated to this change and was fixed in local by an environment change.

There is some other weirdness with the jersey-tck too. It's got a dependency on jakarta.ws.rs:jakarta.ws.rs-tck. The "jaxrs-tck" project has an artifact id of <artifactId>${tck.artifactId}</artifactId>. The tck.artifactId property is set to <tck.artifactId>jakarta-restful-ws-tck</tck.artifactId> which seems very strange.

Agree. I see it now. It was missed from PR #1076. The jersey-tck/pom.xml need to have the same property jakarta-restful-ws-tck</tck.artifactId> set as default and use the same for tck name instead of jakarta.ws.rs-tck. The tck name was kept in consistent to the previous releases of the REST TCK. I will create a separate PR to correct this.

Created PR https://github.com/eclipse-ee4j/jaxrs-api/pull/1087 to fix this .

jamezp commented 2 years ago

Excellent. Thanks @alwin-joseph.

jamezp commented 2 years ago

Yes this is just renaming the packages as part of the spec list emails. It doesn't seem there is a concrete decision yet, however in the vote ee.jakarta.tck.[spec] "won". I don't personally have a strong opinion either way. I just figured if it's going to be required we should prepare for it :)

spericas commented 2 years ago

I'm not totally convinced about the proposed package name, but it seems a renaming is better than no renaming at this time, so I'm approving this PR.

jansupol commented 2 years ago

I'm not totally convinced about the proposed package name, but it seems a renaming is better than no renaming at this time, so I'm approving this PR.

This package name seems to be a must-have for EE 11

mkarg commented 2 years ago

I'm not totally convinced about the proposed package name, but it seems a renaming is better than no renaming at this time, so I'm approving this PR.

This package name seems to be a must-have for EE 11

This discussion is still ongoing. The decision based on too many people abstaining from voting and iJUG is planning to repeat the voting process. Chances are good that the this decision will be changed. So it would be wise to stick with our current package as-is.

spericas commented 2 years ago

-1 as there is no official requirement for this change so far, and "ee" is not following the Java package best practices. The iJUG currently is triggering a repeating of the vote to prevent that package renaming.

Any sort of ETA on this?

mkarg commented 2 years ago

-1 as there is no official requirement for this change so far, and "ee" is not following the Java package best practices. The iJUG currently is triggering a repeating of the vote to prevent that package renaming.

Any sort of ETA on this?

TL;DR: I think one or two weeks, depending on the outcome of the next spec and steering meetings. As vice-embassador of iJUG at the Eclipse Foundation I am bound to the opinion of iJUG here. If we change the rename from "ee.jakarta." to "org.eclipse.jakarta." I immediately will change my vote from -1 to 0.

Background: Just got informed that heavy discussions still going on offline, so currently Eclipse's Jakarta EE Developer Advocate Ivar Grimstad strives to find a solution. The problem is the many abstained votes, which does not provide a future-proof foundation for such a massive cross-project package renaming (the fear is that in future those abstained votes understand the impact they helped to create and will ask for another package rename in EE11 or EE12 then). I will post updates here once I hear about them.

For the time being, the current official status is that it is not any problem for the TCK tests themselves to stick with the jakarta namespace as is (i. e. jakarta.ws.rs.tck.), but not application code (i. e. everything that the TCK contains that is actually to be executed by or within a tested JAX-RS implementation. The problem is that some application servers reject such code. We are officially free to choose any package name for that code, as long as it does not start with "jakarta." due to bugs in their api package detection filters (this is the point unter discussion - Ivar and iJUG think this is a "bug" in the implementation as package names are not hierarchical, but those application vendors think this is "common sense" and reject to "fix" that). We officially may use the "ee.jakarta." package, but iJUG assumes this opens legal problems as Eclipse does not own the ".ee" TLD: If package names are hierarchical, then "ee.jakarta" is not covered by legal protection as the TLD is "ee" not "jakarta" but "ee" cannot be protected as a brand; if package names are not* hierarchical, then "ee.jakarta" is not a problem - but then no rename is needed at all. The clarification of both question will need one to two weeks.

So until both questions are officially and finally resolved, I would agree to renaming just the application code to something other than "ee.", keep the other TCK code as-is, e. g. "org.eclipse.jakarta.ws.rs.tck.". We even could even rename the TCK completely to that package. OTOH the outcome of the discussions could be that the steering committee decides for a completely different package which is mandatory for us all, then we have to rename again...

ivargrimstad commented 2 years ago

Greetings Jakarta RESTful Web Services project team,

The topic of package names for TCKs was discussed in the Specification Committee call on Wednesday, January 26. The outcome of these discussions is that the JESP and the TCK Process will be updated with the clarification and requirement that packages beginning with jakarta are not allowed for any deployment, including applications used by TCKs.

End-user applications are not allowed to use 'jakarta' namespace, and since TCKs are supposed to mimic the behavior of these applications while testing the implementation, it, therefore, does not make sense to use this namespace in TCKs. Implementations experiencing issues with jakarta namespaced TCKs will most likely file challenges to invalidate the tests.

The project teams may choose any package name other than those beginning with jakarta for their TCK. A naming standard for TCK packages other than the one mentioned above may be introduced at a later stage, but not in the Jakarta EE 10 time frame.

The Specification Committee suggests that you go forward with this PR, either with the proposed package name change or change it to something entirely different of your choice. As long as it is not starting with jakarta.

Spec. committee recognizes there has been confusion with respect to this and is working to rectify the relevant guides and process documentation.

jamezp commented 2 years ago

@alwin-joseph @spericas @jansupol @mkarg @andymc12 The official decision is in. We cannot use the package name jakarta.* for the TCK. This PR changes it to ee.jakarta.tck for the reason that is what the initial "vote" determined. Please let me know if we want a different package name.

spericas commented 2 years ago

@jamezp I'm OK with the proposed naming (can be changed later). We need @mkarg to review his vote at this time to proceed.

arjantijms commented 2 years ago

The namespace is fine. Jakarta Faces has used a similar one, and Jakarta Authentication will very likely use a similar one too.

jansupol commented 2 years ago

Please let me know if we want a different package name.

We definitely should be using a name that satisfies Jakarta EE 11 requirements. We do not want to rename it again.

jamezp commented 2 years ago

Please let me know if we want a different package name.

We definitely should be using a name that satisfies Jakarta EE 11 requirements. We do not want to rename it again.

+1 I've got no real strong opinion on what that is as long as everyone agrees.

mkarg commented 2 years ago

We definitely should be using a name that satisfies Jakarta EE 11 requirements. We do not want to rename it again.

We will never be sure about that as the Spec Committee has not mandated ee.jakarta but just allowed to use it.

I'm just waiting for iJUG's answer and will then re-vote on this PR. Stay tuned.

mkarg commented 2 years ago

Ivar meanwhile sent preliminary documentation, definivitely for EE 10 we are free to choose the package name (ee.jakarta.ws.rs.tck) is explicitly allowed to be used but not mandatory), but for EE 11 there will be a discussion on a mandatory package. So we MUST rename for EE 10, but not necessarily to ee, and ee is NOT GUARANTEED to be the one needed for EE 11. That is the final decision of the Spec Committee for EE 10.

As the domain ee.jakarta is held by the EF, I do not see a reason to chose a different package name. I will give one more day for the iJUG to discuss, but if I do not hear from them, I will turn my vote from -1 to 0 tomorrow.

arjantijms commented 2 years ago

Perhaps an even better package name would be ee.jakarta.tck.rest?

jamezp commented 2 years ago

For me personally the package name doesn't matter that much. If we want it changed though, I do need a definitive answer on what it needs to be changed to as it takes some time to do it. Then I will try to find some time to change it if that's what we want.

spericas commented 2 years ago

Perhaps an even better package name would be ee.jakarta.tck.rest?

I believe we should keep the proposed package naming for now, as it aligns better with our current packages. I will merge this PR tomorrow.

arjantijms commented 2 years ago

@spericas

I will merge this PR tomorrow.

+1