This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 207717 - ProjectClassPathModifier needs classPathType for compile-only (Maven scope=provided)
Summary: ProjectClassPathModifier needs classPathType for compile-only (Maven scope=pr...
Status: RESOLVED FIXED
Alias: None
Product: javaee
Classification: Unclassified
Component: Web Project (show other bugs)
Version: 7.1
Hardware: All All
: P3 normal (vote)
Assignee: David Konecny
URL:
Keywords: API
: 180183 189331 206529 (view as bug list)
Depends on: 181474 212694
Blocks: 186221 206529
  Show dependency tree
 
Reported: 2012-01-25 12:53 UTC by Petr Jiricka
Modified: 2013-01-16 15:55 UTC (History)
7 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
patch (30.80 KB, patch)
2012-07-17 02:00 UTC, David Konecny
Details | Diff
patch (2.88 KB, patch)
2012-07-17 03:24 UTC, David Konecny
Details | Diff
patch (8.19 KB, patch)
2012-07-17 03:30 UTC, David Konecny
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Jiricka 2012-01-25 12:53:55 UTC
Accorging to the javadoc, ClassPath.COMPILE is defined as "ClassPath for compiling things." 

In reality, when you add a file on this classpath, both Ant-based projects and Maven project will add the file to compilation classpath AS WELL AS runtime classpath. This is not documented in the Javadoc. Imagine there was another project type that would in this case only add the file on the compilation classpath BUT NOT the runtime classpath, then this would be a perfectly compliant implementation, but would behave differently. This would make it very difficult to write clients of the API that behave consistently across all project implementations.

Additionally, often there is real need to add things to the compilation classpath BUT NOT the runtime classpath (both Maven and Ant-based EE projects support this case) - this case is currently not achievable using the ClassPath API. I can provide examples of where this is used in the IDE if requested.

I suggest to fix this situation as follows (to assure semantic compatibility for existing clients):
* Strengthen the ClassPath.COMPILE definition to specify that adding to it also adds to the runtime classpath.
* Add a new kind of classpath ClassPath.COMPILE_ONLY, that would only add to the compilation classpath, but not to the runtime classpath. This kind may not be supported by all project types, e.g. the Ant-based Java SE project, but it would be supported by Maven project or Ant-based EE projects.

BTW, I am filing this as a defect because the lack of clear semantics may affect clients of the API, but feel free to change to TASK or ENHANCEMENT if that's more appropriate.
Comment 1 Denis Anisimov 2012-01-25 13:01:54 UTC
Does ClassPath.EXECUTE behave as requested ClassPath.COMPILE_ONLY ?
If not what is the purpose of ClassPath.EXECUTE ?
It is not clear from the doc.
Does it work the same for Ant based projects and Maven  if answer is yes ?
Comment 2 Tomas Zezula 2012-01-25 13:31:31 UTC
The classpath has nothing in common with project system, it's project independent.
Any project is allowed to set up runtime cp as needed.
The ClassPath.COMPILE means compile classpath of the file.
The ClassPath.RUNTIME means runtime classpath if the file.

There is no relation among RT and COMPILE cp, it's project specific behavior.
The Ant base project by default sets the RT as ${COMPILE}:${build.classes}
Comment 3 Tomas Zezula 2012-01-25 13:37:07 UTC
CP.EXECUTE JDoc says:
     * Classpath setting for executing things. This type can be used to learn
     * runtime time classpath for execution of the file in question.
     * <p class="nonnormative">
     * It corresponds to the <code>-classpath</code> option to <code>java</code>
     * (the Java launcher): 

CP.COMPILE JDoc says:
     * ClassPath for compiling things. This type can be used to learn
     * compilation time classpath for the file in question.
     * <p class="nonnormative">
     * It corresponds to the <code>-classpath</code> option to <code>javac</code>
 
The difference is obvious from JDoc.
CP.E  -> java -classpath
CP.C  ->javac -classpath

There is no relation among CP.E & CP.C.
In most cases CP.C is subset of CP.E but it's not true generally.
Comment 4 Denis Anisimov 2012-01-25 13:46:46 UTC
There is no ClassPath.RUNTIME.
Did you mean ClassPath.EXECUTE instead of  ClassPath.RUNTIME ?
Comment 5 Tomas Zezula 2012-01-25 14:14:00 UTC
Yes.
Comment 6 Petr Jiricka 2012-01-25 14:23:23 UTC
Ok, so maybe the problem is not in ClassPath, but in ProjectClassPathModifier. The Javadoc for this class makes absolutely no mention that if you add an item to some kind of classpath, some other kind of classpath may be modified as well. This makes this API useless for some scenarios, when you as a client need control over which kinds of classpath should NOT contain a particular item. 

Anyway, Tomas, do you have any constructive comment regarding the "add to compile-time, but not to the execute classpath" usecase? See also bug 206529 for a real-world scenario.
Comment 7 Tomas Zezula 2012-01-25 15:04:49 UTC
>Ok, so maybe the problem is not in ClassPath, but in ProjectClassPathModifier.
As far as I understand the problem is in Web project.
>The Javadoc for this class makes absolutely no mention that if you add an item
>to some kind of classpath, some other kind of classpath may be modified as
>well. This makes this API useless for some scenarios, when you as a client need
>control over which kinds of classpath should NOT contain a particular item.
It simply does not know it. It's natural that the CP.C is subset of CP.E but it's not required, as I've already explained. The ProjectClassPathModifier just delegates on concrete project type, the authors of these project types can add nonnormative info there. But it does not solve you problem.
As far as I understand from scanning the issue #206529 it's a web project specific problem. And the correct solution is already suggested by Petr H.

>I would recommend adding those jars on project classpath (specs.support may
>provide them) while excluding them from packaging (may require change in
>web.project or generic CP infrastructure)

Adding these jars only on compile cp while not having them on execute cp does not seem to  me as a good solution because these jars should be at lest on unit test runtime cp. The better solution is having them on runtime cp but exclude them from packaging.

Also the Ant project has the runtime classpath pre-set as a super set of compile classpath java.classpath=${compile.classpath}:${build.classes}. To create a compile only classpath it will become more complex as javac.classpath will need to be split into 2 parts, something like:

javac.classpath=${base.javac.classpath}:${compile.only.javac.classpath}

this will introduce a big problem in resource ordering, you will not be able to mix the order of  base and compile.only resources.

The solution is to define a special type of class path COMPILE_NO_RT (like we do for endorsed cp) and handle it in Web PCPM by adding the resource into compile cp as now but marking it as exclude from packaging. The compatibility for non web projects can be solved by fallback to CP.COMPILE when COMPILE_NO_RT is not supported. The PCM does not interpret the CP type, it just delegates it to project type. Should be quite easy to do.
Comment 8 Petr Jiricka 2012-01-25 15:29:38 UTC
First of all:

> The solution is to define a special type of class path COMPILE_NO_RT

Sounds like we are in violent agreement! That's exactly my proposal. I am reflecting the summary of this issue to reflect this.

> ...and handle it in Web PCPM by adding the resource into compile cp as now but marking it as exclude 
> from packaging

Yes, and also Maven project would want to handle it. I agree it should be unsupported in the JavaSE project.

More notes:
- I agree what Petr H suggests is the correct solution; I am just thinking how to implement it using our APIs; there should be just one API that would work for both Ant and Maven web projects.
- You bring up a good point about classpath for running unit tests, we need to think about that

Regarding:
> The better solution is having them on runtime cp but exclude them from packaging.

Note that in web projects, there is no "runtime cp". Packaging defines the runtime classpath.
Comment 9 Tomas Zezula 2012-01-25 15:43:38 UTC
The definition of the ClassPath type does not belong to ClassPath. As it's web specific should be a part of web project apis.

Anyway there is a mess with the CP constants.
Some of them are in CP, there is ClassPathSupport.ENDORSED and JavaClassPathConstants.PROCESSOR_PATH. I've worked on API change to deprecate them and create a single place.
Comment 10 Petr Jiricka 2012-01-25 16:31:40 UTC
No, this is not web-specific. This type of classpath can exist in any kind of Maven project. True, web apps are probably the most prominent use case, but in principle this is not limited to web apps. In fact, there is already o.n.m.maven.classpath.CPExtender.CLASSPATH_COMPILE_ONLY, which is generic for any Maven project as is included in java cluster. So defining this constant in enterprise cluster makes no sense.

But point taken that this does not belong to ClassPath, so how about JavaClassPathConstants?
Comment 11 Tomas Zezula 2012-01-25 16:37:24 UTC
What's the use case for maven's CPExtender.CLASSPATH_COMPILE_ONLY? Isn't it a web container?
JavaClassPathConstants may be good for it if it's generic even for j2se.
Comment 12 Petr Jiricka 2012-01-25 16:47:47 UTC
The implementation in Maven is that dependency is added with provided scope, see: http://docs.codehaus.org/display/MAVENUSER/Dependency+Scopes

"This dependency is needed for compiling and/or running the artifact but is not necessary to include in the package, because it is provided by the runtime environment"

I am not a Maven expert, but I think: the "runtime environment" could be a web container, but it does not need to be - it could be any kind of container. And it's not tied to a particular packaging type.

In NetBeans, the truth is that the only usages of this are in the web-centric areas, but it does not need to be so in principle.
Comment 13 Jesse Glick 2012-01-25 17:10:45 UTC
(In reply to comment #11)
> What's the use case for maven's CPExtender.CLASSPATH_COMPILE_ONLY? Isn't it a
> web container?

No, scope=provided is used in various kinds of projects. It may be that only EE-related IDE modules ask to use this scope when adding a library, but that might not always be the case.


JavaClassPathConstants is probably the right place for this. CompilationOnlyClassPathModifier should be deprecated or deleted, as should maven.classpath.CPExtender.CLASSPATH_COMPILE_ONLY.


Also this curious diff block in 9cbd59832985 (pjiricka) should be revisited:

if (NbMavenProject.TYPE_EJB.equals(nbprj.getPackagingType()) || NbMavenProject.TYPE_WAR.equals(nbprj.getPackagingType())) {
  dependency.setScope(Artifact.SCOPE_PROVIDED);
}

Not sure what this was for, but I hope it can be deleted.


We also need to fix org.netbeans.modules.java.api.common.classpath.ClassPathSupport.ENDORSED, which ought to be in ClassPath itself IMHO; not sure if that is already filed somewhere. (BTW this is the only reason maven depends on java.api.common.)


I also found in Maven's ClassPathProviderImpl:

if (type.equals("classpath/packaged"))
  //a semi-private contract with visual web.
  return getProvidedClassPath();

which seems like it is related. web.project has the same secret classpath type, where it bears the comment "packaged classpath = compilation time classpath - J2EE platform classpath" and seems to correspond to just ${javac.classpath}. Used by org.netbeans.modules.visualweb.insync.ModelSet. Not clear to me why this was needed; probably it can be deleted unless someone wants to resurrect visualweb.
Comment 14 Tomas Zezula 2012-01-25 17:19:04 UTC
JavaClassPathConstants is OK in this case if it's not J2EE specific.

>org.netbeans.modules.java.api.common.classpath.ClassPathSupport.ENDORSED, which
>ought to be in ClassPath
No! It definitely does not belong to ClassPath as ClassPath is not java specific, even the other constants in CP should be deprecated and added into JCPC. I've already had some API review for it somewhere but I was not able to fix deprecation warnings in ruby. Now it seems that the problem is resolved.
Comment 15 Jesse Glick 2012-01-25 17:21:57 UTC
(In reply to comment #14)
>> java.api.common.classpath.ClassPathSupport.ENDORSED ought to be in ClassPath
>
> It definitely does not belong to ClassPath as ClassPath is not java
> specific, even the other constants in CP should be deprecated and added into
> JCPC.

Agreed, JCPC would be appropriate. But java.api.common is the wrong place for it; was just put here out of expediency (to not go through real API review).
Comment 16 Tomas Zezula 2012-01-25 17:31:09 UTC
>But java.api.common is the wrong place for it; was just put here out of expediency
Unfortunately yes, this module became an API trash containing API which is OK and can be directly put into the official APIs and parts which should be redesigned to become an official APIs (UpdateHelper, BaseActionProvider). I planned a task to clean this module http://wiki.netbeans.org/EditorPlan71 unfortunately was not approved. Hopefully next release.
Comment 17 David Konecny 2012-01-25 22:39:31 UTC
(In reply to comment #16)
> >But java.api.common is the wrong place for it; was just put here out of expediency
> Unfortunately yes, this module became an API trash

I would not be that harsh on java.api.common. :-) It is not nice but it does help a lot. It is post-NetBeans4.0-work.
Comment 18 Tomas Zezula 2012-01-26 10:44:42 UTC
Yes, they are helpful.
They are mostly impls moved from j2se project and some of them made more generic. It's definitely better than having n copies of j2se project impls. But they are not usable for those who  are not in nb.org. They cannot use them as they are no friends, so people needs to copy them anyway. :-(
What I want is to fix (rewrite some of them) and dissolve this module into regular API modues (java.project, project.ant).
Comment 19 Tomas Zezula 2012-01-26 15:34:01 UTC
Here is the original API review of the move of CP types, issue #181474.
Comment 20 Tomas Zezula 2012-05-23 14:24:57 UTC
Fixed jet-main dba7adbe4edb
Comment 21 Jesse Glick 2012-05-23 15:19:49 UTC
jet-main #dba7adbe4edb has not been pushed yet but I suppose that is just the API change (bug #212694). The broader issue is still open:

(In reply to comment #13)
> CompilationOnlyClassPathModifier should be deprecated or deleted, as should
> maven.classpath.CPExtender.CLASSPATH_COMPILE_ONLY.
> 
> 
> Also this curious diff block in 9cbd59832985 (pjiricka) should be revisited:
> 
> if (NbMavenProject.TYPE_EJB.equals(nbprj.getPackagingType()) ||
> NbMavenProject.TYPE_WAR.equals(nbprj.getPackagingType())) {
>   dependency.setScope(Artifact.SCOPE_PROVIDED);
> }
> 
> Not sure what this was for, but I hope it can be deleted.
> 
> 
> I also found in Maven's ClassPathProviderImpl:
> 
> if (type.equals("classpath/packaged"))
>   //a semi-private contract with visual web.
>   return getProvidedClassPath();
Comment 22 Petr Jiricka 2012-05-23 15:52:54 UTC
> Also this curious diff block in 9cbd59832985 (pjiricka) should be revisited

I agree, this was introduced as a fix for bug 177214. But it may be irrelevant by now, as I believe that the IDE no longer puts web project on the classpath of an ejb project, ever. The right approach to accomplish the remote EJB scenario as discussed in bug 177214 is to create a Java library containing the remote interfaces, which is then put on the classpath of both the ejb project and web project, and this is what the IDE leads you to do I believe. Need to check though.
Comment 23 Tomas Zezula 2012-05-23 16:19:55 UTC
Yes, it's only the java/classpath change.
The rest is maven (web project) specific as j2se does not support compile only.
I've understood this issue as a request for the API change if it should cover also the changes in web | maven project I am reassigning it to web project and then to maven.
Comment 24 Quality Engineering 2012-05-25 05:46:13 UTC
Integrated into 'main-golden', will be available in build *201205250002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/dba7adbe4edb
User: Tomas Zezula <tzezula@netbeans.org>
Log: #207717:ProjectClassPathModifier needs classPathType for compile-only (Maven scope=provided)
Comment 25 David Konecny 2012-07-17 02:00:25 UTC
Created attachment 122078 [details]
patch

Attached is patch which fixes Ant Web Project.

Tomas, in the patch I'm making several methods in org.netbeans.modules.java.api.common.classpath.ClassPathSupport public otherwise I cannot delegate to them and I do not want to reimplement them again. Is that OK with you? Or would you prefer I do it differently?

The patch does following:

* deprecates org.netbeans.modules.j2ee.common.project.CompilationOnlyClassPathModifier - COMPILE_ONLY can be used instead of it

* deprecates org.netbeans.modules.web.project.api.WebProjectLibrariesModifier because it extends CompilationOnlyClassPathModifier and introduces instead WebProjectLibrariesModifier2

* WebProject and AppClient provide implementation of ProjectClassPathModifierImplementation in their lookups which delegates to existing code
Comment 26 David Konecny 2012-07-17 03:24:39 UTC
Created attachment 122080 [details]
patch

I'm also removing from Ant Web Project handling of "classpath/packaged". It was there only for VisualWeb (issue 131785) and is therefore not needed.

Do you think we need to enhance ClassPathProvider to return classpath for COMPILE_ONLY classpath type? I do not think it is needed in practice - 3rd party code checks COMPILE classpath and then decides to add something to either COMPILE or COMPILE_ONLY classpath but knowing content of COMPILE_ONLY is not needed I think.
Comment 27 David Konecny 2012-07-17 03:30:26 UTC
Created attachment 122081 [details]
patch

And last patch for Maven. Few notes:

* CPExtender.addProjects does not need to check project type anymore. Client modifying classpath should do the test and use either COMPILE or COMPILE_ONLY classpath type. However I could not find the client code and therefore have not fixed the client. Any idea who this is or where?

* maven/j2ee/web/EntRefContainerImpl.java still checks project type but I think this case is correct and should stay as is

And again similar question as for Ant Web Project: should ClassPathProviderImpl return some value for COMPILE_ONLY?
Comment 28 Denis Anisimov 2012-07-17 06:33:07 UTC
(In reply to comment #27)

> And again similar question as for Ant Web Project: should ClassPathProviderImpl
> return some value for COMPILE_ONLY?

Do you mean that COMPILE classpath includes COMPILE_ONLY classpath ?
So there is no need to check content of COMPILE_ONLY because it is 
a subset of COMPILE classpath?

In the latter case I would agree that COMPILE_ONLY value is not required.
Comment 29 Milos Kleint 2012-07-17 13:43:34 UTC
(In reply to comment #27)
> Created attachment 122081 [details]
> patch
> 
> And last patch for Maven. Few notes:
> 
> * CPExtender.addProjects does not need to check project type anymore. Client
> modifying classpath should do the test and use either COMPILE or COMPILE_ONLY
> classpath type. However I could not find the client code and therefore have not
> fixed the client. Any idea who this is or where?

most likely just visual web. or something else in web projects, COMPILE_ONLY("provided" maven scope) is used there often.

> 
> * maven/j2ee/web/EntRefContainerImpl.java still checks project type but I think
> this case is correct and should stay as is
> 
> And again similar question as for Ant Web Project: should ClassPathProviderImpl
> return some value for COMPILE_ONLY?
Comment 30 Petr Jiricka 2012-07-17 14:14:54 UTC
> Any idea who this is or where?

I found j2ee.ejbcore/src/org/netbeans/modules/j2ee/ejbcore/api/codegeneration/SessionGenerator.java. See bug 177214 and bug 186221.
Comment 31 David Konecny 2012-07-17 22:37:18 UTC
(In reply to comment #30)
> j2ee.ejbcore/src/org/netbeans/modules/j2ee/ejbcore/api/codegeneration/SessionGenerator.java.
> See bug 177214 and bug 186221.

I've seen these changes and I think they are handled properly in my patch. 

(In reply to comment #28)
> Do you mean that COMPILE classpath includes COMPILE_ONLY classpath ?
> So there is no need to check content of COMPILE_ONLY because it is 
> a subset of COMPILE classpath?

Yes.

OK, I will test it little bit more today and if I do not find any problems I will push the changes. Thanks for your review guys.
Comment 32 David Konecny 2012-07-18 00:38:00 UTC
OK, I pushed the changes (web-main#9ea6e655649f) and fixed two problems I found during testing (web-main#38396b2f5afd). If there is more issues please assign them to me. Thanks.
Comment 33 Petr Jiricka 2012-07-18 08:31:55 UTC
So should this be marked as fixed?
Comment 34 David Konecny 2012-07-18 10:12:59 UTC
Yes, sorry.
Comment 35 TomasKraus 2012-09-07 14:17:13 UTC
*** Bug 206529 has been marked as a duplicate of this bug. ***
Comment 36 Martin Janicek 2012-11-07 08:56:43 UTC
*** Bug 189331 has been marked as a duplicate of this bug. ***
Comment 37 Martin Janicek 2013-01-16 15:55:32 UTC
*** Bug 180183 has been marked as a duplicate of this bug. ***