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

jQA Dependency Analysis Results #125

Open
JanWesterkamp-iJUG opened this issue May 12, 2022 · 29 comments
Open

jQA Dependency Analysis Results #125

JanWesterkamp-iJUG opened this issue May 12, 2022 · 29 comments

Comments

@JanWesterkamp-iJUG
Copy link
Contributor

I updated the dependency analysis with the jQA project:

jQA report export.zip

@JanWesterkamp-iJUG
Copy link
Contributor Author

JanWesterkamp-iJUG commented May 12, 2022

@ivargrimstad , @starksm64 : First finding in Core Profile is a non-optional dependency in CDI to Transacations, which lead into a circular dependency between the two (with different versions of CDI)!

I think this is a problem, that need to be solved in CDI as root cause, but in this and Transactions we might have to do something too.

@JanWesterkamp-iJUG
Copy link
Contributor Author

I enriched the jakartaee-api CSV report with some details and root causes:

jakarta-ee-dependencies_PlatformDependencyVersionsByArtifact.xlsx
jakarta-ee-dependencies_PlatformDependencyVersionsByArtifact.pdf
jakarta-ee-dependencies_PlatformDependencyVersionsByArtifact.ods

I added the topic for the Jakarta EE Platform meeting.

@JanWesterkamp-iJUG
Copy link
Contributor Author

@starksm64 have a look at this jakartaee-core-api dependency graphs:

jakarta-ee-dependencies_SpecificationDependencyDiagram
jakarta-ee-dependencies_PlatformDependencyDiagram

Besides the cycle, there are a lot of dubicated transitive dependencies.

@starksm64
Copy link
Contributor

This analysis is showing deficits in analyzing bulk API artifact dependencies vs actual code level dependencies:

  • CDI has no code level dependencies on Transactions. It has a javadoc dependency in one class to jakarta.transaction.Synchronization.
  • Transactions has a dependency on an earlier version of CDI. It has 2 code level and 1 javadoc dependency on:
    • jakarta.enterprise.util.Nonbinding, jakarta.enterprise.context.NormalScope (code)
    • jakarta.enterprise.context.ContextNotActiveException (javadoc)

Transactions depends on CDI 3.0.1 and these have not changed since 3.0.0 when javax -> jakarta happened. Transactions also has a provided scope dependency, and at least the OSGi manifest places no upper bound on the version, meaning it depends on the range [3.0.1, *). There is nothing I see in this analysis that requires an update to Transactions.

Even an optional dependency from CDI to Transactions is not a correct one. There really should be a javadoc scope of dependency to make it clear that there is no code level dependency.

The path of least impact would be to remove the CDI javadoc dependencies in a new minor release.

@JanWesterkamp-iJUG
Copy link
Contributor Author

@starksm64 , @ivargrimstad here are the updated jqa results refelcting the lates changes:
jQA Report Jakarta EE 10 20220521 02.zip
(the csv contains the list of used specs with versions, in the plantuml folders you can find the rendered SVGs too)

It was delayed a little bit because of the still exisiting issues with the staging repos - details can be found here:

It looks like we are getting closer with the repo issues and the findings in the dependencies too.
The Core Profile issue with Transactions and CDI cyclic dependency is solved now (using CDI 4.0.1). But I am unsure regarding the general configruration having all dependenies on the profile being optional and distinguishing only by compile and provided scope. This is at least a difference to microprofile, where there is a pom artifact with non-optional requirements in provided scope in general for MP platform dependencies.
Is there any special reason for this?

@JanWesterkamp-iJUG
Copy link
Contributor Author

Here are some details regarding the current use of Jakarta 9.1 specs in MicroProfile 5.0 to know, what specs need to be part of the Jakarta EE Core Profile:
JQA Report MicroProfile.5.0 20220521 03.zip

Are there other requirements regarding the content besides MicroProfile?

@arjantijms
Copy link
Contributor

The Faces jar was already stages and released :O To fix the issues revealed, we'll have to release a 4.0.1 service release (just like CDI did).

@JanWesterkamp-iJUG
Copy link
Contributor Author

The Faces jar was already stages and released :O To fix the issues revealed, we'll have to release a 4.0.1 service release (just like CDI did).

@arjantijms yes, 4.0.0 is on Maven Central since 2022-05-03, like a 4.0.0-M6 is since 2022-03-06 too - may be we should prevent releasing too early (before the ballot has ended) in general and Release Candidates or Milestones at all in the future ;-)

@arjantijms
Copy link
Contributor

4.0.0 is on Maven Central since 2022-05-03

I think that date is not the release date, but the staging date. It's super confusing, but when you stage something and then later release it to central, it shows the original date of staging, not the date of the releaser to central.

The exact date of releasing to central was "2022-05-18T16:10:12Z"

See https://ci.eclipse.org/faces/job/3_faces-staging-to-release/5/console

