Bug 152974 - Support "Copy-on-Save" for resources
Support "Copy-on-Save" for resources
Status: RESOLVED FIXED
Product: javaee
Classification: Unclassified
Component: Maven
6.x
All All
: P3 with 1 vote (vote)
: 6.x
Assigned To: Milos Kleint
issues@platform
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-11 17:43 UTC by akochnev
Modified: 2009-06-16 09:47 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
my shot at a patch for this issue (4.21 KB, patch)
2009-04-25 05:34 UTC, akochnev
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description akochnev 2008-11-11 17:43:27 UTC
When a web resource (jsp, html, property file) within the web project changes, it would be great if they could be copied
out to the deploy directory (instead of having to run another build-deploy-run cycle)
Comment 1 akochnev 2009-04-24 15:00:53 UTC
The problem here is a small bug w/ the CopyOnSave implementation. I copied the comments over from
http://www.netbeans.org/issues/show_bug.cgi?id=106522 . I have a local fix to the module on my machine and it works like
a beauty; however, when I tried to export a patch it tries to create a patch for the whole lot more than just this file. 


See the code below to address an issue w/ the current CopyOnSave. In its current state, the resources from 
src/main/resources (e.g. package foo.bar.baz) get copied into [project_home]/target/[web_app_name]/foo/bar/baz, 
instead of [project_home]/target/[web_app_name]/WEB-INF/classes/foo/bar/baz. 

The code below also copies the resources into the output directory (e.g. 
[project_home]/target/classes/foo/bar/baz). 

( I moved a few statements into a new copySrcToDest method) 
---------------
FileObject webAppClassesDir = webBuildBase.getFileObject("WEB-INF/classes");
FileObject destFile = ensureDestinationFileExists(webAppClassesDir, path, fo.isFolder());
copySrcToDest(fo, destFile);
                    
String projClassesPath = mavenproject.getMavenProject().getBuild().getOutputDirectory();
ileObject targetClasses = FileUtil.toFileObject(new File(projClassesPath));
destFile = ensureDestinationFileExists(targetClasses, path, fo.isFolder());
copySrcToDest(fo, destFile);
Comment 2 akochnev 2009-04-25 05:34:13 UTC
Created attachment 80902 [details]
my shot at a patch for this issue
Comment 3 Petr Jiricka 2009-04-27 14:04:48 UTC
Yes this should work for Maven - for Ant projects it also works.
Comment 4 Milos Kleint 2009-05-06 12:03:49 UTC
1. can you comment on the part that copies to target/classes? why is that necessary?
2. your patch might now work for resources, but is broken to items in src/main/webapp which need to be copied directly
to the web root, not the WEB-INF/classes.
Comment 5 Milos Kleint 2009-05-06 13:38:40 UTC
I've slightly modified the patch to address objection 2.
http://hg.netbeans.org/main/rev/43da8e336f08
I've commented out the code related to objection 1.
Also reworked some bad code related to renaming and deleting items..

thanks for the patch.
Comment 6 akochnev 2009-05-06 14:44:53 UTC
Thanks for implementing this fix so that it can make it into 6.7. I'll look for the message that QA integrated it into
the nightly builds to try it out. 

I think the reason for your two comments is that Jetty (jetty:run) uses src/main/webapp to load the web resources, and
target/classes to load classes and resources. There is a chance that it is not needed indeed, I'll take a look at why I
put it there in the first place. Even if Jetty uses target/classes by default, it can easily be configured to look in
WEB-INF/classes. 

When this goes into the nightly builds, it will be huge for Tapestry 5 developers that use NetBeans & Maven, I'll
probably blog about it as with this issue fixed, T5 development w/ Maven projects becomes a real pleasure ! Thanks a LOT
!!! 
Comment 7 Quality Engineering 2009-05-09 07:06:42 UTC
Integrated into 'main-golden', will be available in build *200905090201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/43da8e336f08
User: Milos Kleint <mkleint@netbeans.org>
Log: #152974 src/main/resources belong in WEB-INF/classes and src/main/webapp in the root of the deployed app
Comment 8 akochnev 2009-05-13 08:10:09 UTC
I'm sorry for reopening the issue, but it seems like in order to support consistent working of the copy-on-save
functionality for copying resources, it needs to copy the resources coming from src/main/resources into target/classes
(or mavenproject.getMavenProject().getBuild().getOutputDirectory()). 

