jakartaee / expression-language

Jakarta Expression Language
https://eclipse.org/ee4j/el
Other
69 stars 50 forks source link

Placeholder for GlassFish master (6.0) borked issue with RC2 (reverted to RC1) #130

Closed scottmarlow closed 4 years ago

scottmarlow commented 4 years ago

To be investigated and explained more completely, see eclipse-ee4j/glassfish/commit/a44fe7

scottmarlow commented 4 years ago

CC @smillidge

scottmarlow commented 4 years ago

Some more context is in the GlassFish exception callstack at gist 1dacd0f2d08c8e129b1702de0e74ddd3.

scottmarlow commented 4 years ago

Hmm, there is a big difference between RC1 (has commits from end of January) + RC2 (has changes up to middle of June).

markt-asf commented 4 years ago

That isn't an EL issue. RC2 is using the correct, as per https://wiki.eclipse.org/JakartaEE_Maven_Versioning_Rules, OSGi bundle name of jakarta.el-api. Glassfish or one of its dependencies (I don't know OSGi well enough to read that error message correctly) is trying to use the incorrect name of jakarta-el.

smillidge commented 4 years ago

The contents and manifests of the impl jars generated by this project between RC2 and RC1 are very different. For example RC1 incorporated the apis and exported the apis. Whereas RC2 doesn't is that an intentional change as that obviously could be the problem.

compare https://jakarta.oss.sonatype.org/service/local/repositories/staging/archive/org/glassfish/jakarta.el/4.0.0-RC1/jakarta.el-4.0.0-RC1.jar/!/META-INF/MANIFEST.MF with https://jakarta.oss.sonatype.org/service/local/repositories/staging/archive/org/glassfish/jakarta.el/4.0.0-RC2/jakarta.el-4.0.0-RC2.jar/!/META-INF/MANIFEST.MF

arjantijms commented 4 years ago

Putting the Felix OSGi message through the pretty printer makes it easier to read:

Unable to resolve
    org.glassfish.main.jdbc.config [165]
    missing requirement
        &(package = org.glassfish.connectors.config.validators) (version >= 6.0.0) (!(version >= 7.0.0))
        caused by:
            Unable to resolve
                org.glassfish.main.connectors.internal-api [46]
                missing requirement
                    &(package = com.sun.enterprise.deployment.runtime.connector) (version >= 6.0.0) (!(version >= 7.0.0))
                    caused by:
                        Unable to resolve
                            org.glassfish.main.deployment.dol [70]
                            missing requirement
                                &(package = jakarta.enterprise.inject.spi) (version >= 3.0.0) (!(version >= 4.0.0))
                                caused by:
                                    Unable to resolve
                                        jakarta.enterprise.cdi-api [133]
                                        missing requirement
                                            &(package = jakarta.el) (version >= 4.0.0))]]]

It says here that the module "jakarta.enterprise.cdi-api" can't find the package "jakarta.el" in version 4.0.0 or higher.

Looking at the RC2 manifest indeed exports are missing.

arjantijms commented 4 years ago

RC1 exports packages:

com.sun.el;uses:="com.sun.el.stream,com.sun.el.parser,com.sun.el.lang,jakarta.el,com.sun.el.util";version="4.0.0.SNAPSHOT",

com.sun.el.stream;uses:="com.sun.el.lang,jakarta.el";version="4.0.0.SNAPSHOT",

com.sun.el.parser;uses:="com.sun.el,com.sun.el.lang,jakarta.el,com.sun.el.util";version="4.0.0.SNAPSHOT",

com.sun.el.lang;uses:="com.sun.el,jakarta.el,com.sun.el.parser,com.sun.el.util";version="4.0.0.SNAPSHOT",

jakarta.el;version="4.0.0.SNAPSHOT",

com.sun.el.util;uses:="com.sun.el.lang,jakarta.el";version="4.0.0.SNAPSHOT"

RC2 exports packages:

com.sun.el;uses:="jakarta.el";version="4.0.0.RC2"
smillidge commented 4 years ago

it's because the api classes are not in the impl jar. If that is intentional we can accommodate it at on the GlassFish side but if not should be fixed on your side.

The previous 3.0.0 impl release also contained the api classes.

markt-asf commented 4 years ago

Yes, it is intentional. The change was discussed during the RC2 release process as we tried to clean-up the release process and address some of the issues we had to deal with in previous releases. We are almost, but not quite at the desired end-state now. Medium to long term the implementation will move to at least a separate git repo and likely a separate project. The spec is also likely to move to a separate git repo.

arjantijms commented 4 years ago

it's because the api classes are not in the impl jar. If that is intentional we can accommodate it at on the GlassFish side but if not should be fixed on your side.

You're right.

Mark cleaned up the project recently and did away with the combined jar. Historically this was the major artefact being build by the project, and before it was Mavenized (if I'm not mistaken) there wasn't even a build target let alone release for the standalone implementation.

Having 3 different releases from a single repo has been a bit problematic, but so is perhaps the timing of this change.

Long story short, let's try to accommodate for this on the GF side then.

arjantijms commented 4 years ago

Yes, it is intentional.

Some glitch on my side, my comment didn't post, and only posted it now. Didn't see Mark's comment, but I see it says mostly the same as me, so that's good ;)

Yes, the implementation really has to move to its own repo. For JSP this will happen as well and that one is a bit further in the process as a new project and repo has already been created (but not populated yet).

smillidge commented 4 years ago

Looks like we are now getting errors because jstl needs to be upgraded to use these new artifacts. Shall I raise an issue on JSTL?

jakarta.servlet.jsp.jstl-api [175](R 175.0)] osgi.wiring.package; (&(**osgi.wiring.package=jakarta.el)(version>=4.0.0.SNAPSHOT**))]

scottmarlow commented 4 years ago

+1

arjantijms commented 4 years ago

I've released a new version of JSTL (RC2). I wonder how the previous one got released, as I saw in the POM the -SNAPSHOT for EL got changed at the same day as the release was done. Oh well.