Issue 140 rebased to async-support with fixes by gregw · Pull Request #150 · GoogleCloudPlatform/appengine-java-vm-runtime

gregw · 2016-01-06T07:29:09Z

This is not ready to merge. Created for feedback for #140 against #141

This adds a jetty9-compat-base module, which produces a jetty base artifact which can be run directly with a command like:

java \
-Droot.webapp=../webapps/testwebapp/ \
-Dcom.google.apphosting.logs=./logs \
-jar ../jetty-distribution-9.3.5.v20151012/start.jar \
--module=testMetadataServer

I still need to add support for compiling quickstart.

…pat image. GoogleCloudPlatform#140

…ies. Added a README.md

ludoch · 2016-01-06T23:29:16Z

Use the schema:

<assembly xmlns="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.3" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.3 http://maven.apache.org/xsd/assembly-1.1.3.xsd"> <id>assembly</id>

…te quickstart GoogleCloudPlatform#140

gregw · 2016-01-07T01:52:26Z

@ludoch This now can do quickstart generation. You can generate with:

cd jetty9-compat-base/target/jetty-base
java \
 -Droot.webapp=../webapps/testwebapp/ \
 -Dcom.google.apphosting.logs=/tmp \
 -Dquickstart_web_xml=../webapps/testwebapp/WEB-INF/quickstart-web.xml \
 -jar ../jetty-distribution-9.3.5.v20151012/start.jar \
 --module=testMetadataServer

and then run with

java \
 -Droot.webapp=../webapps/testwebapp/ \
 -Dcom.google.apphosting.logs=/tmp \
 -jar ../jetty-distribution-9.3.5.v20151012/start.jar \
 --module=testMetadataServer

Note also, to see logging to the console, just remove resources/jetty-logging.properties

ludoch · 2016-01-07T02:25:55Z

Great... Can you give the same commands when jettyhome and jettbase are
fully specified? That would make my life easy as I am still debating about
their new locations in the sdk . Also I assume that root.webapp can be
absolute?

On Wed, Jan 6, 2016 at 5:52 PM, Greg Wilkins notifications@github.com
wrote:

@ludoch https://github.com/ludoch This now can do quickstart
generation. You can generate with:

cd jetty9-compat-base/target/jetty-base
java
-Droot.webapp=../webapps/testwebapp/
-Dcom.google.apphosting.logs=/tmp
-Dquickstart_web_xml=../webapps/testwebapp/WEB-INF/quickstart-web.xml
-jar ../jetty-distribution-9.3.5.v20151012/start.jar
--module=testMetadataServer

and then run with

java
-Droot.webapp=../webapps/testwebapp/
-Dcom.google.apphosting.logs=/tmp
-jar ../jetty-distribution-9.3.5.v20151012/start.jar
--module=testMetadataServer

Note also, to see logging to the console, just remove
resources/jetty-logging.properties

—
Reply to this email directly or view it on GitHub
#150 (comment)
.

gregw · 2016-01-07T04:01:01Z

@ludoch, yes you can fully specify jetty home and jetty base. The example I'm using just uses the convenience config of jetty.home==parent-of-start.jar jetty.base=$CWD

You can definitely specify those. However, note that there are still a few default paths starting with "./" that should probably be made relative to jetty.home. I'll add that shortly

ludoch · 2016-01-07T04:35:18Z

Yes, the idea is to use this pattern:

java
-Djetty.home=/absolute/path/to/home
-Djetty.base=/absolute/path/to/base
-Droot.webapp=../webapps/testwebapp/
-Dcom.google.apphosting.logs=/tmp
-Dquickstart_web_xml=/absolute/myapp/WEB-INF/quickstart-web.xml
-D...
-jar /absolute/path/to/home/start.jar
....

On Wed, Jan 6, 2016 at 8:01 PM, Greg Wilkins notifications@github.com
wrote:

@ludoch https://github.com/ludoch, yes you can fully specify jetty home
and jetty base. The example I'm using just uses the convenience config of
jetty.home==parent-of-start.jar jetty.base=$CWD

You can definitely specify those. However, note that there are still a few
default paths starting with "./" that should probably be made relative to
jetty.home. I'll add that shortly