Here's the problem w/ the current implementation: it is inconsistent with the way the Deploy-on-Save works . In the
current setup, the deploy-on-save functionality (invokes compiler:compile if I understand correctly and then does some
extra things) copies the compiled classes into target/classes (or whatever is specified as the <outputDirectory />
element in the pom.xml). Thus, in the current implementation we end up w/ the situation where on initial build, the
resources from src/main/resources are copied by maven into <outputDirectory />, and then when NB copy-on-save kicks in,
it tries to copy to target/[app-name]/WEB-INF/classes. 

The current workaround is to configure the pom to use target/[app-name]/WEB-INF/classes as the output directory, at
which point both the deploy-on-save and copy-on-save put the new classes and resources from src/main/resources in the
same spot. However, this is an unpleasant hack as running "mvn compile" fails unless you had run the "mvn package"
target (which actually creates the target/[app-name]  directory). 

In other words, CopyOnSave, line 146 should be:
FileObject destinationFolder = comesFromWebappRoot(fo) ? webBuildBase :
mavenproject.getMavenProject().getBuild().getOutputDirectory();

It might be a nice convenience to let CopyOnSave continue to copy into WEB-INF/classes in case other things depend on
the WEB-INF/classes directory being up to date as well. 

I apologize for not looking into this earlier (when you committed the change). If you are convinced that this piece of
functionality functions as expected, or if it looks unlikely that it would make it into 6.7, I would probably have to
create a separate module w/ an implementation dependency on Maven project in order to support this. 
Comment 9 Milos Kleint 2009-05-19 14:08:09 UTC
there might a bit of misunderstanding on how things work or should work and how they evolved to work the way they do.

1. copying of resources in webapps was happening even before DoS, for jsps to reload easily. This codebase is still
there and works for webapps only, copying the web resources and resources to the expanded application. The jar projects
will not get any resources copied on save (but only on Run action trigger, then all resources get copied to target/classes)
2. DoS itself didn't work until a few days - see http://www.netbeans.org/issues/show_bug.cgi?id=165045, The main point
is that the java infrastructure (not the compiler plugin) copies and compiles the  class files into target/classes, but
that folder is not picked up by the app server which care only about the expanded webapp. The fix for 165045 was to
ensure that once files are compiled and put in target/classes, they get copied to the expanded webapp. However that code
is placed in the app server deployment code and will only work if you deploy to one of the IDE's supported servers (thus
not for jetty:run)

I see both items as useful, (1. copying of resources to target/classes on actual save, and 2. copying of classes to
expanded webapp upon save without having IDE app server running), but they are not of the 6.7 solution unfortunately and
both would require a substantial amount of work on many places. We have it in plan for 6.8.
In there we should ensure that jetty:run and/or running with javarebel agent works. The additional challenge is to
create a clear UI that will clearly communicate what is happening. The current CompileOnSave/DeployOnSave UI is not very
clear and can be misguiding in the way it promises more flexibility than it actually provides.


Comment 10 akochnev 2009-05-24 06:59:00 UTC
Here's the thing : the 1 line change that I'm asking for will easily knock off the first piece : copying changed
resources from src/main/resources into target/classes. The thing is that the way things currently work is that they are
inconsistent : the initial build does copy the resources into target/classes, and COS doesn't support it (but only
copies to expanded web app). 

On the subject of copying classes when a "supported" app server is not running. This might be a bug in the current
implementation, but if you run the app in a supported server (e.g. Tomcat), then kill the server, the IDE continues to
perform the compile-on-save (e.g. new classes show up in both target/classes and in target/[app-name]/WEB-INF/classes). 

So, could you possibly add the 1 line to copy resources to target/classes as well ? I understand that a proper solution
might require substantial rework, but just this one line will make it a snap for Tapestry 5 users to get started
(without having to jump through hoops)
Comment 11 Milos Kleint 2009-06-16 09:47:28 UTC
please see issue 148499 that describes how the changes to Compile on Save merged in changeset
http://hg.netbeans.org/main/rev/313ba8e8b334
affect copying of resources and Compile on Save feature in general for 6.8

consider this issue fixed now. please verify once the changes appear in daily builds.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo