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

Issue #3162 - Use Jetty specific Servlet API jar. #3168

Merged
merged 38 commits into from
Feb 7, 2019

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Nov 29, 2018

  • Updated module-info.java to reference the "jetty.servlet.api" module.
  • Updated POMs to reference the o.e.j.toolchain:jetty-servlet-api artifact.
  • Removed references to jetty-schemas.jar.
  • Updated attribute "org.eclipse.jetty.server.webapp.ContainerIncludeJarPattern"
    to match the new Jetty Servlet API jar.

Signed-off-by: Simone Bordet simone.bordet@gmail.com

* Updated module-info.java to reference the "jetty.servlet.api" module.
* Updated POMs to reference the o.e.j.toolchain:jetty-servlet-api artifact.
* Removed references to jetty-schemas.jar.
* Updated attribute "org.eclipse.jetty.server.webapp.ContainerIncludeJarPattern"
  to match the new Jetty Servlet API jar.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@@ -13,8 +13,7 @@ logging
threadpool

[lib]
lib/servlet-api-4.0.jar
lib/jetty-schemas-4.0.jar
lib/jetty-servlet-api-4.0.0-SNAPSHOT.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this to lib/jetty-servlet-api-4.*.jar

@@ -105,7 +105,7 @@ The following artifacts are CDDL + GPLv2 with classpath exception.

https://glassfish.dev.java.net/nonav/public/CDDL+GPL.html

org.eclipse.jetty.toolchain:jetty-schemas
org.eclipse.jetty.toolchain:jetty-servlet-api
Copy link
Contributor

@joakime joakime Nov 29, 2018

Choose a reason for hiding this comment

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

@jmcc0nn3ll and @janbartel since this new artifact jetty-servlet-api is now ...

  • CDDL
  • and GPLv2 with Classpath Exception
  • and Eclipse License
  • and Apache License

... wouldn't we need to update this NOTICE file as well?

Or are the other sections of the NOTICE.txt sufficient for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@joakime actually, our standard NOTICE file covers everything already, so that's the one that we should use.

sbordet and others added 15 commits November 29, 2018 17:29
Future proof reference of the jetty-servlet-api.jar.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Removed references to jetty-osgi-servlet-api.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
…ng in the way)

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
…ke on Windows, or within an IDE)

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
…t (client) on OSGi

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@janbartel
Copy link
Contributor

@sbordet what's happening on this pr?

@janbartel
Copy link
Contributor

Now I'm confused: why is this PR for servlet-api 3 when it is targeted for jetty-10???

@joakime
Copy link
Contributor

joakime commented Jan 30, 2019

@janbartel the original goal of this PR is to have a servlet-api jar that is JPMS friendly.
There's a "jetty" servlet api jar that is Servlet 4.0 with JPMS friendly manifest.
https://oss.sonatype.org/content/repositories/jetty-snapshots/org/eclipse/jetty/toolchain/jetty-servlet-api/4.0.1-SNAPSHOT/

Where do you see the Servlet 3.0 stuff?

joakime and others added 6 commits January 30, 2019 11:56
…3162_jetty_servlet_api

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

# Conflicts:
#	jetty-client/pom.xml
#	jetty-http/pom.xml
#	jetty-http2/http2-client/pom.xml
#	jetty-http2/http2-http-client-transport/pom.xml
#	jetty-http2/http2-server/pom.xml
#	jetty-osgi/test-jetty-osgi/pom.xml
#	jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-with-custom-class.xml
#	jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty.xml
#	jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootHTTP2Conscrypt.java
#	jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootWithAnnotations.java
#	jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootWithJavaxWebSocket.java
#	jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootWithWebSocket.java
#	jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestOSGiUtil.java
#	jetty-quickstart/pom.xml
#	jetty-servlets/pom.xml
#	jetty-util/pom.xml
#	jetty-websocket/javax-websocket-client/pom.xml
#	jetty-websocket/javax-websocket-server/pom.xml
#	jetty-websocket/jetty-websocket-server/pom.xml
#	pom.xml
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Introduced ClientConnector and refactored HttpClient transports,
removing duplicated code that was connect() to a remote host.

Refactored also HTTP2Client to reference ClientConnector.

Refactored tests accordingly to the changes introduced in the
implementations.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Added name to default executor and scheduler after review.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet and others added 12 commits February 4, 2019 18:49
Rewrote HttpClientTransportOverUnixSockets in light of ClientConnector.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Reverted refactoring of newConnection() to avoid
to bind the class to a too specific abstract method.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
* Updated module-info.java to reference the "jetty.servlet.api" module.
* Updated POMs to reference the o.e.j.toolchain:jetty-servlet-api artifact.
* Removed references to jetty-schemas.jar.
* Updated attribute "org.eclipse.jetty.server.webapp.ContainerIncludeJarPattern"
  to match the new Jetty Servlet API jar.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Removed references to jetty-osgi-servlet-api.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Updated to jetty-servlet-api:4.0.1, based on javax.servlet 4.0.1.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Updated to jetty-servlet-api:4.0.2, based on jakarta.servlet 4.0.2.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Updated references to Servlet jar to "jetty-servlet-api".

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

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

I'm good with the current state of the PR

@sbordet
Copy link
Contributor Author

sbordet commented Feb 7, 2019

@janbartel I have addressed your concerns with "containerIncludeJarPattern".

@sbordet sbordet dismissed janbartel’s stale review February 7, 2019 16:31

Fixed your concerns.

@sbordet sbordet merged commit e2506d4 into jetty-10.0.x Feb 7, 2019
@sbordet sbordet deleted the jetty-10.0.x-3162_jetty_servlet_api branch February 7, 2019 16:33
@@ -65,7 +65,7 @@ Jetty Server Classpath:
Version Information on 11 entries in the classpath.
Note: order presented here is how they would appear on the classpath.
changes to the --module=name command line options will be reflected here.
0: 3.1.0 | ${jetty.home}/lib/servlet-api-3.1.jar
0: 3.1.0 | ${jetty.home}/lib/jetty-servlet-api-4.0.2.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

whoops. missed the version identifier before the |

@@ -158,7 +158,7 @@ Note: order presented here is how they would appear on the classpath.
1: 1.4.1.v201005082020 | ${jetty.base}/lib/ext/javax.mail.glassfish-1.4.1.v201005082020.jar
2: {VERSION} | ${jetty.base}/lib/ext/test-mock-resources-{VERSION}.jar
3: (dir) | ${jetty.home}/resources
4: 3.1.0 | ${jetty.home}/lib/servlet-api-3.1.jar
4: 3.1.0 | ${jetty.home}/lib/jetty-servlet-api-4.0.2.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto, the version s/3.1.0/4.0.2/

@@ -162,7 +162,7 @@ Jetty Server Classpath:
Version Information on 7 entries in the classpath.
Note: order presented here is how they would appear on the classpath.
changes to the --module=name command line options will be reflected here.
0: 3.1.0 | ${jetty.home}/lib/servlet-api-3.1.jar
0: 3.1.0 | ${jetty.home}/lib/jetty-servlet-api-4.0.2.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

here too. s/3.1.0/4.0.2/

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.

4 participants