—
Reply to this email directly or view it on GitHub
#150 (comment)
.

Issue 140 rebased to async-support with fixes

ludoch · 2016-01-07T18:28:09Z

Tried the real Docker image deployed , and now getting:
0:17:39.000
at org.eclipse.jetty.xml.XmlConfiguration.main(XmlConfiguration.java:1435)
10:17:39.000
at java.security.AccessController.doPrivileged(Native Method)
10:17:39.000
at org.eclipse.jetty.xml.XmlConfiguration$1.run(XmlConfiguration.java:1510)
10:17:39.000
at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:68)
10:17:39.000
at org.eclipse.jetty.server.Server.doStart(Server.java:372)
10:17:39.000
at org.eclipse.jetty.server.handler.AbstractHandler.doStart(AbstractHandler.java:61)
10:17:39.000
at org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:106)
10:17:39.000
at org.eclipse.jetty.server.Server.start(Server.java:405)
10:17:39.000
at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:132)
10:17:39.000
at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:68)
10:17:39.000
at org.eclipse.jetty.server.handler.AbstractHandler.doStart(AbstractHandler.java:61)
10:17:39.000
at org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:114)
10:17:39.000
at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:132)
10:17:39.000
at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:68)
10:17:39.000
at org.eclipse.jetty.server.handler.AbstractHandler.doStart(AbstractHandler.java:61)
10:17:39.000
at org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:114)
10:17:39.000
at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:132)
10:17:39.000
at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:68)
10:17:39.000
at org.eclipse.jetty.server.NCSARequestLog.doStart(NCSARequestLog.java:228)
10:17:39.000
at org.eclipse.jetty.util.RolloverFileOutputStream.(RolloverFileOutputStream.java:166)
10:17:39.000
at org.eclipse.jetty.util.RolloverFileOutputStream.setFile(RolloverFileOutputStream.java:219)
10:17:39.000
Exception in thread "main" java.io.IOException: Cannot write log directory /var/logs/app_engine
10:17:39.000
INFO: Started ServerConnector@4dde6c82{HTTP/1.1,[http/1.1]}{0.0.0.0:8080}
10:17:39.000
Jan 07, 2016 6:17:39 PM org.eclipse.jetty.server.AbstractConnector doStart
10:17:39.000
INFO: jetty-9.3.5.v20151012
10:17:39.000
Jan 07, 2016 6:17:39 PM org.eclipse.jetty.server.Server doStart

gregw · 2016-01-07T21:45:20Z

@ludoch I can see a renaming issue... working on it now and will create a new PR shortly

ludoch · 2016-01-09T00:00:47Z

