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

Upgrade Jetty to 9.x #8712

Closed
dankurka opened this issue Jun 10, 2015 · 20 comments
Closed

Upgrade Jetty to 9.x #8712

dankurka opened this issue Jun 10, 2015 · 20 comments
Assignees
Milestone

Comments

@dankurka
Copy link
Member

Originally reported on Google Code with ID 8747

Jetty 8.1 is outdated, please upgrade to the Jetty release 9.x.

Found in GWT Release: trunk

Detailed description (please be as specific as possible):

If project using GWT depends on Jetty 9.1 then it collides with the Jetty 8.1 packaged
in gwt-dev.jar.

Workaround if you have one:

The only known workaround to make it work is to strip Jetty packages from gwt-dev.jar
and use
more recent Jetty 9.1 installation during fetching from Maven repository.

Unfortunately this works only unless gwt-codeserver.jar is used. Trying to set up SuperDevMode

and start codeserver with stripped gwt-dev.jar refuses to work:

Compile completed in 21470 ms
java.lang.NoClassDefFoundError: org/eclipse/jetty/server/nio/SelectChannelConnector
    at com.google.gwt.dev.codeserver.WebServer.start(WebServer.java:108)
    at com.google.gwt.dev.codeserver.CodeServer.start(CodeServer.java:101)
    at com.google.gwt.dev.codeserver.CodeServer.main(CodeServer.java:71)
    at com.google.gwt.dev.codeserver.CodeServer.main(CodeServer.java:49)


Reported by David.Ostrovsky on 2014-06-03 10:43:17

@dankurka
Copy link
Member Author

Htmlunit is still using Jetty 8.1. That's why I upgraded to that version.

Reported by goktug@google.com on 2014-06-03 21:01:57

@dankurka dankurka self-assigned this Jun 10, 2015
@dankurka
Copy link
Member Author

Can we do this before IO?

Reported by branflake2267 on 2014-06-03 21:02:22

@dankurka
Copy link
Member Author

OK, i see. It seems like monolithic jetty-websocket underwent major redesign in Jetty
9.X
and was splited into different maven modules [1].

OTOH we don't use HtmlUnit at all. As it was pointed out in these changes [2], [3]
we could just
strip Jetty entirely from gwt-dev.jar and upgrade it to 9.1 and everything just work.

With one exception CodeServer.

I wonder if another approach would be not to package Jetty (and HtmlUnit ?) in gwt-dev.jar
at all,
and instead add Maven dependency to gwt-dev-pom.xml like this is already the case for
org.json package?

In addition include gwt-codeserver-jetty-9.X.jar in distribution that is compatible
with latest and greatest
Jetty version.

That would simplify life for projects that want to include newest Jetty version, and
strip the outdated
Jetty version currently packaged in gwt-dev.jar anyway, and would (hopefully) fix broken
SuperDevMode.

[1] https://webtide.com/jetty-9-updated-websocket-api
[2] https://gerrit-review.googlesource.com/51722
[3] https://gerrit-review.googlesource.com/53510

Reported by David.Ostrovsky on 2014-06-04 09:52:53

@dankurka
Copy link
Member Author

We're back to dependency-hell that could be solved with modularization.

The current uses of Jetty in GWT are: DevMode's default ServletContainerLauncher, SuperDevMode
and GWT-JUnit.

If we could move DevMode out of gwt-dev (e.g. gwt-devmode) then we could remove Jetty
from gwt-dev (add dependencies from gwt-user, codeserver and gwt-devmode to Jetty for
Maven, and packaging it in the GWT SDK, like we package validation-api).

