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

Migrate to Jetty 12 #1447

Closed
merks opened this issue Oct 10, 2023 · 28 comments
Closed

Migrate to Jetty 12 #1447

merks opened this issue Oct 10, 2023 · 28 comments
Assignees
Labels
Dependencies Pull requests that update a dependency file
Milestone

Comments

@merks
Copy link
Contributor

merks commented Oct 10, 2023

With Equinox and the Platform migrating to Jetty 12 for 4.30 / 2023-12 it appears that BIRT is forced to upgrade as well because of its use of eclipse.equinox.http.jetty which has migrated.

@claesrosell
Copy link
Contributor

If I recall correctly, we were pulling Jetty bundles, in addition to the bundles that Equinox provided, from Maven when we got this to work the last time.
Yesterday, when I looked at the current BIRT target platform I saw that we seem to pull those from the https://download.eclipse.org/oomph/jetty/release/10.0.15 update site now. A corresponding update site for 12.0.x does not seem to exist yet.

I think that the additional Jetty bundles were:
org.eclipse.jetty.webapp;bundle-version="10.0.11",
org.eclipse.jetty.plus;bundle-version="10.0.11",
org.eclipse.jetty.osgi.boot;bundle-version="10.0.11",
org.eclipse.jetty.apache-jsp;bundle-version="10.0.11",

To support different EE versions: there has been some extensive restructuring of Jetty from 10/11 to 12 (jetty/jetty.project#7638) so the corresponding classes are in other bundles. If we can get access to all the Jetty bundles I suspect that the rest will be pretty straightforward.

@merks
Copy link
Contributor Author

merks commented Oct 10, 2023

For Orbit, I've created this p2 repository with all the jetty 12 bundles (as derived from their BOMs):

https://download.eclipse.org/tools/orbit/simrel/maven-jetty/nightly/latest

I've switched the MANIFEST.MF to use them:

image

I was just now testing how well the Tycho build works locally with all my changes.

I've already noted that strangely the first of these three don't exist:

One thing that struck me as odd looking at this yesterday is that of these three, only the latter to exist:

Even here I have not found replacements for OSGiServerConstants/OSGiWebappConstants

image

So for now I hard-code the String values instead.

What I'm hoping to do in the next while is create a PR such that you can check that PR out and continue from that point of progress.

merks added a commit to merks/birt that referenced this issue Oct 10, 2023
@merks
Copy link
Contributor Author

merks commented Oct 10, 2023

So the PR is there. After you get this into your workspace use the underlined toolbar button (also available as Help -> Perform Setup Tasks), double click the Module Target task (the p2 task doesn't need to run so note it's unchecked). There will update the target platform using the latest update sites as underlined.

image

Note that the only hand modified part of the *.target is the two versions for org.mortbay.jasper.

You can add things to the target platform simply by adding them to a feature or to a MANIFEST.MF; even if that is an error at first, when you resolve the target platform using the above steps, those requirements will be taken into account and added to the target platform automatically...

When you launch a Runtime Workspace just ignore the constraint errors and continue the launch.

The IDE works but the web browser doesn't and nothing is logged when the apps launched:

image

Again, don't be afraid to ask questions.

@merks
Copy link
Contributor Author

merks commented Oct 10, 2023

I don't know if this oversight is a problem for migration:

jetty/jetty.project#10688

Note that 12.0.2 was just released now, so I will produce a build for it...

@merks
Copy link
Contributor Author

merks commented Oct 10, 2023

Actually I'll hold off on the 12.0.2 build until I see the Platform adopt 12.0.2:

eclipse-platform/eclipse.platform.releng.aggregator#1430

@claesrosell
Copy link
Contributor

@merks : I will look into this later today, so you may wake up to some questions.
With that said: I think that the missing ee8-osgi will pose a problem. At least as the WebAppContext is currently set up.

@claesrosell
Copy link
Contributor

I have put some hours into this, most of them to get a SLF4J2 binding in place. :-)
My conclusion is that we probably need the osgi-ee8 stuff to get this to work without big changes to how we set up the web application.

Here is a summary, from "the top of my head"

The OSGI documentation for Jetty 10+ is a bit sparse, but the OSGI documentation for Jetty 9 was pretty good: https://eclipse.dev/jetty/documentation/jetty-9/index.html#framework-jetty-osgi and is still valid to some point. With the OSGI layer in place, Jetty will track OSGI services for Webapps, Contexts, and even Servers and configure and/or attach them to running servers.
This is what the BIRT Webviewer is doing. First, we create and register a Server, then we create a WebAppContext and register that as well. Jetty-OSGI will pick the WebAppContext and register it with the server., restarting it if required. It will also handle the configuration from the deployment descriptor (web.xml), setting up contexts and filters if needed.
But since the osgi-layer isn't available, nothing of this happens.

