jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.86k stars 1.91k forks source link

Wrong BREE in Jetty jars #10665

Closed akurtakov closed 1 year ago

akurtakov commented 1 year ago

Jetty version(s) Jetty 12.0.1

Jetty Environment core, ee8, ee9, ee10

Java version/vendor (use: java -version) openjdk 21 2023-09-19 OpenJDK Runtime Environment (build 21+35-2513) OpenJDK 64-Bit Server VM (build 21+35-2513, mixed mode, sharing)

OS type/version Fedora 39

Description Jars have wrong Bundle-RequiredExecutionEnvironment: JavaSE-11 but there is Import-Package: java.lang.runtime and this package has been added in Java 14 thus the declared BREE is not sufficient.

How to reproduce? Open e.g. https://repo1.maven.org/maven2/org/eclipse/jetty/jetty-server/12.0.1/jetty-server-12.0.1.jar/META-INF/MANIFEST.MF and verify the statements above It has been identified while moving Eclipse to use Jetty 12.0.1 and the following issue was reported https://github.com/eclipse-pde/eclipse.pde/issues/779 . https://github.com/eclipse-pde/eclipse.pde/issues/779#issuecomment-1747342572 has a proposal for a fix. It would be nice to have this in the upcoming https://github.com/eclipse/jetty.project/issues/10649 .

akurtakov commented 1 year ago

The reason to recommend using osgi.ee capability instead of BREE is that the later has been deprecated for years - https://docs.osgi.org/specification/osgi.core/8.0.0/framework.module.html#framework.module.bree

joakime commented 1 year ago

The contents of jetty-server-12.0.1.jar!/META-INF/MANIFEST.MF

Manifest-Version: 1.0
Created-By: Apache Maven Bundle Plugin 5.1.9
Build-Jdk-Spec: 21
Implementation-Vendor: Eclipse Jetty Project
Implementation-Version: 12.0.1
url: https://eclipse.dev/jetty/
Bundle-Classpath: .
Bundle-Copyright: Copyright (c) 2008-2021 Mort Bay Consulting Pty Ltd an
 d others.
Bundle-Description: Jetty module for Core :: Server
Bundle-DocURL: https://eclipse.dev/jetty/
Bundle-License: https://www.eclipse.org/legal/epl-2.0/, https://www.apac
 he.org/licenses/LICENSE-2.0
Bundle-ManifestVersion: 2
Bundle-Name: Core :: Server
Bundle-RequiredExecutionEnvironment: JavaSE-11
Bundle-SymbolicName: org.eclipse.jetty.server
Bundle-Vendor: Eclipse Jetty Project
Bundle-Version: 12.0.1
Export-Package: org.eclipse.jetty.server;uses:="javax.net.ssl,org.eclips
 e.jetty.http,org.eclipse.jetty.http.content,org.eclipse.jetty.io,org.ec
 lipse.jetty.io.content,org.eclipse.jetty.io.ssl,org.eclipse.jetty.serve
 r.handler,org.eclipse.jetty.util,org.eclipse.jetty.util.annotation,org.
 eclipse.jetty.util.component,org.eclipse.jetty.util.resource,org.eclips
 e.jetty.util.ssl,org.eclipse.jetty.util.thread,org.slf4j";version="12.0
 .1",org.eclipse.jetty.server.handler;uses:="org.eclipse.jetty.http,org.
 eclipse.jetty.http.content,org.eclipse.jetty.http.pathmap,org.eclipse.j
 etty.io,org.eclipse.jetty.server,org.eclipse.jetty.util,org.eclipse.jet
 ty.util.annotation,org.eclipse.jetty.util.component,org.eclipse.jetty.u
 til.resource,org.eclipse.jetty.util.thread";version="12.0.1",org.eclips
 e.jetty.server.handler.gzip;uses:="org.eclipse.jetty.http,org.eclipse.j
 etty.io,org.eclipse.jetty.server,org.eclipse.jetty.util,org.eclipse.jet
 ty.util.compression,org.eclipse.jetty.util.thread,org.slf4j";version="1
 2.0.1",org.eclipse.jetty.server.handler.jmx;uses:="org.eclipse.jetty.jm
 x,org.eclipse.jetty.server.handler,org.eclipse.jetty.server.jmx,org.ecl
 ipse.jetty.util.annotation";version="12.0.1",org.eclipse.jetty.server.j
 mx;uses:="org.eclipse.jetty.jmx,org.eclipse.jetty.server.handler,org.ec
 lipse.jetty.util.annotation";version="12.0.1"
