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

JENKINS-24827 Port hpi:run to jetty 8.x #12

Merged
merged 6 commits into from Jul 28, 2015

Conversation

Vlatombe
Copy link
Member

This pull request replaces #11

The notation

<configuration>
  <contextPath></contextPath>
</configuration>

has been deprecated by the jetty-maven-plugin and is now exposed as

<configuration>
  <webApp>
    <contextPath></contextPath>
  </webApp>
</configuration>

So the plugins parent should be updated to use this new structure.

recampbell and others added 2 commits May 11, 2015 16:28
…-- doesn't boot correctly, perhaps due to classpath issues. See JENKINS-24827 for more information.
- Update jetty to latest 8.x
- Expose jetty packages to webapp as it is required for jndi initial
factory
- Rename org.mortbay.jetty to org.eclipse.jetty
@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@ghost
Copy link

ghost commented Jul 2, 2015

Argh!!! ok we need to figure out how to get the bot to actually correctly identify cloudbees employees some how....

@Vlatombe
Copy link
Member Author

Vlatombe commented Jul 2, 2015

Maybe you could look up if the pull request creator is part of the Cloudbees GH organization?

@stephenc
Copy link
Member

stephenc commented Jul 2, 2015

@Vlatombe ha! that is what we were doing, but that was giving us a lot of false negatives. I suspect I am going to have to resort to a hard-coded list of bees :-(

@jtnord
Copy link
Member

jtnord commented Jul 6, 2015

Given we are just soon to require servlet 3.x I wonder if this should go futher than jetty 8 which is only 3.0?

@Vlatombe
Copy link
Member Author

Vlatombe commented Jul 6, 2015

I could give a shot to Jetty 9.x if this gets some traction.

sp.setName("HUDSON_HOME");
sp.setValue(jenkinsHome.getAbsolutePath());
sp.setIfNotSetAlready();
setSystemPropertyIfEmpty("HUDSON_HOME",jenkinsHome.getAbsolutePath());
Copy link
Member

Choose a reason for hiding this comment

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

should this also set JENKINS_HOME?

sp.setIfNotSetAlready();
// set JENKINS_HOME
setSystemPropertyIfEmpty("HUDSON_HOME",jenkinsHome.getAbsolutePath());
setSystemPropertyIfEmpty("JENKINS_HOME",jenkinsHome.getAbsolutePath());
Copy link
Member

Choose a reason for hiding this comment

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

this does break backwards compatibility with anyone who sets HUDSON_HOME and uses hpi:run - as JENKINS_HOME will take precedence over HUDSON_HOME (IIRC)
I'm inclined to say if you set one you set both - or only set JENKINS_HOME.

But we should be encouraging devs to use JENKINS_HOME as HUDSON is long dead :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll just remove HUDSON_HOME then.

@jtnord
Copy link
Member

jtnord commented Jul 23, 2015

🐝

@jglick
Copy link
Member

jglick commented Jul 23, 2015

🐝 without understanding most of the details. I have to assume you have tested this interactively with a variety of plugins, as there are a lot of special cases buried in this mojo that can regress easily, and there is zero test coverage.

Vlatombe added a commit that referenced this pull request Jul 28, 2015
@Vlatombe Vlatombe merged commit b2d139e into jenkinsci:master Jul 28, 2015
@Vlatombe Vlatombe deleted the JENKINS-24827 branch July 28, 2015 09:43
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

6 participants