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

Play 'dist' artifact includes duplicate files #3473

Closed
arobertn opened this issue Oct 2, 2014 · 4 comments · Fixed by #4116
Closed

Play 'dist' artifact includes duplicate files #3473

arobertn opened this issue Oct 2, 2014 · 4 comments · Fixed by #4116
Milestone

Comments

@arobertn
Copy link

arobertn commented Oct 2, 2014

Running the 'dist' sbt task creates a zip which is said to be the way to deploy to production. However, this zip contains duplicate files, for example both a fully stocked /conf directory at top level, and contained within the project's jar bundled under lib. In addition to unneccesary bloat, this can cause confusion when wanting to make a quick manual change to a server running in "production" mode (which might well be a test server or something else). The presence of the file at top level makes one assume that editing and restarting the app will make a change, when it may not depending on the class loading order (if it is even on the classpath to begin with). Issue #3472 was mistakenly reported due to this.

The raw files should be excluded since this is a production deploy, but if they are going to be there then they should be on the classpath first.

@jroper
Copy link
Member

jroper commented Oct 3, 2014

Agreed, this is something I've never looked too carefully at, but I was aware there was some weirdness there.

I don't think those files are added to the classpath at all. We could remove the mappings to include them, but I think some people depend on them being there and then they manually configure Play to point to them. The option is to include that conf directory on the classpath (and ensure it's first on the classpath), which is probably not a bad idea, but it does change existing behaviour.

You can put it on the classpath in your build by doing something along the lines of this in build.sbt:

import NativePackagerKeys._

scriptClasspath := "../conf" :+ scriptClasspath.value

One problem with the above, I'm not sure if it will work on Windows - the generated batch script will probably end up with the forward slash, not sure how the JDK on Windows copes with that.

@tloist
Copy link

tloist commented Nov 4, 2014

This conf directory in the dist packaging is something that bit me, too.

As an indication that this thing bites others as well, here are some issues I think are somewhat related:

That is what I found after a quick search. The point I'm trying to make here is:
What might be expected behaviour by the team doesn't seem to be expected by the users.

What is the purpose of the dist app/conf folder, if it is not loaded anyway?
Somewhere I read that you can refer to it by -Dfile.config and those files there are meant as templates. In this case I would vote for calling the directory template instead.

I would appreciate if we could somehow change the status quo and would propose to either remove the content or rename / move it somewhere where the purpose gets clearer.
What makes this worse is that the conf folder in Dev mode is actually the place to configure the application - so the developer are actually used to expect the contents of it to be configuration (not templates).

@chenbekor
Copy link

could not agree more with the troubles we had with this in prod. we find it very useful to change things like logger, connection strings etc even if in production. otherwise what's the point of including the conf dir? please make conf folder operational so that we could alter it's content and restart to see the effect. this refers to both regular conf files and logback configuration files!

@benmccann
Copy link
Contributor

The problem is that conf is being used by people for two different things. The first is that it's a resources directory that goes on your classpath like src/main/resources would in a typical project. The second is that some people want a place to put external configuration files that are included in a directory packaged with the app when you run dist. Right now the conf directory gets both included into your jar and then included again a second time in the dist zip. Having it in two places as we do now is a recipe for confusion. I know folks on my team have been caught off guard by this in the past and I needed to explain to them how it worked because it was causing trouble for them.

I personally think the best solution here would be to stop using this directory for two different things. I would create a new directory called resources that has the resources you want to be on your classpath and use conf just for the external configuration. This would have to be mentioned in the migration guide to make sure folks move files from conf to resources as needed.

If we split the current conf directory into two like this, then there's the question of where do you want application.conf to be located? You can put it in either place and it will work just fine. You just need to set the command line flag accordingly to load it either as a resource or a file. I believe more people would probably want it located externally.

jroper added a commit to jroper/playframework that referenced this issue Mar 23, 2015
* conf directory in dist artifact is now added to the classpath by
  default
* Added externalizeResources SBT setting to turn off including the
  conf directory as an external (as in, not in the applications jar)
  directory and including it in the classpath
* Added --no-exit-sbt flag to Play start and stop commands to help with
  scripted tests
* Updated distribution scripted test, which now tests running Play in
  prod mode, as well as testing if the conf directory is on the
  classpath.

Fixes playframework#3473
jroper added a commit to jroper/playframework that referenced this issue Mar 23, 2015
* conf directory in dist artifact is now added to the classpath by
  default
* Added externalizeResources SBT setting to turn off including the
  conf directory as an external (as in, not in the applications jar)
  directory and including it in the classpath
* Added --no-exit-sbt flag to Play start and stop commands to help with
  scripted tests
* Updated distribution scripted test, which now tests running Play in
  prod mode, as well as testing if the conf directory is on the
  classpath.

Fixes playframework#3473
@jroper jroper added the has-pr label Mar 23, 2015
jroper added a commit to jroper/playframework that referenced this issue Mar 23, 2015
* conf directory in dist artifact is now added to the classpath by
  default
* Added externalizeResources SBT setting to turn off including the
  conf directory as an external (as in, not in the applications jar)
  directory and including it in the classpath
* Added --no-exit-sbt flag to Play start and stop commands to help with
  scripted tests
* Updated distribution scripted test, which now tests running Play in
  prod mode, as well as testing if the conf directory is on the
  classpath.

Fixes playframework#3473
jroper added a commit to jroper/playframework that referenced this issue Mar 24, 2015
* conf directory in dist artifact is now added to the classpath by
  default
* Added externalizeResources SBT setting to turn off including the
  conf directory as an external (as in, not in the applications jar)
  directory and including it in the classpath
* Added --no-exit-sbt flag to Play start and stop commands to help with
  scripted tests
* Updated distribution scripted test, which now tests running Play in
  prod mode, as well as testing if the conf directory is on the
  classpath.

Fixes playframework#3473
ClaraAllende pushed a commit to ClaraAllende/playframework that referenced this issue Aug 28, 2015
* conf directory in dist artifact is now added to the classpath by
  default
* Added externalizeResources SBT setting to turn off including the
  conf directory as an external (as in, not in the applications jar)
  directory and including it in the classpath
* Added --no-exit-sbt flag to Play start and stop commands to help with
  scripted tests
* Updated distribution scripted test, which now tests running Play in
  prod mode, as well as testing if the conf directory is on the
  classpath.

Fixes playframework#3473
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 a pull request may close this issue.

5 participants