Import-Package: org.slf4j;version="[1.7,3.0)",java.io,java.lang,java.lan
 g.invoke,java.lang.reflect,java.lang.runtime,java.net,java.nio,java.nio
 .channels,java.nio.charset,java.nio.file,java.security,java.security.ce
 rt,java.time,java.time.format,java.time.temporal,java.util,java.util.co
 ncurrent,java.util.concurrent.atomic,java.util.concurrent.locks,java.ut
 il.function,java.util.regex,java.util.stream,java.util.zip,javax.net.ss
 l,org.eclipse.jetty.http;version="[12.0.1,13)",org.eclipse.jetty.http.c
 ontent;version="[12.0.1,13)",org.eclipse.jetty.http.pathmap;version="[1
 2.0.1,13)",org.eclipse.jetty.io;version="[12.0.1,13)",org.eclipse.jetty
 .io.content;version="[12.0.1,13)",org.eclipse.jetty.io.ssl;version="[12
 .0.1,13)",org.eclipse.jetty.jmx;version="[12.0.1,13)";resolution:=optio
 nal,org.eclipse.jetty.server.handler,org.eclipse.jetty.server.jmx,org.e
 clipse.jetty.util;version="[12.0.1,13)",org.eclipse.jetty.util.annotati
 on;version="[12.0.1,13)",org.eclipse.jetty.util.component;version="[12.
 0.1,13)",org.eclipse.jetty.util.compression;version="[12.0.1,13)",org.e
 clipse.jetty.util.resource;version="[12.0.1,13)",org.eclipse.jetty.util
 .ssl;version="[12.0.1,13)",org.eclipse.jetty.util.statistic;version="[1
 2.0.1,13)",org.eclipse.jetty.util.thread;version="[12.0.1,13)",org.slf4
 j.event;version="[1.7,3.0)",org.slf4j.helpers;version="[1.7,3.0)",org.s
 lf4j.spi;version="[1.7,3.0)"
Tool: Bnd-6.3.1.202206071316
joakime commented 1 year ago

For Jetty 12 the Bundle-RequiredExecutionEnvironment: JavaSE-11 is also wrong as Jetty 12 is JDK 17 minimum.

joakime commented 1 year ago

Opened PR #10666

joakime commented 1 year ago

With PR #10666, the manifest now looks something like this ... (I used jetty-xml for this example)

Manifest-Version: 1.0
Build-Jdk-Spec: 17
Bundle-Classpath: .
Bundle-Copyright: Copyright (c) 1995 Mort Bay Consulting Pty Ltd and oth
 ers.
Bundle-Description: Jetty xmodule for Core :: XML
Bundle-DocURL: https://eclipse.dev/jetty/
Bundle-License: https://www.eclipse.org/legal/epl-2.0/, https://www.apac
 he.org/licenses/LICENSE-2.0
Bundle-ManifestVersion: 2
Bundle-Name: Core :: XML
Bundle-RequiredExecutionEnvironment: JavaSE-17
Bundle-SymbolicName: org.eclipse.jetty.xml
Bundle-Vendor: Eclipse Jetty Project
Bundle-Version: 12.0.2.SNAPSHOT
Created-By: Apache Maven Bundle Plugin 5.1.9
Export-Package: org.eclipse.jetty.xml;uses:="org.eclipse.jetty.util.anno
 tation,org.eclipse.jetty.util.component,org.eclipse.jetty.util.resource
 ,org.xml.sax";version="12.0.2"
Implementation-Vendor: Eclipse Jetty Project
Implementation-Version: 12.0.2-SNAPSHOT
Import-Package: org.slf4j;version="[1.7,3.0)",org.eclipse.jetty.util;ver
 sion="[12.0.2,13)",org.eclipse.jetty.util.annotation;version="[12.0.2,1
 3)",org.eclipse.jetty.util.component;version="[12.0.2,13)",org.eclipse.
 jetty.util.resource;version="[12.0.2,13)",org.eclipse.jetty.util.thread
 ;version="[12.0.2,13)",org.w3c.dom,org.xml.sax,org.xml.sax.helpers,org.
 slf4j.event;version="[1.7,3.0)",org.slf4j.helpers;version="[1.7,3.0)",o
 rg.slf4j.spi;version="[1.7,3.0)"
