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 170056 - Separate module for Lookup API
Summary: Separate module for Lookup API
Alias: None
Product: platform
Classification: Unclassified
Component: Lookup (show other bugs)
Version: 6.x
Hardware: All All
: P3 blocker (vote)
Assignee: Jaroslav Tulach
Depends on: 83734 178260 178807
  Show dependency tree
Reported: 2009-08-07 16:35 UTC by Jesse Glick
Modified: 2009-12-20 23:55 UTC (History)
4 users (show)

See Also:
Issue Type: TASK
Exception Reporter:

hg diff --git -r fa9887b26708:5662c2e3f7b9 (2.63 KB, patch)
2009-11-08 14:06 UTC, Jaroslav Tulach
Details | Diff
hg diff --git -r fa9887b26708:5662c2e3f7b9 (113.60 KB, patch)
2009-11-11 03:32 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2009-08-07 16:35:28 UTC
See issue #169844 for background. Suggest lookup/src/org/netbeans/modules/lookup/{Lookup*.java,lookup/} or similar, with
Comment 2 Jaroslav Tulach 2009-11-08 14:06:58 UTC
Created attachment 90561 [details]
hg diff --git -r fa9887b26708:5662c2e3f7b9

The patch introduces new module openide.util.lookup and changes dependencies of related platform modules to use it. It also improves the fix-dependencies task and runs the task before compilation (so the update of dependencies is done automatically). There is no  ModuleAutoDeps right now, as the new module will stay on JDK classpath and there shall be no runtime issues.
Comment 3 Jaroslav Tulach 2009-11-08 14:11:46 UTC
The proposed change in versioning is to increase both versions of openide.util and openide.util.lookup to 8.0 and split their apichanges.xml appropriately. Dependencies of all modules in the source tree will need to be updated as shown in patch
Comment 4 Antonin Nebuzelsky 2009-11-09 07:51:49 UTC
As this is a source incompatible change, a change in apisupport should be introduced together with this change to warn the users / help them fix the dependency their projects have on lookup.
Comment 5 Jaroslav Tulach 2009-11-10 01:52:40 UTC
Re. TN01: Right now there is a change in harness to automatically modify the project.xml to contain dependency on openide.util > 8.0 as well as openide.util.lookup > 8.0. So it is enough to compile and the dependencies are fixed.
Comment 6 Milos Kleint 2009-11-10 02:33:27 UTC
a change in harness will not cater to maven users unfortunately. OTOH it should be sort of easy to find the artifact for classes once people recognize the problem. Can we make this incompatible change visible in documentation somehow? In addition to the apichanges document which very few people actually read.

Why introduce this a week before Code freeze BTW?
Comment 7 Jaroslav Tulach 2009-11-10 02:59:47 UTC
Re. dependencies on bug 83734 and bug 167817: I consider those nice to have. The current diff achieves the same without any general infrastructure.
Comment 8 rmichalsky 2009-11-10 03:31:43 UTC
Just a couple of comments:

RM01: The attachment doesn't contain patch to fix-dependencies.

RM02: Is the case of newer harness used with older platform covered? Dependencies should not be updated in this case and this is the default when someone builds against non-IDE platform.

RM03: It will nonetheless break build of projects with custom and/or older harness and newer platform, e.g. JavaFx.

RM04: Build process that changes version-controlled files without being told so can cause confusion and some problems for users.

I also agree with Milos that this change needs to be documented visibly, especially because of points 3 and 4.
Comment 9 Antonin Nebuzelsky 2009-11-10 10:31:19 UTC
> Why introduce this a week before Code freeze BTW?

This change is not for 6.8!
Comment 10 Jaroslav Tulach 2009-11-11 03:32:35 UTC
Created attachment 90782 [details]
 hg diff --git -r fa9887b26708:5662c2e3f7b9

Providing correct diff. Sorry.
Comment 11 Jesse Glick 2009-11-12 12:10:35 UTC
[JG01] -1 on any special handling for this refactoring in harness. Please use ModuleAutoDeps only. We are in no rush so there is no reason to do this the wrong way.

[JG02, seconding RM04] Do not call fix-dependencies from basic-init, nor fix-test-dependencies from build-test-dist. Modifying source files during a build is unacceptable. If a module has obsolete dependencies acc. to ModuleAutoDeps, correct them transiently in <jarwithmoduleattributes> and issue a warning suggesting that the user run fix-dependencies at their convenience. Optionally, improve apisupport.project to put a warning icon on Libraries in case a MAD rule is triggered and add a context menu item to run fix-dependencies.

[JG03] Do not use <implementation-version/> for deps on org.openide.util.lookup. Please correct all instances in your patch to use <specification-version/> with the exception of core pseudomodules which were already using an impl dep on org.openide.util for reasons of access to some class in org.netbeans.modules.openide.util now moved to openide.util.lookup, such as NamedServicesProvider. In particular, openide.nodes does not appear to need any impl dep on openide.util.lookup; do not blindly transfer impl deps from openide.util, evaluate each individually (and check whether it is possible to remove the impl dep on openide.util if one is being added on openide.util.lookup).