@JanWesterkamp-iJUG
Copy link
Contributor Author

But released was the original version from 2022-05-03 and not the one with the fixes applied during the ballot, right?
Or do we might have the same problem here as we have with the jQA analsysis - the index of the nexus instance of the staging repo is not updated the right way - and then an older version with the same maven coordinates will be shown (!)

@starksm64
Copy link
Contributor

starksm64 commented May 23, 2022 via email

@JanWesterkamp-iJUG
Copy link
Contributor Author

@arjantijms You are right, when forward an artifact from Staging to Central the original time stamp is unchanged. This is good to know, because this means there is some additional field beyond the maven coordinates to distinguish artifacts and that should be refected to trigger an update of the search index!

I think we have multiple problems here with Faces:

You triggered a rebuild of the Faces 4.0.0 artifact in Staging before you want to push it to central on 2022-05-18, right?
https://ci.eclipse.org/faces/job/1_faces-build-and-stage/41/console

In this run there are warnings regarding the maven-resources-plugin:

... Project version: [WARNING] [WARNING] Some problems were encountered while building the effective model for jakarta.faces:jakarta.faces-api:jar:4.0.0-SNAPSHOT [WARNING] 'profiles.profile[docs].plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.apache.maven.plugins:maven-resources-plugin @ line 511, column 29 [WARNING] [WARNING] It is highly recommended to fix these problems because they threaten the stability of your build. [WARNING] [WARNING] For this reason, future Maven versions might no longer support building such malformed projects. [WARNING] 4.0.0-SNAPSHOT Mojarra version: 4.0.0-M3 ...
During the run, most of the steps are skipped - and it looks like the artifact itself has been unchanged through that run.

I was wondering, that the Mojarra version was set to 4.0.0-M3 - could that cause problems too?
I did not looked at the details of the pipelines yet, but may be forcing setting current parameters or defaults might be helpful.

Then the forwarding to Central was triggerd and pushed the 4.0.0 from 2022-05-03 to it:
https://ci.eclipse.org/faces/job/3_faces-staging-to-release/5/

Now a the Nexus issue prevented to update the search index - it is stuck to the state from 2022-05-17! The forwarding on 2022-05-18 of the Faces 4.0.0 artifact from 2022-05-03 is not refelcted in the search index (2022-05-17), even when there is an older artifact state (2022-05-03)! And the push did not trigger the index update.

@starksm64 The same Nexus issue takes place with CDI on Central! Artifacts newer then 2022-05-17T15:58:19 CEST can not be fetched by jQA (and other tools using the search).

I will open 2 new issues at Sonatype for the Faces and CDI GroupIDs and reference them in the original ticket to follow up that research.

@JanWesterkamp-iJUG
Copy link
Contributor Author

@JanWesterkamp-iJUG
Copy link
Contributor Author

@ivargrimstad and @starksm64: I updated the jQA analysis with the new released artifacts:
jQA Report Jakarta EE 10 20220531 01.zip

This is based on the #127.

@JanWesterkamp-iJUG
Copy link
Contributor Author

Here is the updated report (a local cache clean helped this time luckily) with the current state:
jQA Report Jakarta EE 10 20220531 02.zip

@JanWesterkamp-iJUG
Copy link
Contributor Author

Here is the latest update for the reports:
jQA Report Jakarta EE 20220602 01.zip

@arjantijms
Copy link
Contributor

@JanWesterkamp-iJUG I updated the faces api jar and staged 4.0.1. See https://jakarta.oss.sonatype.org/content/repositories/staging/jakarta/faces/jakarta.faces-api/4.0.1/

Can you let me know if it's okay now?

@JanWesterkamp-iJUG
Copy link
Contributor Author

JanWesterkamp-iJUG commented Jun 3, 2022

@JanWesterkamp-iJUG I updated the faces api jar and staged 4.0.1. See https://jakarta.oss.sonatype.org/content/repositories/staging/jakarta/faces/jakarta.faces-api/4.0.1/

Can you let me know if it's okay now?

@arjantijms It looks much better, the main issue is fixed! But there are still transitive issues left over, here is the report:
jQA Report Faces 4.0.1 20220603 02.zip

  • CDI 4.0.1 is referencing Lang Model 4.0.1 (you started the discussion about the need already)
  • Transaction(s) 2.0.1 is referencing CDI 3.0.1 (instead of CDI 4.0.1) and Interceptor 2.0.1 (instead of 2.1.0) with the already mentioned issues
  • EJB 4.0.1 is referencing an old version of transaction (2.0.0) with issues
  • WebSocket 2.1.0 depends on WebSocket Client 2.1.0 - not the other way around

