'play war' should preserve the lib and classes dirs in /war/WEB-INF

[Isaac Arias]

When running 'play war' on an app with a legacy Servlet-based tree under <app_dir>/war/ the command should preserve the contents of <app_dir>/war/WEB-INF/lib and <app_dir>/war/WEB-INF/classes to make it easier to integrate Play with an existing webapp.

Currently, after copying the files to the WAR destination the command replaces the contents of <war_dir>/WEB-INF/classes with the contents of <app_dir>/conf and deletes <war_dir>/WEB-INF/lib before creating an empty one to hold all the play Jars. A workaround is to manually move <app_dir>/war/WEB-INF/classes/* to <app_dir>/conf/ and <app_dir>/war/WEB-INF/lib/* to <app_dir>/lib before running the 'play war' command. If it merges the directory contents instead of replacing them we can eschew this step.

I'm including a simple patch to /framework/pym/play/utils.py that does the job for me. In this implementation Play files override the <app_dir>/war/WEB-INF/* files of the same name. After the export check <war_dir>/WEB-INF/lib for duplicate libraries. You may have to do some cleanup to make sure you don't end up with multiple versions of libraries from both the webapp and play.

[Erwan Loisant (eloisant)]

Hi, thanks for your patch. However we need to keep a distinction between directories with generated content and the one managed by the user. With your current patch WEB-INF/lib is managed by both the script and the user, and this is a problem.

For example, if you change your Play version you can end up with old jar in your lib (from a previous play war command).

We need to do it in a way that keeps a clear separation between what the developer manages and what the script generates.

[Isaac Arias (ikester)]

Hi Erwan,

I agree with the notion of keeping framework-managed files separated from user ones, if possible. However, the current implementation doesn't accomplish this. If a user places conflicting libs in /lib they will still end up mixed with the Play ones under WEB-INF/lib. I mention the conflicting jar caveat in the description.

I think there are two issues here, one has to do with mixing libs from the webapp and Play, the other one has to do with keeping stale libs on subsequent calls to 'play war', which is the one you were alluding to.

Since this is a convenience command I don't think we should worry too much about the former. The latter is not a problem because the whole directory is completely rebuilt on every run. If you upgrade your Play version you will only get the new libs the next time you run the command.

So the workflow for pure Play apps (greenfield projects) doesn't change at all if there is nothing in /war. Alternatively, if the user populates /war I would assume that they're trying to deploy existing webapp assets together with the Play app and this patch simplifies the process. Otherwise they need to manually move /war/WEB-INF/lib to /lib and /war/WEB-INF/classes to /conf as I mentioned, and I don't see what is gained by keeping the current behavior.

The reason I opened this is because I believe it simplifies the work of migrating legacy webapps to hybrid Play apps without impacting the typical use case. It's also much simpler to explain and document: "Just run 'play new <name>', drop your existing webapp inside /war, run 'play war...' and off you go!"

Of course it would be even better if the command would take care of updating the user's web.xml file by inserting the <context-param>, <servlet> and <servlet-mapping> nodes for Play. But we can't add a blanket mapping to /* because it would override all the other Servlet mappings, and trying to analyze and auto-merge the legacy mappings with the mappings in /conf/routes seems a bit daunting. So I think this is an acceptable compromise: do nothing and your webapp runs just like before, do a simple edit to your web.xml and you can let Play take care of part of the URL space.

[Isaac Arias (ikester)]

I have not tried this patch on a Windows installation. We should make sure that dir_util is available in standard Python distributions.

This change shouldn't break when /war doesn't exist because dir_util.copy_tree should behave exactly like shutil.copytree in that case. In other words, when the destination directory doesn't exist copy_tree will create it first, according to the documentation.

[Erwan Loisant (eloisant)]

Having the user's libs and Play libs mixed is not a problem, because that's in a directory managed by the script. What would be a problem is if the user starts to take care about WEB-INF himself and the script messes with it.

So a better solution would be to generate the war in a separate directory, and use any existing WEB-INF dir as a basis. That way the user's custom WEB-INF stays clean, and controlled by the user.

[Isaac Arias (ikester)]

I'm not sure I understand your point. Isn't that the way it works now? This is my understanding of the script:

1.- If /war dir exists it is copied to destination war_path otherwise create empty war_path and war_path/WEB-INF.
2.- A standard web.xml file is copied to war_path/WEB-INF/ if there isn't one already there.
3.- /app is copied to war_path/WEB-INF/application/.
4.- /conf is copied to war_path/WEB-INF/classes/.
5.- Play jar files are copied into war_path/WEB-INF/lib.
6.- Framework templates are copied to war_path/WEB-INF/framework/templates
7.- Modules are copied to war_path/WEB-INF/modules/
8.- Framework resources are copied to war_path/WEB-INF/resources/messages

So the WEB-INF directory is already being disturbed quite a bit. What's the point in giving the user the option to drop a webapp in /war and then completely ignore the contents of their lib and classes directories? In the simplest of examples, with a brand new empty Play app and a working webapp in /war, after running this script the web.xml will be preserved but the webapp won't work at all without those other dirs. How was this supposed to work in the first place?