[JG04] Do not use Logger.getLogger("global") or its equivalent. Use a logger based on the containing class name.

RM02 is I think just too bad; if you want to build against 6.8+1 platform, you will need to either use 6.8+1 harness, or manually update your dependencies to include org.openide.util.lookup. We make no promises that all future Platform releases can be built using an arbitrarily old harness.
Comment 12 Jaroslav Tulach 2009-12-07 04:49:58 UTC
Jesse, Richard. Thanks for your comments. However most of them are depending on better handling of ModuleAutoDeps in harness. I do not feel I have power to implement that. Will one of you help me?
Comment 13 Jaroslav Tulach 2009-12-11 07:53:32 UTC
The new ModuleAutoDeps stuff works fine. Thanks Jesse. I removed the modification of the build script before compilation. I'll process your other comments by Dec 17 and then I'd like to integrate the change.
Comment 14 Jaroslav Tulach 2009-12-14 13:07:50 UTC
Re. RM02: The new patch achieves that by using ModuleAutoDeps. Nothing is updated until user explicitly invokes fix-dependencies. Then the dependencies are updated to last state known by the harness. In case this is not what people want, they can always update their dependencies manually.

Re. RM04, JG02: OK. No longer the case.

Re. RM03: You mean it will break old, custom modified harnesses. Well, that is said, but such people just have to properly upgrade.

Re. JG01: Done now, thanks to Jesse's help.

Re. JG03: Fixed in ac605796011f for openide.nodes and openide.awt

Re. JG04: Common, it is just in a test.

I am providing the current diff to tentatively scheduled for integration on Dec 17. In addition to previous, it mostly contains fixes of javadoc links:
Comment 15 Jesse Glick 2009-12-14 15:19:33 UTC
JG01 does not seem to be done in FixTestDependencies, which continues to have a fixOpenideUtil method. Nor does it seem to be done in nbbuild/templates/common.xml, which also has special handling for this separation.

[JG05] Second hunk in nbbuild/build.xml (<subant-junit buildpath="${nb_all}${file.separator}performance"...>) looks suspicious and is probably wrong. Mishandled merge conflict perhaps? Remove.

[JG06] Misplaced indentation in o.n.bootstrap/nbproject/project.xml. Fix, and make sure it is not the fault of a tool like fix-dependencies (which should really be using XMLUtil to read/write structure, not doing textual substitutions).

[JG07] Bogus URL in openide.util.lookup/src/org/openide/util/package.html.

[JG08] Typo in openide.util/module-auto-deps.xml#//description.

[JG09] openide.util/nbproject/ is missing an extra ".0".
Comment 16 Jaroslav Tulach 2009-12-17 06:28:52 UTC
Thanks for throughout review. I plan to integrate on Saturday, Sunday Dec 19-20, 2009. Latest diff:

Re. JG07: No, this is correct URL which will be replaced during javadoc to proper reference.

Re. JG08: Fixed in b66ff57336d7

Re. JG09: Fixed in eb209c9e1d24

Re. JG06: The problem in o.n.bootstrap fixed in 0c0a09c2444a

Re. JG01: The fix-dependencies is now not executed automatically, it needs to be triggered manually. That also lowers the size of the problem. The fact that by default there is no "fixing" by default made me claim JG01 as fixed in my previous comment. I do not see the current state as blocker. The harness (common.xml in fact) already contains conversion rules for fixing dependencies. Adding new one for a module that almost everyone is depending on is OK. It is not general solution, but in case of openide.util it is justifiable. If there is a need for general solution, let's track it as separate issue. 

Re. JG05: Maintaining the branch is costly and not without errors (fixed in 989ad948c240). Let's integrate this highly desirable change now (when there is quiet time full of vacations). Remaining polishing can be tracked as separate issues.
Comment 17 Jesse Glick 2009-12-17 13:37:23 UTC
JG07 - http://top/org/openide/util/lookup/doc-files/lookup-api.html is replaced? By what? I think you meant @TOP@/org/openide/util/lookup/doc-files/lookup-api.html or similar.

JG01 - I filed bug #178807.
Comment 18 Jaroslav Tulach 2009-12-18 01:04:32 UTC
Re. javadoc. The meaning of http://top/ is the same as @TOP@, but for some reason @TOP@ does not work in package.html

Re. JG06: I found the indentation problem and fixed it in 9303708a12f8
Comment 19 Jaroslav Tulach 2009-12-19 14:09:32 UTC
Merged as core-main#58d27b9032b8
Comment 20 Quality Engineering 2009-12-20 23:55:34 UTC
Integrated into 'main-golden', will be available in build *200912210200* on (upload may still be in progress)
User: Jaroslav Tulach <>
Log: Merge of #170056: Splitting Lookup into own module