Require-Capability: osgi.serviceloader;filter:="(osgi.serviceloader=org.
 eclipse.jetty.xml.ConfigurationProcessorFactory)";resolution:=optional;
 cardinality:=multiple,osgi.extender;filter:="(osgi.extender=osgi.servic
 eloader.processor)";resolution:=optional
Tool: Bnd-6.3.1.202206071316
url: https://eclipse.dev/jetty/

@akurtakov did I miss anything?

joakime commented 1 year ago

@akurtakov would this Require-Capability be more correct? (and dropping Bundle-RequiredExecutionEnvironment: JavaSE-17 per recommendations)

Require-Capability: osgi.ee;filter:="((&(osgi.ee=OSGi/Minimum)(version=1
 .2))(&(osgi.ee=JavaSE)(version=17)))",osgi.serviceloader;filter:="(osgi
 .serviceloader=org.eclipse.jetty.xml.ConfigurationProcessorFactory)";re
 solution:=optional;cardinality:=multiple,osgi.extender;filter:="(osgi.e
 xtender=osgi.serviceloader.processor)";resolution:=optional
akurtakov commented 1 year ago

Why osgi.ee;filter:="((&(osgi.ee=OSGi/Minimum)(version=1.2)) ? I'm pretty sure that OSGi/Minimum-1.2 is not sufficieent for pretty much anything.

HannesWell commented 1 year ago
Require-Capability: osgi.ee;filter:="((&(osgi.ee=OSGi/Minimum)(version=1.2))(&(osgi.ee=JavaSE)(version=17)))"

It looks like it is both OSGi/Minimum-1.2 and/or? JavaSE-17. But I also wondered from where OSGi/Minimum-1.2 is coming.

joakime commented 1 year ago

Why osgi.ee;filter:="((&(osgi.ee=OSGi/Minimum)(version=1.2)) ? I'm pretty sure that OSGi/Minimum-1.2 is not sufficieent for pretty much anything.

@akurtakov got that from the recommendations in your documentation.
That entire line is voodoo magic to me. The documentation syntax is also very cryptic to me. (Keep in mind that nobody here on this project uses osgi, and lives in the world of IETF and RFC specs / syntaxes, which the osgi docs do not seem to follow)

joakime commented 1 year ago

The documentation makes it seem that Bundle-RequiredExecutionEnvironment is no longer used, and we need to use the Require-Capability instead.

What is the syntax you want us to use for Require-Capability ?

BTW, if I remove Bundle-RequiredExecutionEnvironment from the maven-bundle-plugin configuration, that entry is super flaky across the artifacts. Most of the artifacts generate without that entry, a few generate with the build JDK (not the compiler source/target/release configurations)

HannesWell commented 1 year ago

BTW, if I remove Bundle-RequiredExecutionEnvironment from the maven-bundle-plugin configuration, that entry is super flaky across the artifacts.

Have you removed https://github.com/eclipse/jetty.project/blob/476874584943daf3041eb0ca584d4de3a87f19fd/pom.xml#L856 too? Because that tells the maven-bundle-plugin to not generate EE requirements.

joakime commented 1 year ago

Have you removed

https://github.com/eclipse/jetty.project/blob/476874584943daf3041eb0ca584d4de3a87f19fd/pom.xml#L856

too? Because that tells the maven-bundle-plugin to not generate EE requirements.

Removing the _noee is possible in Jetty 10 and Jetty 11. But causes problems in Jetty 12. That's because Jetty 12 is a core (with no EE), and environments. We call them ee8/ee9/ee10, but in reality they are just Servlet + WebSocket with no extra EE stuff. When we remove _noee then the first EE environment detected is (apparently) used for all further manifests, making everything, in all environments the same EE level, which is wrong.

HannesWell commented 1 year ago

What is the syntax you want us to use for Require-Capability ?

I would expect Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=17))" for jetty-server and for jetty-xml

Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=17))",
 osgi.serviceloader;filter:="(osgi.serviceloader=org.eclipse.jetty.xml.ConfigurationProcessorFactory)";resolution:=optional;cardinality:=multiple,
 osgi.extender;filter:="(osgi.extender=osgi.serviceloader.processor)";resolution:=optional

Please note that line-breaks are irrelevant (I formatted it nicely).

