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 #5174 - Reorganized jetty-distribution #5186

Merged
merged 1 commit into from
Sep 3, 2020

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Aug 21, 2020

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

@joakime joakime added the Build label Aug 21, 2020
@joakime joakime requested a review from olamy August 21, 2020 21:31
@joakime joakime self-assigned this Aug 21, 2020
@joakime
Copy link
Contributor Author

joakime commented Aug 21, 2020

I still need to update the README in the root of the jetty-distribution, just wanted to get something up here for others, so that we can start the conversation about this approach.

@janbartel
Copy link
Contributor

janbartel commented Aug 22, 2020

FYI there's a couple of problems in the reorganized distro:

  • the test-jndi webapp doesn't deploy. It can't find any of the resources that should be bound, and doesn't find javax.mail.Session class.
  • there's demo-base/demo-base nested directory

@janbartel
Copy link
Contributor

@joakime I pushed a fix to the correct locations for test-spec and test-jndi configs.

Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

as discussed:


[152] git grep '\.\./start.jar' | cut -d: -f1 | uniq -c
      1 jetty-distribution/src/main/resources/README.TXT
      1 jetty-distribution/src/main/resources/start.ini
      1 jetty-documentation/src/main/asciidoc/distribution-guide/connectors/configuring-connectors.adoc
      1 jetty-documentation/src/main/asciidoc/distribution-guide/connectors/configuring-ssl-distribution.adoc
      1 jetty-documentation/src/main/asciidoc/distribution-guide/contexts/temporary-directories.adoc
      1 jetty-documentation/src/main/asciidoc/distribution-guide/logging/configuring-jetty-request-logs.adoc
     10 jetty-documentation/src/main/asciidoc/distribution-guide/logging/configuring-logging-modules.adoc
      2 jetty-documentation/src/main/asciidoc/distribution-guide/logging/default-logging-with-stderrlog.adoc
      2 jetty-documentation/src/main/asciidoc/distribution-guide/sessions/session-configuration-file-system.adoc
      3 jetty-documentation/src/main/asciidoc/distribution-guide/sessions/session-configuration-gcloud.adoc
      4 jetty-documentation/src/main/asciidoc/distribution-guide/sessions/session-configuration-hazelcast.adoc
      5 jetty-documentation/src/main/asciidoc/distribution-guide/sessions/session-configuration-infinispan.adoc
      2 jetty-documentation/src/main/asciidoc/distribution-guide/sessions/session-configuration-jdbc.adoc
      2 jetty-documentation/src/main/asciidoc/distribution-guide/sessions/session-configuration-mongodb.adoc
      3 jetty-documentation/src/main/asciidoc/distribution-guide/startup/custom-modules.adoc
      1 jetty-documentation/src/main/asciidoc/distribution-guide/startup/screen-list-modules.adoc
      1 jetty-documentation/src/main/asciidoc/distribution-guide/startup/start-jar.adoc
      5 jetty-documentation/src/main/asciidoc/distribution-guide/startup/startup-modules.adoc
      1 jetty-documentation/src/main/asciidoc/quickstart-guide/getting-started/jetty-running.adoc

@joakime joakime marked this pull request as ready for review August 27, 2020 18:33
@joakime joakime requested a review from gregw August 27, 2020 18:36
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

I like this a lot, but a few more changes... mostly to the README.
I also think we need a README in the demo-base directory that describes exactly what it demos and how it was setup.

distributing Jetty.
Eclipse Jetty is open source and is dual licensed using the
Apache 2.0 and Eclipse Public License 2.0.
You may choose either license when distributing Jetty.
Copy link
Contributor

Choose a reason for hiding this comment

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

At this point we need a section called "JETTY DISTRIBUTION" that describes what this directory is... how it contains a jetty-home (available separately), a demo-base and I think it should also contain a minimal jetty-base with just server module enabled.

We should make it clear that the jetty-home directory should be read-only and modifications should only be done to the base directories. Can we actually make jetty-home read-only in a portable way?

We could even say that to upgrade then only the jetty-home directory should be replaced with the jetty-home artefact.

distributing Jetty.
Eclipse Jetty is open source and is dual licensed using the
Apache 2.0 and Eclipse Public License 2.0.
You may choose either license when distributing Jetty.


RUNNING JETTY
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the readme should then go as follows:

RUNNING JETTY
Assuming unix commands the general form of running jetty is:
   $ JETTY_HOME=/path/to/jetty-home
   $ cd /path/to/jetty-base
   $ java -jar $JETTY_HOME/start.jar

To see all the options to the start command:
  $java -jar $JETTY_HOME/start.jar --help

RUNNING THE DEMO_BASE
To see the configuration of the included demo-base
 $ cd demo-base
 $ java -jar $JETTY_HOME/start.jar --list-config

To run the demo (from the demo-base directory):
  $ java -jar $JETTY_HOME/start.jar

RUNNING A WAR
This distribution contains a jetty-base directory with a minimal configuration.
To enable http and webapp deployment for this base
  $ JETTY_HOME=$PWD/jetty-home
  $ cd jetty-base
  $ java -jar $JETTY_HOME/start.jar --add-to-start=http,deploy
  $ cp /path/to/mywebapp.war webapps

To see what other modules can be configured:
  $ java -jar $JETTY_HOME/start.jar --list-modules

This war in this base can then be run with
 $ java -jar $JETTY_HOME/start.jar

CREATING A NEW JETTY BASE
A new Jetty base can be created anywhere on the file system and with any name:
 $ mkdir /path/to/my-jetty-base
 $ cd /path/to/my-jetty-base
 $ java -jar $JETTY_HOME/start.jar --create-startd --add-to-start=server,http,deploy
 $ cp /path/to/mywebapp.war webapps

This base can then be run with

 $ java -jar $JETTY_HOME/start.jar

I don't think we really need the rest of the readme other than links to the documentation!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
I'm good with what you wrote (just some minor formatting niggles)

Copy link
Member

@olamy olamy left a comment

Choose a reason for hiding this comment

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

👍

@joakime joakime requested a review from gregw September 2, 2020 21:09
 + Updating license / notice files for EPLv2

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime force-pushed the jetty-10.0.x-5174-reorg-jetty-dist branch from 1a4e90c to a54ed92 Compare September 3, 2020 12:14
@joakime joakime merged commit 392215e into jetty-10.0.x Sep 3, 2020
@joakime joakime deleted the jetty-10.0.x-5174-reorg-jetty-dist branch September 3, 2020 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reorganize jetty-distribution in version 10.0.0 to encourage jetty-home vs jetty-base split
4 participants