Except for the CDI <-> Model Lang dependency, the Transaction and EJB old transitive dependencies don't get pulled automatically, because the direct dependecies are optional. But users changing that to false need to take extra care in which order they overwrite that to not pull old versions accidentally. I think we should do what we can to fix that as soon as possible to also reduce the exisitng complexity for new releases!

The last finding may have historical reasons, but having a backend or more general interface depending on a client looks a little bit weird to me ;-)

I hope that information is helpful, especially to the other projects too, where things need to be fixed. May be that is worth to wait and to release the Faces 4.0.1 to central finally, when these issues could be fixed before. Or doing an Faces 4.0.2+ with these fixes then.

@starksm64 By the way - this release gives the opportunity to remove the optional org.glasfish.faces Milestone dependency from the jakarta.platform.jakartaee-web-api and jakartaee-api (Web Profile and Platform/Full Profile) already.

@arjantijms
Copy link
Contributor

Or doing an Faces 4.0.2+ with these fixes then.

What can I specifically fix in Faces?

The only option left I think is to hand craft a pom.xml and remove EJB and Transactions etc. It's a weird dependency in Faces, but it results from the bizarre way in which the Faces API is generated from Mojarra.

Otherwise, can you do a PR to the Faces repo with proposed changes?

@JanWesterkamp-iJUG
Copy link
Contributor Author

JanWesterkamp-iJUG commented Jun 3, 2022

Analysis of the Platform/Full Profile generates the following issues:

  • Web Profile should add Core Profile as first dependency and drop separate dependency definitions instead (like the Full Profile/Platform does with the Web Profile)
  • Web ans Core Profile should remove the Mojara (from Glasfish Faces) dependency
  • Transaction(s) 2.0.1 is referencing CDI 3.0.1 (instead of CDI 4.0.1) and Interceptor 2.0.1 (instead of 2.1.0) with the already mentioned issues (new Transactions 3.0.0 or at least 2.0.2 release?)
  • EJB 4.0.1 is referencing an old version of Transaction (2.0.0) with issues
  • WebSocket 2.1.0 depends on WebSocket Client 2.1.0 - not the other way around
  • Faces 4.0.1+ instead of 4.0.0 need to be used in the Web Profile now (see above)
  • Batch has a dependeny to CDI 4.0.0, but should use 4.0.1(+) instead (takes version from it's parent)
  • JMS 3.1.0 has a commented but not as such configured dependency to Annotations 2.1.0-B1 (this is on central, but should not) instead of the final version 2.1.0
  • Concurrent 3.0.0 has a dependency to CDI 4.0.0 (compile, should be 4.0.1+), EJB 4.0.0 (provided, should be 4.0.1+) and Interceptor 2.0.1 (provided, should be 2.1.0)
  • Concurrent 3.0.0 has a compile (!) dependency to TestNG 6.14.3 that should be none at all or 7.6.0
  • Concurrent 3.0.0 has a compile (!) dependency to Arquillian TestNG Container 1.6.0 that should be none at all or may be another scope
  • Concurrent 3.0.0 has test dependency to jUnit 4.13, which should be none at all or 5.8.2 or at least 4.13.2
  • Mail 2.1.0 has test dependencies to JUnit 4.13.2 (last verison of major 4 releases) and Angus Activation 1.0.0-M2 (available on central, but should not), that should be 1.0.0 on none at all
  • CDI 4.0.1 has a test dependency to TestNG 6.8.8 that should be none at all or 7.6.0 or at least 6.14.3
  • Resource 2.1.0 has a test dependency to TestNG 6.8.8 that should be none at all or 7.6.0 or at least 6.14.3
  • JSON Bind 3.0.0 has a test dependency to JUnit 4.13.1 which should be non at all or migrated to JUnit Jupiter 5.8.2 or at least 4.13.2
  • Servlet 6.0.0 has test dpendencies to JUnit 5.8.1 (engine and params), but should have none at all or at least version 5.8.2
  • Servlet 6.0.0 has test dependency to Hamcrest 2.2, but should not have a test dependency at all

@starksm64 and @ivargrimstad How we want to proceed with these findings?
Note, I tried to get a little bit an order of severity (from my perspective) in the list ;-)

Not analysed yet, but some of the Maven related updated need to be done in sync with other module technics too (like JPMS, OSGi).

@JanWesterkamp-iJUG
Copy link
Contributor Author

JanWesterkamp-iJUG commented Jun 3, 2022

Or doing an Faces 4.0.2+ with these fixes then.

What can I specifically fix in Faces?

The only option left I think is to hand craft a pom.xml and remove EJB and Transactions etc. It's a weird dependency in Faces, but it results from the bizarre way in which the Faces API is generated from Mojarra.

Otherwise, can you do a PR to the Faces repo with proposed changes?