Some of the Jetty-osgi layer seem to be available in jetty-core/jetty-osgi . The OSGiServerConstants as well. I will look more into that tomorrow, perhaps all we need is available there.

All this is ofcause possible to do without the Jetty-osgi layer and I have done parts of it before, since the company I am working for uses an embedded Jetty 10 with OSGI, but has moved away from the Jetty-osgi layer. However, there was a lot of pain to get it to work and we are not even using deployment descriptors.

@olamy
Copy link

olamy commented Oct 12, 2023

The Jetty project has started a PR to add OSGI artifacts for Jetty 12 ee8.
Please test it.
jetty/jetty.project#10711
We will appreciate any help you can provide.

@merks
Copy link
Contributor Author

merks commented Oct 12, 2023

@olamy

I'm not sure how we can test this. Are there builds available somewhere? I can't find any information about where one might get Jetty beyond the release zips here:

https://eclipse.dev/jetty/download.php

@olamy
Copy link

olamy commented Oct 12, 2023

@merks there will be snapshots deployed but only when the PR will be merged.
now you can just build locally:

git clone https://github.com/eclipse/jetty.project/
cd jetty.project
git switch jetty-12.0.x-ee8-osgi
mvn install -Pfast

@claesrosell
Copy link
Contributor

I will build Jetty locally and use those bundles for testing.

@claesrosell
Copy link
Contributor

Tried to build that branch locally with:

git clone https://github.com/eclipse/jetty.project/
cd jetty.project
git switch jetty-12.0.x-ee8-osgi
mvn install -Pfast

These steps are currently not enough to build this branch of Jetty locally.
First, it complained about:
<modify-sources-plugin.version>1.0.7-SNAPSHOT</modify-sources-plugin.version>
which it couldn't find, I tried switching back to 1.0.6, to see what happened, but then the build failed with:

[ERROR] Failed to execute goal on project jetty-ee8-maven-plugin: Could not resolve dependencies for project org.eclipse.jetty.ee8:jetty-ee8-maven-plugin:maven-plugin:12.0.3-SNAPSHOT: The following artifacts could not be resolved: org.eclipse.jetty:jetty-home:zip:12.0.3-SNAPSHOT (absent): Could not find artifact org.eclipse.jetty:jetty-home:zip:12.0.3-SNAPSHOT

Since the change to 1.0.7-SNAPSHOT was made, I guess that change is necessary to build. But as it is now I can not test this locally.

@claesrosell
Copy link
Contributor

Forget what I just wrote, the build works with
<modify-sources-plugin.version>1.0.6</modify-sources-plugin.version>
... I was in the wrong folder when I tried to build the second time. Sorry

@olamy
Copy link

olamy commented Oct 12, 2023

ah yes 1.0.7-SNAPSHOT I have to release this.
I will try now.

@speckyspooky
Copy link
Contributor

@merks
I merged the latest changes from the BIRT-master to my fork to create a new fix.
When I start the viewer with the current dependencies the viewer which is started from eclipse will be ended in ERROR-500.

Please confirm that this is due to the current mixed situation of the platform 2023-09 and the Jetty-version-topic.
(Or do we have another topic with "javax/xml/rpc" and the local Jetty viewer should work...?)

dialog local viewer 2023-09

@olamy
Copy link

olamy commented Oct 12, 2023

@claesrosell you can return to <modify-sources-plugin.version>1.0.7</modify-sources-plugin.version> this is now in maven central and btw updated in the branch

@hvbtup
Copy link
Contributor

hvbtup commented Oct 13, 2023

I'm in the same situation as Thomas ATM.

@claesrosell
Copy link
Contributor

claesrosell commented Oct 13, 2023

@merks I merged the latest changes from the BIRT-master to my fork to create a new fix. When I start the viewer with the current dependencies the viewer which is started from eclipse will be ended in ERROR-500.

Please confirm that this is due to the current mixed situation of the platform 2023-09 and the Jetty-version-topic. (Or do we have another topic with "javax/xml/rpc" and the local Jetty viewer should work...?)

dialog local viewer 2023-09

I created a new issue #1451 for this.

@speckyspooky speckyspooky added this to the 4.14 milestone Oct 14, 2023
@speckyspooky speckyspooky added the Dependencies Pull requests that update a dependency file label Oct 14, 2023
@claesrosell
Copy link
Contributor

I just wanted to inform you that I have fallen ill and have not been able to put any time into this for the last couple of days. Hopefully, I will soon be better so I can try the new 12.0.3 build of Jetty which includes initial support of the OSGI layer for EE8.

@claesrosell
Copy link
Contributor