HannesWell commented 1 year ago

That's because Jetty 12 is a core (with no EE), and environments. We call them ee8/ee9/ee10, but in reality they are just Servlet + WebSocket with no extra EE stuff. When we remove _noee then the first EE environment detected is (apparently) used for all further manifests, making everything, in all environments the same EE level, which is wrong.

I'm not sure if we have a misunderstanding here. For me EE refers to the OSGi term Execution-Environment, which nowdays basically defines the minimum required Java version. I assume for you EE refers to the Java/Jakarta EE?

joakime commented 1 year ago

I have to run to an appointment.

But in short, when I remove the _noee I get all kinds of errors / warnings / failures (From the maven-bundle-plugin) during the build surrounding attempts to figure out the Jakarta EE level of the project being built (often not being able to figure out the Jakarta EE level). Seems it wants to add/update something in the Require-Capability configuration when it fails.

I'll be back in a few hours.

joakime commented 1 year ago

OK, I'm back.

So, here's an example of removing <_noee>true</_noee> on Jetty 12 when compiling with JDK 21. (which is required for the release process)

The current maven-bundle-plugin configuration at top level /pom.xml

        <plugin>
          <groupId>org.apache.felix</groupId>
          <artifactId>maven-bundle-plugin</artifactId>
          <version>${maven.bundle.plugin.version}</version>
          <configuration>
            <supportedProjectTypes>
              <supportedProjectType>jar</supportedProjectType>
              <supportedProjectType>maven-plugin</supportedProjectType>
            </supportedProjectTypes>
            <instructions>
              <Bundle-SymbolicName>${bundle-symbolic-name}</Bundle-SymbolicName>
              <Bundle-Description>Jetty module for ${project.name}</Bundle-Description>
              <Bundle-DocURL>${jetty.url}</Bundle-DocURL>
              <Bundle-Vendor>Eclipse Jetty Project</Bundle-Vendor>
              <Bundle-Classpath>.</Bundle-Classpath>
              <Bundle-Copyright>Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.</Bundle-Copyright>
              <Import-Package>
                ${osgi.slf4j.import.packages},
                !java.io,
                !java.lang,
                !java.lang.annotation,
                !java.lang.invoke,
                !java.lang.reflect,
                !java.net,
                !java.nio.charset,
                !java.nio.file,
                !java.util,
                !java.util.function,
                !java.util.stream,
                !javax.xml.catalog,
                !javax.xml.parsers,
                *
              </Import-Package>
              <Require-Capability>
                ${osgi.require.capability},
                *
              </Require-Capability>
              <_provider-policy><![CDATA[$<range;[===,=+)>]]></_provider-policy>
              <_consumer-policy><![CDATA[$<range;[===,+)>]]></_consumer-policy>
            </instructions>
          </configuration>
        </plugin>

The relevant jetty-core/jetty-xml/pom.xml section.

      <plugin>
        <groupId>org.apache.felix</groupId>
        <artifactId>maven-bundle-plugin</artifactId>
        <extensions>true</extensions>
        <configuration>
          <instructions>
            <Require-Capability>
              osgi.serviceloader; filter:="(osgi.serviceloader=org.eclipse.jetty.xml.ConfigurationProcessorFactory)";resolution:=optional;cardinality:=multiple,
              osgi.extender; filter:="(osgi.extender=osgi.serviceloader.processor)";resolution:=optional
            </Require-Capability>
          </instructions>
        </configuration>
      </plugin>

The resulting META-INF/MANIFEST.MF

Manifest-Version: 1.0
Build-Jdk-Spec: 21
Bundle-Classpath: .
Bundle-Copyright: Copyright (c) 1995 Mort Bay Consulting Pty Ltd and oth
 ers.
Bundle-Description: Jetty module for Core :: XML
Bundle-DocURL: https://eclipse.dev/jetty/
Bundle-License: https://www.eclipse.org/legal/epl-2.0/, https://www.apac
 he.org/licenses/LICENSE-2.0
Bundle-ManifestVersion: 2
Bundle-Name: Core :: XML
Bundle-SymbolicName: org.eclipse.jetty.xml
Bundle-Vendor: Eclipse Jetty Project
Bundle-Version: 12.0.2.SNAPSHOT
Created-By: Apache Maven Bundle Plugin 5.1.9
Export-Package: org.eclipse.jetty.xml;uses:="org.eclipse.jetty.util.anno
 tation,org.eclipse.jetty.util.component,org.eclipse.jetty.util.resource
 ,org.xml.sax";version="12.0.2"
