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

Wip/windows service #329

Closed
wants to merge 3 commits into from
Closed

Wip/windows service #329

wants to merge 3 commits into from

Conversation

muuki88
Copy link
Contributor

@muuki88 muuki88 commented Aug 21, 2014

@laguiz , I rebased your changes and fixed some compilation errors. Feel free to
fork from this branch (I'll close this PR if you can continue your work).

@lightbend-cla-validator

Hi @laguiz,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://typesafe.com/contribute/cla

@muuki88 muuki88 added this to the 0.8.0 - windows services milestone Aug 21, 2014
laguiz and others added 3 commits September 12, 2014 10:36
It's far from being ready for a pull request
I have some questions that you may discover in the code itself
@muuki88 muuki88 force-pushed the wip/windows-service branch from a60a0ff to 408cbb2 Compare September 12, 2014 09:05
muuki88 referenced this pull request in laguiz/sbt-native-packager Sep 12, 2014
It's far from being ready for a pull request
I have some questions that you may discover in the code itself
@laguiz
Copy link
Contributor

laguiz commented Sep 12, 2014

Fyi I signed the Typesafe Contributors License Agreement.

@laguiz
Copy link
Contributor

laguiz commented Oct 2, 2014

Quick info : I will try to push my current new version to this branch before october 12.

@muuki88
Copy link
Contributor Author

muuki88 commented Oct 3, 2014

Awesome, looking forward to it. If you need help on rebasing this one, feel free to ask :)

@laguiz
Copy link
Contributor

laguiz commented Oct 12, 2014

Well it's 12 october but unfortunately I did not get the time to finish. I pulled the branch and reintegrate my changes and it compile but I did not tested. Next date 19 october. I keep you informed. Sorry for the delay :)

@muuki88
Copy link
Contributor Author

muuki88 commented Oct 13, 2014

Hi :) Thanks keeping us up-to-date. No pressure, we are currently working very hard on the autoplugins refactoring. I'm branching off a 0.8.x. It's fine, when your code compiles/runs against this branch. We will do the autoplugins integration later on

laguiz referenced this pull request in laguiz/sbt-native-packager Oct 13, 2014
…not manage Wix + Winsw but only add Winsw files into ZIP file in `target/windows` folder if user call `windows:packageZip` or if user change the default strategy. Winws XML file looks OK but I did not test properly yet. Know issue : Users HAVE to create Windows environment variable named `MY_PROJECT_OPTS` even if this variable is empty otherwise Winsw won't work because `%MY_PROJECT_OPTS%` will be used as is if no env variable is found (see Winsw doc).
@laguiz
Copy link
Contributor

laguiz commented Oct 13, 2014

Current version : laguiz@7bb1909

@laguiz
Copy link
Contributor

laguiz commented Dec 15, 2014

This start to become old :)

However I want to share experience we had with colleagues. Since we are using Java 7, we can use "wildcard" classpath (in fact since Java 6 we can do this!). See http://stackoverflow.com/questions/219585/setting-multiple-jars-in-java-classpath for more details.

This mean for us that the final Winsw XML become pretty simple. In fact the configuration does not depends on listing each libraries one by one. This mean if we ship a new version of the application that include for example a new library then Winsw XML file does not change.

Here is an example of an Winsw XML file using wildcard for the classpath:

<service>
        <id>myapp</id>
        <name>My App</name>
        <description>My App description</description>
        <executable>java</executable>
        <arguments>%MY_APPLICATION_OPTS% -cp "c:\full\path\to\myapp\lib\*" -Dpidfile.path="NUL" play.core.server.NettyServer</arguments>
        <logmode>rotate</logmode>
        <onfailure action="restart"/>
</service>

Important part here is -cp "full\path\to\myapp\lib\*"

In the scenario where we ship a new version of the application, this XML file will still works. Administrator just have to rename folders and stop/start the windows service without changing Winsw XML file.

So my conclusion on this PR would be do not pull it :) but simply add a tutorial on sbt-native-packager website that would describe this simple scenario.

Let me know what you think.

@muuki88
Copy link
Contributor Author

muuki88 commented Dec 16, 2014

@laguiz I think it's awesome you put this much effort into this :)

Important part here is -cp "full\path\to\myapp\lib*"

Yeah, that will work. However this is not a good idea, because of two main points

  1. Security. Anybody can put things on the classpath without noticing
  2. Classpath ordering.

And I'm not sure about the getWinswExe, why isn't this needed anymore? (sorry if I ask obvious things)

@laguiz
Copy link
Contributor

laguiz commented Dec 17, 2014

Hi,

Regarding my effort, I said I will follow up so I do :)

Regarding your points :

  1. In my opinion, server should be protected anyway to avoid access to it. For me even if we list jars, because they are not signed, we can easily patch any jar (for example the well knowhibernate-xxx.jar) that will do the malicious things. Not really convinced let say :)
  2. Here in fact I did not think about this potential issue and even if we (on our current Play 2.3.x application) don't have issue currently it may happen.

From Javadoc I can read this
http://docs.oracle.com/javase/6/docs/technotes/tools/windows/classpath.html

The order in which the JAR files in a directory are enumerated in the expanded class path is not specified and may vary from platform to platform and even from moment to moment on the same machine. A well-constructed application should not depend upon any particular order. If a specific order is required then the JAR files can be enumerated explicitly in the class path.

I'm not trolling here ;) Are you aware of any classpath ordering issue in Scala/Play/Akka ... ? Would be interesting to know. If I print the classpath it looks like there is a sorting logic in appClasspath: Seq[String] (looks like we first load application code then scala then .... then application assets).

If one of the point 1 or 2 is not negotiable then we continue on this PR.

Regarding getWinswExe we need it to fetch winsw.exe file from Internet. Winsw is the Windows Service Wrapper from https://github.com/kohsuke/winsw (used to install, uninstall the Windows Service based on the XML file we generate). Idea is to have an out-of-the-box experience. In the future (I don't think it's very important for now) we could provide more options to let user describe where we should go to fetch the exe (ex: from the project, from the disk, from alternative url, alternative version). Let me know if you saw something I'm missing here.

Let me know what you think.

@laguiz
Copy link
Contributor

laguiz commented Dec 17, 2014

Just saw this comment :

#72 (comment)

It looks like wildcard approach is possible but should not be the default or at least should not be the only option.

@muuki88
Copy link
Contributor Author

muuki88 commented Dec 17, 2014

I like that one

A well-constructed application should not depend upon any particular order.

This should really be the case in a well constructed application. However I think this is not interesting for this conversation as your pull request already solves this issue perfectly :)

Idea is to have an out-of-the-box experience

This is the main point and the reason why I want to continue this pull request. It should be as easy as possible to install a windows server and I think the way we are going is an awesome first start.

  • Creating the winsw.xml configuration
  • Providing a service installer winsw.exe (at the moment predefined, we can make this flexible as user require this)
  • enable the java_server archetype and you get what you want!

If you need help on merge from master, ping me :)

@muuki88 muuki88 mentioned this pull request May 17, 2016
@muuki88 muuki88 closed this Nov 2, 2016
@muuki88 muuki88 deleted the wip/windows-service branch November 2, 2016 18:53
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.

3 participants