there is a lots of problems associated with compile on save feature. among other things the fact that it's running ant instead of maven. So far I didn't see any reports on the problem with classfile. AFAIK when the CoS was off for main sources by default (6.8?) we've shown a warning describing the exact problem to the users when they turned it on.

The copying of class files is not only relevant for JavaRunner (which I consider rather obscure and really useful for simple projects only.) but also for tools like jrebel and javeleon which after proper setup DO work even for the biggest applications (where the speed up is most noticeable)

Option 2 sound good to me.

http://hg.netbeans.org/core-main/rev/8ba87520e739

disabled CoS by default.

on top of that, added a few more places where IDE's classfiles are removed. On project close and on each change of the project that disables CoS.

Updated new and noteworthy as well.

Integrated into 'main-golden', will be available in build *201209040001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/8ba87520e739
User: Milos Kleint <mkleint@netbeans.org>
Log: #217595 disable CoS by default. On top of that, be more consistent in cleaning IDE's own class files. Additionally clean on closing the project and immediately when the project setting changes (was delayed until first build)

Seems to work.

(In reply to comment #1)
> The copying of class files is [...] relevant for [...]
> tools like jrebel and javeleon which after proper setup DO work

But it is hardly necessary for these either; you can just run 'mvn compile' which will be safer and more reliable, with only a modest performance penalty in typical cases. (If mvnsh gets resurrected, it would eliminate most of that overhead.) And if you do trust CoS classes enough to use with these tools for maximum speed, it would suffice for an IDE integration plugin to add the internal class file location to the effective reload classpath.

(In reply to comment #4)
> Seems to work.
> 
> (In reply to comment #1)
> > The copying of class files is [...] relevant for [...]
> > tools like jrebel and javeleon which after proper setup DO work
> 
> But it is hardly necessary for these either; you can just run 'mvn compile'
> which will be safer and more reliable, with only a modest performance penalty
> in typical cases. 

Well, the assumed feature of CoS is that you don't need to remember to trigger a build at all. YOu just save the file and the changes are live. That's the ultimate goal which CoS more or less fulfils. Yes it can fail, but the same applies to the class reloading java agents as well.



> And if you do trust CoS classes enough to use with these tools for
> maximum speed, it would suffice for an IDE integration plugin to add the
> internal class file location to the effective reload classpath.

Yes, if they indeed exist. AFAIK the jrebel is only limited to debugging hacks.


in general I'm not happy with CoS either, especially the part where we start ant to run stuff. Ideally it would be just maven to have 100% compatibility with regular builds. Copying of resources is also error prone and ideally would be done by maven as well to cover all the filtering and other features.
And while I'm at it, I'd like to rework how we edit actions, ideally we would have one Run action and only have either automated or manual diffs for Debug, Profile, CoS

(In reply to comment #5)
> especially the part where we start ant to run stuff

Bug #166379.

> Ideally it would be just maven to have 100% compatibility
> with regular builds. Copying of resources is also error prone and ideally would
> be done by maven as well to cover all the filtering and other features.

Well in that case you could ignore IDE-generated classes entirely and just run 'mvn compile' automatically when anything in src/** is saved.

> have one Run action and only have either automated or manual diffs for Debug,
> Profile

StartupExtender should make this possible. The profiler is already using it for some things; perhaps the traditional way the debugger is invoked (as first-class actions, with JVM args hardcoded in the project type) should be reworked to be based on SE. Rather off topic here though.

(In reply to comment #6)
> (In reply to comment #5)
> > especially the part where we start ant to run stuff
> 
> Bug #166379.

Well, I'm marginally sceptical that we are really able to transfer any possible surefire configuration into our api calls (be it ant or extexecution).

> 
> > Ideally it would be just maven to have 100% compatibility
> > with regular builds. Copying of resources is also error prone and ideally would
> > be done by maven as well to cover all the filtering and other features.
> 
> Well in that case you could ignore IDE-generated classes entirely and just run
> 'mvn compile' automatically when anything in src/** is saved.


Well, anything that runs a lifecycle stage is potentially slow as we cannot anticipate what's going to be executed.
I was more thinking about using surefire:test for tests and exec:exec for main class execution. There the maven overhead is more or less constant and typically small (not tens of seconds)

> 
> > have one Run action and only have either automated or manual diffs for Debug,
> > Profile
> 
> StartupExtender should make this possible. The profiler is already using it for
> some things; perhaps the traditional way the debugger is invoked (as
> first-class actions, with JVM args hardcoded in the project type) should be
> reworked to be based on SE. Rather off topic here though.

Not entirely sure here, still will need to investigate what StartupExtender is, but since we need something for maven based exec anyway for a lot of execution types (people can turn CoS off), having just one way of doing things would help.

I must say I don't agree with this change in case of Java EE projects. 

Compile on Save/Deploy on Save was turned on by default for EE projects in 7.1, and the feedback on this change was very positive. I am not aware of any complaints about this, and even if there are some problems, I think they the advantages far outweigh them.

From the discussion above and based on hallway conversation with Milos I understand that:

- In some corner cases incorrect code may get into your build output. When you turn on CoS, there is a warning that warns about this. As I wrote, I did not hear complains about this from EE users. If you think it would be beneficial, we can think about showing this warning at some other point, when CoS is on.

- Ant is used when CoS is on, which may lead to incorrect behavior. But this is only when running projects with a main class, no? So users of EE projects will not run into this. 

- With respect to JRebel, you could in theory use JRebel with CoS off (you just hit build manually and JRebel does the rest), but this workflow is not what the majority of EE users expect. The expected usage scenario is that you save the file and the change is autodeployed by JRebel immediately, for which you need to turn CoS on. 

So, can we turn CoS back on by default for EE projects?

I would suggest enabling CoS but only in separate IDE-specific directories under target/. For example, in the case of SE-oriented projects, classes-cos/ (resp. test-classes-cos/) could be maintained with CoS *.class, plus copies of non-*.class resources synchronized from classes/ (resp. test-classes/). If you use JavaRunner.QUICK_RUN/DEBUG (resp. QUICK_TEST/_DEBUG), use that alternate classpath entry from this project and all dependency projects.

For JavaRebel/Javeleon, classes-cos/ can be used for immediate reloading.

For EE projects using a decent app server, instruct the server’s unpacked mode to include classes-cos/ instead of classes/ or war/WEB-INF/classes/ or whatever it is.

For EE projects using app servers which are too dumb to understand anything more than a single path to an unpacked WAR, create a shadow war-cos/ which is a complete copy of war/ except for the CoS classes (again listen to non-*.class file changes in war/, or a known part of src/, and automatically synchronize those).

This would give the benefits of CoS without there being any chance of contamination of the project’s official artifacts.

I still think CoS should be off by default at least for non-EE projects; there are just way too many setups which break it, and you should not need to make IDE-specific changes to a project to run or test it when it works from the command line. For tests we already (7.2) make this opt-in with a hint in the status bar, so you can try it if you are bothered by Maven execution time and leave it on if it works. No strong opinion about what the default should be for EE projects.

There should probably be a global setting in Maven options about whether to use CoS unless otherwise specified or not. (The profile in comment #0 serves as a workaround.)

I agree with Petr that benefits of CoS ON outweigh its problems. I would definitely keep it ON. And fix problems like Ant execution.

> This would give the benefits of CoS without there being any chance of
> contamination of the project’s official artifacts.

I would think that if contamination is really a problem then we would have handful of issues reported on it. Just that it can happen does not mean we have to fix it. I mean what you propose Jesse is right solution to fix this but is all the work worth it?

I agree, let's just turn it on for EE projects ASAP (this MUST be done for 7.3 beta), and we can file a separate enhancement request for the classes-cos solution and address it separately.

We usually immediately get a bug when we regress in CoS, but we have never received bug like this. So judged by this, CoS should be enabled by default.

(In reply to comment #12)
> We usually immediately get a bug when we regress in CoS, but we have never
> received bug like this. So judged by this, CoS should be enabled by default.

Indeed we've so far never got a report about this. Most likely because it happens just infrequently that our cos files end up in binaries in maven repositories. Even if it does, we might get away with it because the class compiles fine.


I do get some semi-frequently about people using filtered resources that don't end up filtered upon change. That's because we have no direct way of processing the resource file changed the same way as maven does. Oftentimes just explaining what's going on helps. But there is really no solution in sight. We would have to depend on maven resources plugin when copying resources and last time I checked it only processes the whole source root at once with no way of forcing just one file for filtered copy.

I assume this applies to web projects as much as it does for simple jar projects, maybe even more given the number of resources/web content is usually higher in web projects..

(In reply to comment #9)
> I would suggest enabling CoS but only in separate IDE-specific directories
> under target/. For example, in the case of SE-oriented projects, classes-cos/
> (resp. test-classes-cos/) could be maintained with CoS *.class, plus copies of
> non-*.class resources synchronized from classes/ (resp. test-classes/). If you
> use JavaRunner.QUICK_RUN/DEBUG (resp. QUICK_TEST/_DEBUG), use that alternate
> classpath entry from this project and all dependency projects.
> 
> For JavaRebel/Javeleon, classes-cos/ can be used for immediate reloading.

it's going to cause a lot of subtle problems, starting from the jrebel maven plugin (+users) not knowing about classes-cos at all to problems with syncing resources between target/classes and classes-cos

<snip>
> 
> There should probably be a global setting in Maven options about whether to use
> CoS unless otherwise specified or not. (The profile in comment #0 serves as a
> workaround.)

once we have different defaults for different packagings the global option looses any clear meaning.


In general I believe the the class file contamination problem is fairly low in importance for multiple reasons. Including broad usage of maven release plugin and/or CI machines to generate consumable binaries. Almost never one uploads a non-snapshot binary built from dev sources. Please note that the same problem is known to occur on Eclipse AFAIK.

The bigger problem is the JavaRunner and how we execute projects or tests when CoS is on. IMHO.

http://hg.netbeans.org/core-main/rev/c2fae7e8873e enables CoS for war, ejb and ear projects by default

the reason why this issue was reopened is now fixed. -> resolved/fixed

(In reply to comment #13)
> Indeed we've so far never got a report about this. Most likely because it
> happens just infrequently that our cos files end up in binaries in maven
> repositories.

That is probably true, as comment #14 discusses. My case was in a local repo snapshot, not in a public repo. It was still disturbing to have classes broken by an IDE editor bug present in a command-line build.

> Even if it does, we might get away with it because the class
> compiles fine.

Or because the message "RuntimeException: Uncompilable source code" does not offer any hint that a NB bug is involved; you would have to be an advanced NB user who understands CoS in some detail.

Integrated into 'main-golden', will be available in build *201209190001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/c2fae7e8873e
User: Milos Kleint <mkleint@netbeans.org>
Log: #217595 CoS on for application execution when packaging is war, ejb, ear