-
Notifications
You must be signed in to change notification settings - Fork 43
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
Comments
@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. |
I enriched the jakartaee-api CSV report with some details and root causes: jakarta-ee-dependencies_PlatformDependencyVersionsByArtifact.xlsx I added the topic for the Jakarta EE Platform meeting. |
@starksm64 have a look at this jakartaee-core-api dependency graphs: Besides the cycle, there are a lot of dubicated transitive dependencies. |
This analysis is showing deficits in analyzing bulk API artifact dependencies vs actual code level dependencies:
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. |
@starksm64 , @ivargrimstad here are the updated jqa results refelcting the lates changes: 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. |
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: Are there other requirements regarding the content besides MicroProfile? |
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 ;-) |
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 |
But released was the original version from 2022-05-03 and not the one with the fixes applied during the ballot, right? |
https://ci.eclipse.org/cdi/job/MavenCentralRelease/
Is the CI job to release staged artifacts to central.
4.0.0 was released on 2022-04-19 (YYYY-MM-DD)
4.0.1 was released on 2022-05-18
…On May 23, 2022 at 2:11:03 PM, JanWesterkamp-iJUG ***@***.***> wrote:
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 (!)
—
Reply to this email directly, view it on GitHub
<#125 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACRDMRR24GJWABC6JTBD3LVLPJ4PANCNFSM5VXZNHWQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@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? In this run there are warnings regarding the maven-resources-plugin:
I was wondering, that the Mojarra version was set to 4.0.0-M3 - could that cause problems too? Then the forwarding to Central was triggerd and pushed the 4.0.0 from 2022-05-03 to it: 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. |
Tickets updated and created on the Sonatype side: |
@ivargrimstad and @starksm64: I updated the jQA analysis with the new released artifacts: This is based on the #127. |
Here is the updated report (a local cache clean helped this time luckily) with the current state: |
Here is the latest update for the reports: |
@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:
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. |
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? |
Analysis of the Platform/Full Profile generates the following issues:
@starksm64 and @ivargrimstad How we want to proceed with these findings? Not analysed yet, but some of the Maven related updated need to be done in sync with other module technics too (like JPMS, OSGi). |
@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. |
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. |
Here is the updated version of the jQA dependency analysis: |
Here is the updated version of the jQA dependency analysis: |
Here is the updated version of the jQA dependency analysis: Regardning the duplicate and non-final versions referenced we made only very little progress. 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)? 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... ;-) |
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. 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. |
As requested, here the current state of the proposed target picture for the Platform Profiles: 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.. One interesting result: We need 8 waves for a complete clean Jakarta EE API release! |
I updated the dependency analysis with the jQA project:
jQA report export.zip
The text was updated successfully, but these errors were encountered: