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.
Summary: | Provide classpaths for ~/.m2/repository/**/*-sources.jar | ||
---|---|---|---|
Product: | java | Reporter: | Jesse Glick <jglick> |
Component: | Source | Assignee: | Tomas Zezula <tzezula> |
Status: | RESOLVED WONTFIX | ||
Severity: | normal | CC: | anebuzelsky, jbecicka, jlahoda, mkleint, pjiricka |
Priority: | P3 | Keywords: | PLAN |
Version: | 7.0 | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Bug Depends on: | 190849, 191518 | ||
Bug Blocks: | 192214 | ||
Attachments: |
patch
Fix for Find Usages. Previous patch wrongly attached |
Description
Jesse Glick
2010-11-29 21:33:24 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 Created attachment 115950 [details]
patch
patch containing the implementation of ClassPathProvider for source jars in local maven repository.
(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. (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. reassigning to jbecicka for evaluation (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. reassigning to jbecicka for evaluation jglick: once we have a working solution I'll incorporate your comments on the maven side. Created attachment 116185 [details]
Fix for Find Usages.
Created attachment 116186 [details]
Previous patch wrongly attached
Fix for Find Usages
Unfortunately after my fix, there are some more issues with Find Usages for classes from src.zip Problem seems to be in SourceUtils.getDependentRoots 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. on maven side, applied patch https://hg.netbeans.org/core-main/rev/85f62d16baea (including changes suggested by jesse) 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. 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. Report from old NetBeans version. Due to code changes since it was reported likely not reproducible now. Feel free to reopen if happens in 8.0.2 or 8.1. |