Implementation-Vendor: Eclipse Jetty Project
Implementation-Version: 12.0.2-SNAPSHOT
Import-Package: org.slf4j;version="[1.7,3.0)",org.eclipse.jetty.util;ver
 sion="[12.0.2,13)",org.eclipse.jetty.util.annotation;version="[12.0.2,1
 3)",org.eclipse.jetty.util.component;version="[12.0.2,13)",org.eclipse.
 jetty.util.resource;version="[12.0.2,13)",org.eclipse.jetty.util.thread
 ;version="[12.0.2,13)",org.w3c.dom,org.xml.sax,org.xml.sax.helpers,org.
 slf4j.event;version="[1.7,3.0)",org.slf4j.helpers;version="[1.7,3.0)",o
 rg.slf4j.spi;version="[1.7,3.0)"
Require-Capability: osgi.serviceloader;filter:="(osgi.serviceloader=org.
 eclipse.jetty.xml.ConfigurationProcessorFactory)";resolution:=optional;
 cardinality:=multiple,osgi.extender;filter:="(osgi.extender=osgi.servic
 eloader.processor)";resolution:=optional,osgi.ee;filter:="(&(osgi.ee=Ja
 vaSE)(version=17))"
Tool: Bnd-6.3.1.202206071316
url: https://eclipse.dev/jetty/

This seems to add the osgi.ee entry in the Require-Capability entry. Is this configuration what you are looking for?

HannesWell commented 1 year ago

But in short, when I remove the _noee I get all kinds of errors / warnings / failures (From the maven-bundle-plugin) during the build surrounding attempts to figure out the Jakarta EE level of the project being built (often not being able to figure out the Jakarta EE level). Seems it wants to add/update something in the Require-Capability configuration when it fails.

I have to admit that this surprises me, because noee from the maven-bundle-plugin refers to the OSGi Execution Environment and actually has nothing to do with the Jakarta EE version. Unless there are some cross requirements based on the Java version or something like this.

Eventually _noee is this instruction (the doc is not the prettiest): https://bnd.bndtools.org/instructions/noee.html

joakime commented 1 year ago

I don't need this configuration in the top level pom, with _noee removed.

              <Require-Capability>
                ${osgi.require.capability},
                *
              </Require-Capability>

I was curious as to when <_noee>true</_noee> showed up. It first made an appearance in commit 4080cc6c3b764567dd6e52228656f73344f7cfa2 (Nov 23, 2018) That configuration has been present in every version of Jetty 10 / 11 / 12 (but not Jetty 9)

HannesWell commented 1 year ago

So, here's an example of removing <_noee>true</_noee> on Jetty 12 when compiling with JDK 21. (which is required for the release process)

The current maven-bundle-plugin configuration at top level /pom.xml

The removal of <Bundle-RequiredExecutionEnvironment> and <_noee> looks good, but the extra elements in <Import-Package> and the entire <Require-Capability> element seem not to be necessary, as far as I understood your build. Or did you add some osgi.require.capability property?

Adjustments in the sub-projects should not be necessary at all, just removing <Bundle-RequiredExecutionEnvironment> and <_noee> should do the job, but of course double-checking the result would be good.

I also tried to do the proposed changes locally but I fail to build the current jetty-12.0.x branch even unchanged. Maven fails with Required array size too large and setting MAVEN_OPTS=-Xms2g -Xmx4g as you do in your Jenkinsfile does not help. So I cannot test it locally.

This seems to add the osgi.ee entry in the Require-Capability entry. Is this configuration what you are looking for?

Yes that's what we want eventually. :)

joakime commented 1 year ago

I also tried to do the proposed changes locally but I fail to build the current jetty-12.0.x branch even unchanged. Maven fails with Required array size too large and setting MAVEN_OPTS=-Xms2g -Xmx4g as you do in your Jenkinsfile does not help. So I cannot test it locally.

Some advice on Maven.

Use Maven 3.9.2 (or newer) Use this command line (to be on par with the CI builds)

# full build, CI style
mvn clean install -Pci
# full build, CI style, no tests
mvn clean install -Pci -DskipTests
janbartel commented 1 year ago

Fixed via #10666