@arjantijms I think we should include Faces 4.0.1 as it is in the Profiles (Web & Full), but we may not release is to central now, so some of the changes above could go into it too.
Or we release 4.0.1 now to central and we have a 4.0.2 to include, when the fixes in the other component specs are done - but that requires additional changes to the upper profiles again then. For now, I think your POM works fine as it is and as it could be solved withing Faces or from the current Faces perspective.

@arjantijms
Copy link
Contributor

Okay, I'll push Faces 4.0.1 to central then. Let me know if there's anything specific that I can add or change to the Faces pom for a 4.0.2.

@JanWesterkamp-iJUG
Copy link
Contributor Author

JanWesterkamp-iJUG commented Jun 8, 2022

Some jQA details to Concurrent API 3.0.0:
jakarta-ee-dependencies_PlatformDependencyDiagram
jakarta-ee-dependencies_SpecificationDependencyDiagram

@JanWesterkamp-iJUG
Copy link
Contributor Author

Here is the updated version of the jQA dependency analysis:
jQA Report Jakarta EE 20220617 01.zip

@JanWesterkamp-iJUG
Copy link
Contributor Author

Here is the updated version of the jQA dependency analysis:
jQA Report Jakarta EE 20220802 01.zip

@JanWesterkamp-iJUG
Copy link
Contributor Author

Here is the updated version of the jQA dependency analysis:
jQA Report Jakarta EE 10 20220810 01 .zip

Regardning the duplicate and non-final versions referenced we made only very little progress.
That affects all Profiles, but can be seen in the Platform Dependency Diagram especially on the Web Profile in the eastern part of the landscape (in this version) with Transaction 2.0.1 and EJB 4.0.1 still referencing Transaction 2.0.0.
In the Platform/Full Profile this is even worse because Batch 2.1.1 still references CDI 4.0.0 and Resource 2.1.0 references CDI 3.0.1! Also JMS 3.1.0 references Annotation 2.1.0-B1 (far north east) and Mail 2.1.0 references Angus Activationn 1.0.0-M2 (far south west, as test dependency).

The last two Issues and the one with Batch (if possible with a Service Release) in the Platform/Full Profile might be fixed esaily, but the other ones would require a fixing release of Transaction and then updates to all of it's users two.

@arjantijms in the Web/Full Profile, do we still need the Glassfish Jakarta Faces (Impl) dependency as Faces 4.0.1 has a Glassfish Jakarta Faces Sources 4.0.0 dependency now (both provided and optional)?
@starksm64 If the answer is no, then we should drop these dependencies for sure from the Profiles!

As far as I analysed it yet, none of this issues is a real blocker for the release in respect to API dependenies as Maven magic should solve them - so they could be accepted as technical dept. But clean is something else... ;-)
Regarding the TCK environment for component and umbrella specs, this might not be true, as mentioned in the Platform call.

@arjantijms
Copy link
Contributor

@arjantijms in the Web/Full Profile, do we still need the Glassfish Jakarta Faces (Impl) dependency as Faces 4.0.1 has a Glassfish Jakarta Faces Sources 4.0.0 dependency now (both provided and optional)?

It should be a source dependency only, which is used during the build. The final API artefact should not have it. If it's there., it should be removed. PR welcome.

@JanWesterkamp-iJUG
Copy link
Contributor Author

@arjantijms in the Web/Full Profile, do we still need the Glassfish Jakarta Faces (Impl) dependency as Faces 4.0.1 has a Glassfish Jakarta Faces Sources 4.0.0 dependency now (both provided and optional)?

It should be a source dependency only, which is used during the build. The final API artefact should not have it. If it's there., it should be removed. PR welcome.

@arjantijms I am preparing a PR for this. Just to be sure: I can remove the workaround in jakartaee-api project (and subprojects), because this is used only for the build (compile time) of the jakarta.faces-api project itself and nothing need to be build and included into the Web Profile and Full Profile APIs anymore because you fixed that already.
It looks like this would require to remove the glassfish-plugin execution too (otherwise the Web/Full Profile build is failing) and this fact is fine, because it was part of the original workaround, right? Or are there other requirements for this plugin configuration?

Unfortunately the documentation of it is very week - can somebody (@arjantijms, @starksm64, @ivargrimstad) explain the functionality of the plugin and it's role in the Profile builds? I was wondering a little bit, because it is configured in the Core Profile too, which contradicts my original assumption.

@JanWesterkamp-iJUG
Copy link
Contributor Author

JanWesterkamp-iJUG commented Aug 17, 2022

As requested, here the current state of the proposed target picture for the Platform Profiles:
Core Target 20220817 01.zip

These files are manual edited versions of the jQA report results showing TODOs in different colours, i.e. green for a simple new release with updated dependencies, yellow the updated test dependencies (removed completely at best) to orange and red that might need further investigation, non-Service-Releases etc..
Edited PlantUML files and renderd SVGs are included.

One interesting result: We need 8 waves for a complete clean Jakarta EE API release!

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

No branches or pull requests

3 participants