Almost there. It seems that for me, the quickstart xml generated now contains hardcoded path like:

  <context-param>
    <param-name>org.eclipse.jetty.tlds</param-name>
    <param-value><![CDATA[
    "jar:file:///Users/ludo/a/appengine-java-vm-guestbook-extras/stage3/target/appengine-staging/WEB-INF/lib/jstl-1.2.jar!/META-INF/scriptfree.tld",

instead of

  <context-param>
    <param-name>org.eclipse.jetty.tlds</param-name>
    <param-value><![CDATA[
    "jar:${WAR}//WEB-INF/lib/jstl-1.2.jar!/META-INF/scriptfree.tld",

gregw · 2016-01-12T21:23:42Z

On 9 January 2016 at 11:00, ludovic Champenois notifications@github.com
wrote:

Almost there. It seems that for me, the quickstart xml generated now
contains hardcoded path like:
org.eclipse.jetty.tlds instead of
org.eclipse.jetty.tlds @ludoch,

The issue is down to a difference in URI generation, where sometimes we get file:///Users/ludo/... and sometimes we just get file:/Users/ludo/...

I could probably hack around this in the vm context, but I think it is best to fix this in a new jetty release - both to make it more regular how it generates URI's, but also more accepting in the matching.

Also I note that quickstart is not substituting jetty.home and jetty.base in the quickstart-web.xml, which it needs to do to make this totally portable.

I'll get a 9.3.x release out over the next day or two with these fixes in.

gregw · 2016-01-12T22:51:50Z

@ludoch
to be precise, the problem is due to the JVM being inconsistent:

file     =./subdir/test.txt
file.getAbsoluteFile().toURI()         =file:/home/gregw/src/jetty-9.3.x/jetty-util/./subdir/test.txt
file.toPath().toAbsolutePath().toUri() =file:///home/gregw/src/jetty-9.3.x/jetty-util/./subdir/test.txt

So we definitely need a new jetty-9.3.x release, as we have changed a few more things to use Paths rather than files in 9.3.x and this will affect the portability of quickstart among other things.

ludoch · 2016-01-12T23:00:31Z

Bummer... No workaround without waiting for a new release that would cause
integration delays in private repo?

On Tue, Jan 12, 2016 at 2:51 PM, Greg Wilkins notifications@github.com
wrote:

@ludoch https://github.com/ludoch
to be precise, the problem is due to the JVM being inconsistent:

file =./subdir/test.txt
file.getAbsoluteFile().toURI() =file:/home/gregw/src/jetty-9.3.x/jetty-util/./subdir/test.txt
file.toPath().toAbsolutePath().toUri() =file:///home/gregw/src/jetty-9.3.x/jetty-util/./subdir/test.txt

So we definitely need a new jetty-9.3.x release, as we have changed a few
more things to use Paths rather than files in 9.3.x and this will affect
the portability of quickstart among other things.

—
Reply to this email directly or view it on GitHub
#150 (comment)
.

gregw · 2016-01-13T02:04:38Z

@ludoch unfortunately a work around is not really possible as the code is in several private methods on the quickstart generator and processor.

I have the fix ready to check in now and we should be able to create a release overnight or early tomorrow - so it will be in public m2 repo very soon.

Note that there are also some good performance improvements after 9.3.5, so worthwhile updating anyway.

gregw · 2016-01-13T20:51:56Z

@ludoch I have a commit ready to go that upgrades to the freshly in maven central: 9.3.7.RC1 and I've test that the quickstart is generated correctly.

Shall I commit that to the async-support branch? Create a PR? or wait a few days for a 9.3.7.v201601xx final release?

gregw added 2 commits January 6, 2016 15:22

Created jetty9-compat-base to create jetty base for use by jetty9-com…

c5b9893

…pat image. GoogleCloudPlatform#140

Modified base so that it can be run locally with a few system propert…

0024465

…ies. Added a README.md

ludoch reviewed Jan 6, 2016
View reviewed changes

gregw added 3 commits January 7, 2016 11:03

Updated archive naming, README and LICENSE for GoogleCloudPlatform#140

d7e0db9

removed empty logs directory

5aca730

Added the ability to set quickstart_web_xml system property to genera…

db38f10

…te quickstart GoogleCloudPlatform#140

remove . file usage

ebf4358

ludoch added a commit that referenced this pull request Jan 7, 2016

Merge pull request #150 from jetty-project/issue-140

3475ab0

Issue 140 rebased to async-support with fixes

ludoch merged commit 3475ab0 into GoogleCloudPlatform:fixes Jan 7, 2016

This was referenced Jan 8, 2016

Async support: create a distro in a tar gz file #140

Closed

java-compat from MVM's - what should I set to read logging.properties ?? #124

Open

gregw mentioned this pull request Jan 13, 2016

updated to jetty-9.3.7.RC1 #157

Merged

Conversation

gregw commented Jan 6, 2016

Uh oh!

ludoch Jan 6, 2016

Choose a reason for hiding this comment

Uh oh!

gregw commented Jan 7, 2016

Uh oh!

ludoch commented Jan 7, 2016

Uh oh!

gregw commented Jan 7, 2016

Uh oh!

ludoch commented Jan 7, 2016

Uh oh!

ludoch commented Jan 7, 2016

Uh oh!

gregw commented Jan 7, 2016

Uh oh!

ludoch commented Jan 9, 2016

Uh oh!

gregw commented Jan 12, 2016

Uh oh!

gregw commented Jan 12, 2016

Uh oh!

ludoch commented Jan 12, 2016

Uh oh!

gregw commented Jan 13, 2016

Uh oh!

gregw commented Jan 13, 2016

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants