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

appclient container needs 2.1.1 Jakarta Annotation ManagedBean #24020

Merged
merged 2 commits into from
Jun 28, 2022

Conversation

scottmarlow
Copy link
Member

Signed-off-by: Scott Marlow smarlow@redhat.com

Possible change for #24014

@scottmarlow
Copy link
Member Author

Note that this doesn't build correctly for me yet due to Could not find artifact org.jboss.weld.se:weld-se-shaded:jar:5.0.SP2

@dmatej
Copy link
Contributor

dmatej commented Jun 22, 2022

Oh, nice, so I can drop my local changes where I "hacked" appclient by adding newer annotation-api to the classpath before weld-se-shaded.

@arjantijms
Copy link
Contributor

@scottmarlow If I look here, there's no 5.0.SP2 yet:

https://repo1.maven.org/maven2/org/jboss/weld/se/weld-se-shaded

The last one is

5.0.0.SP2         2022-06-08 13:08         -

@starksm64
Copy link
Member

starksm64 commented Jun 23, 2022

That artifact is there:
https://repo1.maven.org/maven2/org/jboss/weld/se/weld-se-shaded/5.0.0.SP2/

weld-se-shaded-5.0.0.SP2-javadoc.jar              2022-06-08 13:08   7205180      
weld-se-shaded-5.0.0.SP2-javadoc.jar.asc          2022-06-08 13:08       659      
weld-se-shaded-5.0.0.SP2-javadoc.jar.md5          2022-06-08 13:08        32      
weld-se-shaded-5.0.0.SP2-javadoc.jar.sha1         2022-06-08 13:08        40      
weld-se-shaded-5.0.0.SP2-sources.jar              2022-06-08 13:08   2913339      
weld-se-shaded-5.0.0.SP2-sources.jar.asc          2022-06-08 13:08       659      
weld-se-shaded-5.0.0.SP2-sources.jar.md5          2022-06-08 13:08        32      
weld-se-shaded-5.0.0.SP2-sources.jar.sha1         2022-06-08 13:08        40      
weld-se-shaded-5.0.0.SP2.jar                      2022-06-08 13:08   3464828      
weld-se-shaded-5.0.0.SP2.jar.asc                  2022-06-08 13:08       659      
weld-se-shaded-5.0.0.SP2.jar.md5                  2022-06-08 13:08        32      
weld-se-shaded-5.0.0.SP2.jar.sha1                 2022-06-08 13:08        40      
weld-se-shaded-5.0.0.SP2.pom                      2022-06-08 13:08      7598      
weld-se-shaded-5.0.0.SP2.pom.asc                  2022-06-08 13:08       659      
weld-se-shaded-5.0.0.SP2.pom.md5                  2022-06-08 13:08        32      
weld-se-shaded-5.0.0.SP2.pom.sha1                 2022-06-08 13:08        40 

@starksm64
Copy link
Member

There will have to be a new post 5.0.0.SP2 release to pickup the update of common annotations to 2.1.1.

@dmatej
Copy link
Contributor

dmatej commented Jun 23, 2022

That artifact is there: https://repo1.maven.org/maven2/org/jboss/weld/se/weld-se-shaded/5.0.0.SP2/

weld-se-shaded-5.0.0.SP2-javadoc.jar              2022-06-08 13:08   7205180      
weld-se-shaded-5.0.0.SP2-javadoc.jar.asc          2022-06-08 13:08       659      
weld-se-shaded-5.0.0.SP2-javadoc.jar.md5          2022-06-08 13:08        32      
weld-se-shaded-5.0.0.SP2-javadoc.jar.sha1         2022-06-08 13:08        40      
weld-se-shaded-5.0.0.SP2-sources.jar              2022-06-08 13:08   2913339      
weld-se-shaded-5.0.0.SP2-sources.jar.asc          2022-06-08 13:08       659      
weld-se-shaded-5.0.0.SP2-sources.jar.md5          2022-06-08 13:08        32      
weld-se-shaded-5.0.0.SP2-sources.jar.sha1         2022-06-08 13:08        40      
weld-se-shaded-5.0.0.SP2.jar                      2022-06-08 13:08   3464828      
weld-se-shaded-5.0.0.SP2.jar.asc                  2022-06-08 13:08       659      
weld-se-shaded-5.0.0.SP2.jar.md5                  2022-06-08 13:08        32      
weld-se-shaded-5.0.0.SP2.jar.sha1                 2022-06-08 13:08        40      
weld-se-shaded-5.0.0.SP2.pom                      2022-06-08 13:08      7598      
weld-se-shaded-5.0.0.SP2.pom.asc                  2022-06-08 13:08       659      
weld-se-shaded-5.0.0.SP2.pom.md5                  2022-06-08 13:08        32      
weld-se-shaded-5.0.0.SP2.pom.sha1                 2022-06-08 13:08        40 