I think we can do that by I/O.
It won't solve Gerrit issues though, just move the problem: they just wouldn't need
to strip Jetty from gwt-dev as it would no longer be bundled, but still have to bring
it in the classpath when launching CodeServer (they don't have GWTTestCase), which
currently requires a small refactoring of the build scripts, and pulling Jetty 8 from
Central only for CodeServer.

In the future, we could similarly extract GWT-JUnit into its own set of JARs: one with
the API to compile your code against, and one with the "runtime"; only the runtime
needs Jetty.
We could technically have GWT-JUnit depend on the same version of Jetty that's used
by HtmlUnit, and CodeServer depend on another version, but it could become a nightmare
for GWT users to manage their classpaths!

@David: as I said on the Gerrit forum, ideally you'd manage 3 classpaths: build classpath,
server-side runtime classpath and GWT classpath (used by CodeServer and GWT compiler).
build and server-side runtime classpaths can use Jetty 9.x, and GWT classpath (CodeServer
only, and/or GWT-JUnit for those who have GWTTestCase) can include Jetty 8.x.

Reported by t.broyer on 2014-06-04 10:25:34

@dankurka
Copy link
Member Author

Sorry I shouldn't have mentioned a can you do it by, kicking my self for saying that
after I posted. 

I'm looking at using one of the runners to use with the enhancement I'm working on
and this particular issue might play apart. I'll bring it up later when I cross that
bridge.

Thanks for more details, that helps with what I'm working on at the moment. 

Reported by branflake2267 on 2014-06-04 14:29:59

@dankurka
Copy link
Member Author

I've done most of the work (that I experienced as necessary) to upgrade the in-built
jetty to version 9 (this is for the "classic" dev mode launcher) as it was necessary
to bootstrap my spring-based server code via annotations. I don't know how "correct"
it is. Particularly, I didn't update the SSL configuration logic to work I simply commented
it out or deleted it, since I don't use it.

This may not be the "whole problem" but as far as running jetty 9 in dev mode it met
my needs.

Reported by heptet2014 on 2014-06-10 03:35:36

@dankurka
Copy link
Member Author

Change under review:

https://gwt-review.googlesource.com/7856

Reported by David.Ostrovsky on 2014-06-15 06:32:53

@dankurka
Copy link
Member Author

I filed issue for HtmlUnit project to upgrade to Jetty 9: [1].

https://sourceforge.net/p/htmlunit/bugs/1656/

Reported by David.Ostrovsky on 2014-12-20 12:33:51

@dankurka
Copy link
Member Author

Patch that migrates htmlunit to Jetty 9.2.6 was merged yesterday. htmlunit doesn't depend
on Jetty 8 anymore. htmlunit 2.16 snapshot artifacts can be doenloaded here: [1].

Can we move forward with this patch: [2]?

[1] https://ci.canoo.com/teamcity/viewLog.html?buildId=137468&buildTypeId=Htmlunit_CheckInBuild&tab=artifacts

[2] https://gwt-review.googlesource.com/#/c/7857/

Reported by David.Ostrovsky on 2015-01-14 20:18:46

@dankurka
Copy link
Member Author

I have no objections to upgrading to Jetty 9. However, I'll point out that another approach
would be to launch Super Dev Mode separately and use -launcherDir to write the files
to your Jetty 9 war dir, so that they can have two different classpaths.

Here's a post about some work Brandon Donnelson is doing in GPE that allows support
for arbitrary servlet engines, not just Jetty:
https://plus.google.com/111739836936169749229/posts/1BKe5jsaUCR

Reported by bslesinsky on 2015-01-18 04:26:22

@dankurka
Copy link
Member Author

> Htmlunit is still using Jetty 8.1. That's why I upgraded to that version.

I fixed that: HtmlUnit 2.16 uses Jetty 9.2.10.

> I have no objections to upgrading to Jetty 9.

Can we consider to fix this issue and merge [1] before GWT 2.8 final release?

[1] https://gwt-review.googlesource.com/7857

Reported by David.Ostrovsky on 2015-05-17 11:58:08

@dankurka
Copy link
Member Author

I think it would be a good thing; let's tentatively add it to the milestone.

Reported by t.broyer on 2015-05-17 16:35:52

  • Labels added: Milestone-2_8

@dankurka
Copy link
Member Author

> let's tentatively add it to the milestone 2.8.

Thanks.

Can someone change the status to ReviewPending?

Reported by David.Ostrovsky on 2015-05-17 18:19:56

@dankurka
Copy link
Member Author

Reported by t.broyer on 2015-05-19 13:38:54

  • Status changed: ReviewPending

@tbroyer tbroyer modified the milestone: 2.8 Jun 20, 2015
@pgtaboada
Copy link

What is the status of this issue?

@wisebaldone
Copy link

Is there anyway that this can be pushed up in priority

@davido
Copy link
Contributor

davido commented Dec 4, 2015

It's done and is ready to be merged: [1].

@wisebaldone
Copy link

@davido you still need to put the jetty 9 libs in the tools repo from talking to devs on ##gwt so that the automated tests can work.

@tbroyer
Copy link
Member

tbroyer commented Dec 4, 2015

I think you can make a pull request on the tools repo

@davido
Copy link
Contributor

davido commented Dec 5, 2015

I think you can make a pull request on the tools repo

gwtproject/tools#3

@gkdn gkdn assigned tbroyer and unassigned dankurka Jan 22, 2016
niloc132 pushed a commit to niloc132/gwt-playground that referenced this issue Feb 29, 2016
The Jetty 9.2 quick start mechanism provides almost an order of
magnitude improvement in start time [1]. Moreover since Jetty 8
is horribly outdated, more and more projects that depend on GWT
upgraded to current Jetty 9 and because of incompatibility between
Jetty 8 and Jetty 9 have classpath collisions, that can be only
resolved with very ugly hacks, including forking of part of GWT
source code: [2].

Because HtmlUnit project, used for test toolchain had dependency
on outdated Jetty 8 (WebSocket) we had not an upgrade path here.
Luckily, HtmlUnit was recently upgraded to use Jetty 9.2.x: [3].

The following transitive dependencies were updated:

* commons-codec: 1.10
* commons-lang: 3.4
* commons-logging: 1.2
* cssparser: 0.9.18
* htmlunit: 2.19
* htmlunit-core-js: 2.15
* httpclient: 4.5.1
* httpcore: 4.4.4
* httpmime: 4.5.1
* jetty: 9.2.14.v20151106
* log4j: 1.2.17
* nekohtml: 1.9.22
* slf4j-api: 1.7.12
* slf4j-log4j12: 1.7.12
* tomcat-servlet-api: 8.0.28

TEST PLAN:

  $ cd user && ant test.dev.htmlunit test.draft.htmlunit

Note, that animation tests were disabled, because they break with
htmlunit-2.19. However unchanged code works against 2.18.  So that
we can alternatively upgrade to older version of the library,
instead of disabling the tests. See code comments for more details.

[1] https://webtide.com/jetty-9-quick-start
[2] https://gerrit-review.googlesource.com/59354
[3] https://sourceforge.net/p/htmlunit/bugs/1656

Bug-Link: gwtproject/gwt#8712
Change-Id: Id1d338a01f4fc72b2adf2f43b57aeddfb06cfec1
@tbroyer tbroyer closed this as completed Feb 29, 2016
openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Sep 20, 2016
There are number of important changes in this GWT release:

1. HtmlUnit, Jetty and their dependencies upgrade: [1],[2],[3]
2. Unbundled most dependencies from GWT Maven artifacts

With 1. we can remove patched WebServer fork from GWT project, that
was needed to adapt to Jetty 9 version. With 2. we don't need to
strip Jetty classes from gwt-dev artifact any more to avoid classpath
collisions (we wouldn't get classpath collisions to start with because
of 1.). However, because of 2. we need to add quite some dependencies
that we got for granted in gwt-dev in early GWT releases on our own:

* Apache Ant, Apache 2.0 License
* Apache Tapestry, Apache 2.0 License
* CERN colt, CERN own or LGPL License
* Google JS Interop annotations, Apache 2.0 License
* W3C CSS sac library, License W3C IPR SOFTWARE NOTICE

According to the release notes: [4].

  Double/Boolean are not boxed anymore

That leads to the NPE when such method:

  void showLineEndings(boolean s);

is used with null value:

  p.showLineEndings(in.showLineEndings);

where in.showLineEndings is null. Adapt the code to accept
Boolean instead.

gwtjsonrpc is updated to version 1.10 which is built for GWT 2.8.0-rc2.

TEST PLAN:

* Verified that daemon works
* Verified that SDM works

[1] gwtproject/gwt#8712
[2] https://sourceforge.net/p/htmlunit/bugs/1656/
[3] https://gwt-review.googlesource.com/7857
[4] http://www.gwtproject.org/release-notes.html#Release_Notes_2_8_0_RC1

Change-Id: I3f009f3ef0cbb8bafac236fb9a81c951697a5903
davido added a commit to davido/gerrit that referenced this issue Sep 6, 2019
There are number of important changes in this GWT release:

1. HtmlUnit, Jetty and their dependencies upgrade: [1],[2],[3]
2. Unbundled most dependencies from GWT Maven artifacts

With 1. we can remove patched WebServer fork from GWT project, that
was needed to adapt to Jetty 9 version. With 2. we don't need to
strip Jetty classes from gwt-dev artifact any more to avoid classpath
collisions (we wouldn't get classpath collisions to start with because
of 1.). However, because of 2. we need to add quite some dependencies
that we got for granted in gwt-dev in early GWT releases on our own:

* Apache Ant, Apache 2.0 License
* Apache Tapestry, Apache 2.0 License
* CERN colt, CERN own or LGPL License
* Google JS Interop annotations, Apache 2.0 License
* W3C CSS sac library, License W3C IPR SOFTWARE NOTICE

According to the release notes: [4].

  Double/Boolean are not boxed anymore

That leads to the NPE when such method:

  void showLineEndings(boolean s);

is used with null value:

  p.showLineEndings(in.showLineEndings);

where in.showLineEndings is null. Adapt the code to accept
Boolean instead.

gwtjsonrpc is updated to version 1.10 which is built for GWT 2.8.0-rc2.

TEST PLAN:

* Verified that daemon works
* Verified that SDM works

[1] gwtproject/gwt#8712
[2] https://sourceforge.net/p/htmlunit/bugs/1656/
[3] https://gwt-review.googlesource.com/7857
[4] http://www.gwtproject.org/release-notes.html#Release_Notes_2_8_0_RC1

Change-Id: I3f009f3ef0cbb8bafac236fb9a81c951697a5903
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

No branches or pull requests

5 participants