Bug 192647 - Provide classpaths for ~/.m2/repository/**/*-sources.jar
Provide classpaths for ~/.m2/repository/**/*-sources.jar
Status: STARTED
Product: java
Classification: Unclassified
Component: Source
7.0
All All
: P3 (vote)
: 7.2
Assigned To: Tomas Zezula
issues@java
: PLAN
Depends on: 190849 191518
Blocks: 192214
  Show dependency treegraph
 
Reported: 2010-11-29 21:33 UTC by Jesse Glick
Modified: 2012-04-09 22:07 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
patch (11.70 KB, patch)
2012-02-20 13:43 UTC, Milos Kleint
Details | Diff
Fix for Find Usages. (1.01 KB, patch)
2012-02-28 15:24 UTC, Jan Becicka
Details | Diff
Previous patch wrongly attached (1.07 KB, patch)
2012-02-28 15:27 UTC, Jan Becicka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2010-11-29 21:33:24 UTC
See bug #192214 comment #9.
Comment 1 Milos Kleint 2012-02-17 15:34:04 UTC
to get the complete CP for the source jar we will need to resolve the pom. there are cases where pom is missing, but that's file, because the lib is probably self sustaining..


there is going to be a time delay before the CP is constructed properly, in most cases all should be cached locally but I believe in some instances it will still download stuff from remote places, like when a project that depends on A also depends on B and fixates B's version to 1.0, but A alone uses 1.1.. when we resolve CP for A's source jar, the B 1.1 version will have to be downloaded. (no project reloads, priming or builds will ever download the 1.1 version.


for release binaries I think we can cache the result of the computation for subsequent IDE restarts
Comment 2 Milos Kleint 2012-02-20 13:43:25 UTC
Created attachment 115950 [details]
patch

patch containing the implementation of ClassPathProvider for source jars in local maven repository.
Comment 3 Jesse Glick 2012-02-20 22:05:00 UTC
(In reply to comment #1)
> there are cases where pom is missing, but that's [fine], because the lib is
> probably self sustaining..

Right, these would be manually uploaded binaries and we can assume it has no deps.

> in some instances it will still download stuff from remote places

The best behavior would be to use the project embedder initially, then asynchronously use the online embedder and fire changes. Not sure if it is worth the effort. But using the online embedder synch is bad; better to only use the project embedder, even at the expense of incorrectly producing an empty classpath for some unusual cases.

> for release binaries I think we can cache the result of the computation for
> subsequent IDE restarts

Perhaps, but probably not worth the bother.

Since you are introducing a new global service, be sure to run with -verbose:class to verify that loading RepositoryMavenCPProvider does not trigger loading of referenced classes (e.g. EmbedderFactory) unless the provider is actually returning a result.

Converting to FileObject for FileUtil.isParentOf and getRelativePath may be overkill; try using toURI (on normalized files!) and URI.relativize.

Use JavaPlatform.getBootstrapLibraries for BOOT - a one-liner and you can delete createBootCPI.

createCompileCPI should not add binary (and does not need this param at all); only createExecuteCPI should use it. You can also add it to the CP whether the File happens to exist at the moment or not.

Not obvious why we would add remote repos from RepositoryPreferences here... if the POM needs another repo it should mention it, right? I guess the problem is with POMs whose parent POM defined the repo, and the _maven.repositories file only specifies the ID of its owning repo. There are other cases (e.g. SourceJavadocAttacher) where we are doing something with a repo artifact without any particular project context; this kind of behavior needs to be reviewed.
Comment 4 Milos Kleint 2012-02-21 08:20:51 UTC
(In reply to comment #3)

> > in some instances it will still download stuff from remote places
> 
> The best behavior would be to use the project embedder initially, then
> asynchronously use the online embedder and fire changes. Not sure if it is
> worth the effort. But using the online embedder synch is bad; better to only
> use the project embedder, even at the expense of incorrectly producing an empty
> classpath for some unusual cases.

The online embedder should have the same performance characteristics for non-snapshot binaries with the exception of the unusual cases mentioned. But should be always right.


> 
> > for release binaries I think we can cache the result of the computation for
> > subsequent IDE restarts
> 
> Perhaps, but probably not worth the bother.
> 
> Since you are introducing a new global service, be sure to run with
> -verbose:class to verify that loading RepositoryMavenCPProvider does not
> trigger loading of referenced classes (e.g. EmbedderFactory) unless the
> provider is actually returning a result.

Well, I'm using the embedder factory stuff as the last condition applied before deciding, but it's mandatory because we only want to handle items in the local repository cache..

> 
> Converting to FileObject for FileUtil.isParentOf and getRelativePath may be
> overkill; try using toURI (on normalized files!) and URI.relativize.
> 
> Use JavaPlatform.getBootstrapLibraries for BOOT - a one-liner and you can
> delete createBootCPI.

ok

> 
> createCompileCPI should not add binary (and does not need this param at all);
> only createExecuteCPI should use it. You can also add it to the CP whether the
> File happens to exist at the moment or not.

jlahoda claims otherwise in #192214 (the compile cp)

> 
> Not obvious why we would add remote repos from RepositoryPreferences here... if
> the POM needs another repo it should mention it, right? 

not really reliable to base the list on pom only. the recommended usage is to bootstrap repositories in settings file. If I'm not mistaken, it didn't work for me when I specified nothing..  will double check


I guess the problem is
> with POMs whose parent POM defined the repo, and the _maven.repositories file
> only specifies the ID of its owning repo. There are other cases (e.g.
> SourceJavadocAttacher) where we are doing something with a repo artifact
> without any particular project context; this kind of behavior needs to be
> reviewed.

the _maven.repositories files are irrelevant for artifact resolution AFAIK, it's only used for caching queries to remote repositories to limit the number of remote calls.
Comment 5 Milos Kleint 2012-02-21 09:15:50 UTC
reassigning to jbecicka for evaluation
Comment 6 Jesse Glick 2012-02-21 18:50:43 UTC
(In reply to comment #4)
> The online embedder should have the same performance characteristics for
> non-snapshot binaries with the exception of the unusual cases mentioned. But
> should be always right.

I know. My point is that for the unusual cases, it is better to return quickly with an incorrect result than to block queries on a network call.

>> be sure to run with -verbose:class
> 
> the embedder factory stuff as the last condition applied before deciding

Right, just use -verbose:class to verify that this actually works the way you think, and that other classes are not getting loaded eagerly.

>> createCompileCPI should not add binary
> 
> jlahoda claims otherwise in #192214 (the compile cp)

[That is bug #192214 comment #9 - use the "bug #" prefix and optional "comment #" syntax to force hyperlinks in BZ.]

OK, seems counterintuitive to me for source files to specify their own binaries on COMPILE, but if that what the Java infrastructure needs then that is what we have to do. (java.j2seplatform seems to do the same for Ant libraries.)

> the _maven.repositories files are irrelevant for artifact resolution AFAIK,
> it's only used for caching queries

For command-line usage, yes; I am just saying it would be useful for our purposes (inspecting repository JARs without further context) to know what actual repo they came from.
Comment 7 Milos Kleint 2012-02-28 12:50:43 UTC
reassigning to jbecicka for evaluation

jglick: once we have a working solution I'll incorporate your comments on the maven side.
Comment 8 Jan Becicka 2012-02-28 15:24:23 UTC
Created attachment 116185 [details]
Fix for Find Usages.
Comment 9 Jan Becicka 2012-02-28 15:27:19 UTC
Created attachment 116186 [details]
Previous patch wrongly attached

Fix for Find Usages
Comment 10 Jan Becicka 2012-02-28 15:33:10 UTC
Unfortunately after my fix, there are some more issues with Find Usages for classes from src.zip
Problem seems to be in SourceUtils.getDependentRoots
Comment 11 Jan Becicka 2012-02-29 14:39:10 UTC
Tomasi, please apply my latest patch.
Create a new JavaProject and try Find Usages for instance on String. 
SourceUtils.getDependentRoots(src.zip) should return all source roots from all projects, which has jdk corresponding to src.zip on their class path. It works with maven projects (at least with Milos' patch), but not with J2se projects.
Comment 12 Milos Kleint 2012-03-13 12:26:30 UTC
on maven side, applied patch https://hg.netbeans.org/core-main/rev/85f62d16baea (including changes suggested by jesse)
Comment 13 Quality Engineering 2012-03-17 10:41:48 UTC
Integrated into 'main-golden', will be available in build *201203170400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/85f62d16baea
User: Milos Kleint <mkleint@netbeans.org>
Log: #192647 a special global CPP implementation for source jars in local maven repository.
Comment 14 Jesse Glick 2012-04-09 22:07:40 UTC
I am getting warnings in log which are probably due to this:

WARNING [org.netbeans.modules.java.source.parsing.JavacParser]: ClassPath identity changed for AbstractFileObject@...[org/apache/felix/bundleplugin/BundlePlugin.java], class path owner: null original sourcePath: .../org/apache/felix/maven-bundle-plugin/2.3.7/maven-bundle-plugin-2.3.7-sources.jar new sourcePath: .../org/apache/felix/maven-bundle-plugin/2.3.7/maven-bundle-plugin-2.3.7-sources.jar

I guess RepositoryMavenCPProvider is supposed to be providing the exact same ClassPath object when asked repeatedly about the same *-sources.jar, at least for SOURCE. Not sure exactly how that should be done without creating a memory leak.

JavacParser checks scp != cpInfo.getClassPath(PathKind.SOURCE); could it use !equals instead (*), given the recent change to make ClassPath.equals be based on ClassPathImplementation.equals, or would it be losing listeners that way?

(*) I.e. !cpInfo.gCP(SOURCE).equals(scp); it seems that ClasspathInfo.srcClassPath is never null, even though ClasspathInfo.hashCode/equals perform a null check on it.


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