This one was already used, see dates, 8th june. The version 5.0.SP2 instead of 5.0.0.SP2 seems suspicious to me ...

@dmatej
Copy link
Contributor

dmatej commented Jun 23, 2022

Seems also there are no other changes in Weld core github repo, including weld-se-shaded project. So we should probably create a PR with updated dependencies for them.
image

@dmatej
Copy link
Contributor

dmatej commented Jun 23, 2022

Aha, I get it ... we need to update weld-core's dependency on weld-api ...

@dmatej
Copy link
Contributor

dmatej commented Jun 23, 2022

See weld/core#2747

@scottmarlow
Copy link
Member Author

weld/core#2747 + https://issues.redhat.com/browse/WELD-2722 sound like a great change but considering that the pull request is still in draft mode and the work in my opinion, sounds like something that might not be solved quickly, I wonder if it is better to hack in the individual common annotations 2.1.1 jar into the appclient container as @dmatej was likely doing before I created this broken pr! :-)

@dmatej
Copy link
Contributor

dmatej commented Jun 23, 2022

My "hack" wasn't nice and @manovotn estimates that tomorrow they will release new Weld, so we can probably wait for that :-)

@scottmarlow
Copy link
Member Author

I changed the pull request to update to Weld 5.0.0.SP3 (looks like next likely tag in https://github.com/weld/core/tags), if that is not the right version (when released), I will update this pr again.

@manovotn
Copy link

@scottmarlow The version will be 5.0.1.Final (using the Weld API 5.0.SP2).
The release is running now so if there are no issues with it, I assume it could land in Central within an hour or so.

Signed-off-by: Scott Marlow <smarlow@redhat.com>
@dmatej
Copy link
Contributor

dmatej commented Jun 24, 2022

@scottmarlow The version will be 5.0.1.Final (using the Weld API 5.0.SP2). The release is running now so if there are no issues with it, I assume it could land in Central within an hour or so.

Just FYI, it reached Maven Central, co I initiated rebuild on Jenkins.

@dmatej
Copy link
Contributor

dmatej commented Jun 24, 2022

Eh, Maven default rules - it saves failures, so it will recheck after 24 h again. Another option is to purge the Maven repository.

Copy link
Contributor

@dmatej dmatej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems OSGi doesn't like this change ...

@arjantijms
Copy link
Contributor

After a quick run it seems like the CDI TCK also doesn't like this upgrade:

[ERROR] Failures: 
[ERROR]   ProcessBeanAttributesNotFiredForBuiltinBean>Arquillian.run:138->testProcessBeanAttributesNotFired:51 expected [true] but found [false]
[ERROR]   AfterTypeDiscoveryTest>Arquillian.run:138->testFinalAlternatives:128 expected [true] but found [false]
[ERROR]   AfterTypeDiscoveryTest>Arquillian.run:138->testInitialAlternatives:81 expected [3] but found [11]
[ERROR]   AfterTypeDiscoveryMassOperationsTest>Arquillian.run:138->testInitialAlternatives:77 expected [3] but found [11]

Maybe a Weld 5.0.1 is just too much for now? What we really need is a 5.0.0.SP3, which only changes the problematic Jakarta Annotation ManagedBean and absolutely nothing else. No comment, no whitespace, nothing. Just ManagedBean.

@dmatej
Copy link
Contributor

dmatej commented Jun 27, 2022

After a quick run it seems like the CDI TCK also doesn't like this upgrade:

[ERROR] Failures: 
[ERROR]   ProcessBeanAttributesNotFiredForBuiltinBean>Arquillian.run:138->testProcessBeanAttributesNotFired:51 expected [true] but found [false]
[ERROR]   AfterTypeDiscoveryTest>Arquillian.run:138->testFinalAlternatives:128 expected [true] but found [false]
[ERROR]   AfterTypeDiscoveryTest>Arquillian.run:138->testInitialAlternatives:81 expected [3] but found [11]
[ERROR]   AfterTypeDiscoveryMassOperationsTest>Arquillian.run:138->testInitialAlternatives:77 expected [3] but found [11]

Maybe a Weld 5.0.1 is just too much for now? What we really need is a 5.0.0.SP3, which only changes the problematic Jakarta Annotation ManagedBean and absolutely nothing else. No comment, no whitespace, nothing. Just ManagedBean.

Finally we found that these CDI TCK failures were caused by something between Krazo and CDI TCK, it has nothing to do with this PR.

But with this PR GlassFish simply doesn't start, because Weld requires jboss-logging >=3.5.0

Signed-off-by: David Matějček <dmatej@seznam.cz>
@dmatej dmatej merged commit 2cdfe0f into eclipse-ee4j:master Jun 28, 2022
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

Successfully merging this pull request may close these issues.

5 participants