I have got the WebViewer working to some capacity in my development environment. The context path is wrong and there are probably other things that don't work as well. But at least the WebAppContext is up-n-running on Jetty-12.
image
But I did stumble upon the issues mentioned in #1451as well, which is unrelated to Jetty-12

@merks
Copy link
Contributor Author

merks commented Oct 24, 2023

Having something that works in some capacity is a great and huge improvement over the blank window or stack traces window!

I suppose we need the 12.0.3 release to be available to have this also working in the builds and on other developer machines?

@olamy
Copy link

olamy commented Oct 24, 2023

Can you try with adding some instructions such:
Web-ContextPath
Jetty-Environment ee8

hopefully, we should cut a release soon.
Thanks for your testing time!

@merks
Copy link
Contributor Author

merks commented Oct 24, 2023

Thank you both!! 🥇

@claesrosell
Copy link
Contributor

I have something that is "working" now, but I cannot get it to build.

It is the build of "org.eclipse.birt.chart.ui" that fails with:

Error:  Failed to execute goal org.eclipse.tycho:target-platform-configuration:2.7.5:target-platform (default-target-platform) on project org.eclipse.birt.chart.ui: Execution default-target-platform of goal org.eclipse.tycho:target-platform-configuration:2.7.5:target-platform failed: The Maven artifact to be added to the target platform is not stored at the required location on disk: required "/home/runner/.m2/repository/jakarta/annotation/jakarta.annotation-api/1.3.5/jakarta.annotation-api-1.3.5.jar" but was "/home/runner/.m2/repository/jakarta/annotation/jakarta.annotation-api/2.1.1/jakarta.annotation-api-2.1.1.jar" -> [Help 1]

@merks : Is the target platform automatically generated upon build by Maven/Tycho as well, or do I need to commit the automatically generated target-platform ?

@merks
Copy link
Contributor Author

merks commented Oct 25, 2023

It must be committed.

@claesrosell
Copy link
Contributor

claesrosell commented Oct 27, 2023

I have digged more into the build issue that I have and it seems like it is not available in version 4.0.3 of Tycho.

Here is the error again, better formatted this time:

Error:  Failed to execute goal org.eclipse.tycho:target-platform-configuration:2.7.5:target-platform (default-target-platform) on project org.eclipse.birt.chart.ui:
Execution default-target-platform of goal org.eclipse.tycho:target-platform-configuration:2.7.5:target-platform failed: 
The Maven artifact to be added to the target platform is not stored at the required location on disk: required
"/home/runner/.m2/repository/jakarta/annotation/jakarta.annotation-api/1.3.5/jakarta.annotation-api-1.3.5.jar"
but was
"/home/runner/.m2/repository/jakarta/annotation/jakarta.annotation-api/2.1.1/jakarta.annotation-api-2.1.1.jar" -> [Help 1]

I think that Tycho is confused by the fact that org.eclipse.e4.core.di has Import-Package headers for packages that are exported from two different bundles that have the same Bundle-SymbolicName ("jakarta-annotation-api") but with different version numbers(1.3.5, 2.1.1, respectively). That is a guess though.

With that said, un upgrade from Tycho 2.7.5 to 4.0.4 solved that problem, but introduced a new one. Maven complains about a cycle in the graph

Edge between 'Vertex{label='org.eclipse.birt:org.eclipse.birt.report.designer.tests:4.14.0-SNAPSHOT'}' and 'Vertex{label='org.eclipse.birt:org.eclipse.birt.report.designer.ui:4.14.0-SNAPSHOT'}' introduces to cycle in the graph
org.eclipse.birt:org.eclipse.birt.report.designer.ui:4.14.0-SNAPSHOT -->
org.eclipse.birt:org.eclipse.birt.report.engine:4.14.0-SNAPSHOT -->
org.eclipse.birt:org.eclipse.birt.report.designer.tests:4.14.0-SNAPSHOT -->
org.eclipse.birt:org.eclipse.birt.report.designer.ui:4.14.0-SNAPSHOT}

disabling that test makes the build of the Jetty-12 stuff work.

@merks
Copy link
Contributor Author

merks commented Oct 27, 2023

Maybe the mentioning of x-friends is confusing something into thinking there is dependency in the wrong direction:

 org.eclipse.birt.report.engine.internal.document;x-friends:="org.eclipse.birt.report.engine.tests",

Let's not forget about any follow up work we need to do after this is complete, i.e., to re-enable this test after we figure how to do that...

claesrosell added a commit that referenced this issue Oct 30, 2023
The Jetty server and WebAppContext is now configured programmatically
and without the OSGI layer

---------

Co-authored-by: Ed Merks <ed.merks@gmail.com>
@olamy
Copy link

olamy commented Oct 30, 2023

Thanks for your testing. Jetty 12.0.3 has been released.
Please let use if you have more issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

5 participants