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 #3241 Jetty runner add missing Main-Class in MANIFEST #3260

Merged
merged 3 commits into from
Jan 17, 2019

Conversation

olamy
Copy link
Member

@olamy olamy commented Jan 15, 2019

No description provided.

<javaHome>${java.home}</javaHome>
<environmentVariables>
<JAVA_HOME>${java.home}</JAVA_HOME>
</environmentVariables>
Copy link
Contributor

Choose a reason for hiding this comment

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

OMG, what is going on here??

Copy link
Member Author

Choose a reason for hiding this comment

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

just to be sure the forked maven used the current java version

<pomIncludes>
<pomInclude>*/pom.xml</pomInclude>
</pomIncludes>
<localRepositoryPath>${project.build.directory}/local-repo</localRepositoryPath>
Copy link
Contributor

Choose a reason for hiding this comment

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

What if my local repo path is different? or even on a different drive?

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, this is in ../target/local-repo isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's the repo path for the forked maven invocation :) so don't worry http://maven.apache.org/plugins/maven-invoker-plugin/run-mojo.html#localRepositoryPath

<pomInclude>*/pom.xml</pomInclude>
</pomIncludes>
<localRepositoryPath>${project.build.directory}/local-repo</localRepositoryPath>
<settingsFile>src/it/settings.xml</settingsFile>
Copy link
Contributor

@joakime joakime Jan 15, 2019

Choose a reason for hiding this comment

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

ew! i don't understand this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

we use a settings file which simply use your local repo as a mirror (so we do not download all the internet again :) )
same configuration as jetty-maven-plugin

jetty-jspc-maven-plugin/pom.xml Outdated Show resolved Hide resolved
jetty-maven-plugin/pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@olamy olamy force-pushed the jetty-9.4.x_jetty_runner_no_manifest branch from 1aee234 to d9f2c29 Compare January 17, 2019 01:50
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
@olamy olamy force-pushed the jetty-9.4.x_jetty_runner_no_manifest branch from d9f2c29 to 2295849 Compare January 17, 2019 01:51
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
@olamy
Copy link
Member Author

olamy commented Jan 17, 2019

@janbartel @joakime everything cleaned up. Now this pr only concerns the mentioned issue. Once this is merged. I will make a separate pr for cleaning up.

@olamy olamy merged commit 766e088 into jetty-9.4.x Jan 17, 2019
@olamy olamy deleted the jetty-9.4.x_jetty_runner_no_manifest branch January 17, 2019 05:24
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.

None yet

3 participants