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 25988 - Undesirable effect caused by recent changes to lookup (meta-inf mixed with layer)
Summary: Undesirable effect caused by recent changes to lookup (meta-inf mixed with la...
Status: VERIFIED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Lookup (show other bugs)
Version: 3.x
Hardware: PC Windows ME/2000
: P3 blocker (vote)
Assignee: David Strupl
URL:
Keywords:
Depends on:
Blocks: 30425
  Show dependency tree
 
Reported: 2002-07-24 17:28 UTC by Martin Matula
Modified: 2008-12-22 19:34 UTC (History)
6 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Change in order how Lookup.getDefault () is composed, FolderLookup is the first (4.49 KB, patch)
2002-07-25 18:09 UTC, Jaroslav Tulach
Details | Diff
Enhances the MetaInfLookup with remove functionality (4.84 KB, patch)
2002-07-26 10:25 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Matula 2002-07-24 17:28:49 UTC
We have a MDR standalone library. This library can be used 
by standalone applications. It contains default 
implementation of MDRManager abstract class and 
implementation of bunch of other MDR API 
classes/interfaces. Users of MDR use lookup to find these 
implementations (or more precisely - abstract classes in 
API contain getDefault methods that search for 
implementation using org.openide.Lookup). After meta-inf 
lookup was introduced, we happily added manifests to the 
MDR engine jar files so that now our users don't need to 
implement their own lookup to make MDR work.
However we need different implementation of MDRManager 
when MDR is used in IDE. Till yesterday it worked fine - 
we register different MDRManager in mdr layer file. 
However since yesterday IDE merges meta-inf and layer 
lookup so the unwanted implementation of MDRManager gets 
returned from lookup. In addition, we were using lookup in 
IDE to control which implementation of e.g. JMIMapper is 
used (based on ordering). One implementation generates 
class files, the other one generates java sources. So 
removing registration from layer is not an option as 
registration in manifest cannot enforce ordering and it is 
not configurable. Both registrations (in manifest and in 
layer) in place cause that each object appears twice in 
lookup. An intermediate solution can be to give layer 
lookup higher priority than meta-inf lookup (this would 
solve our problem with MDRManager and ordering of 
JMIMappers), however a solution is needed also to suppress 
duplication of entries (by some kind of hiding?). In 
addition Yarda told me that it is not guaranteed that 
layer gets parsed soon enough for other modules to get 
instance of MDRManager from it (due to some changes in 
threading model?). That's also an issue as it can be 
source of non-deterministic runtime errors.
Comment 1 Jaroslav Tulach 2002-07-25 17:18:38 UTC
One way to get around the problem is to have a bit different set of
JARs for Nb module and for standalone library. Suppose:

mdr-api.jar manifest contains:
ClasPath: mdr-standalone.jar ext/mdr-inside-nb.jar

and the standalone distribution contains just the mdr-standalone.jar
with META-INF/services describing the standalone implementation of
MDRRepository.

When deployed as module, mdr-standalone.jar is missing and instead the
ext/mdr-inside-nb.jar could be provided registering the Nb specific
MDRManager (if necessary, layer registration will work as well) except
cases where a FileSystem or DataSystem code during initialization
would like to access the MDRManager, in such case the returned value
can be null, but such code is time sensitive and should not do such
things (hopefully).

I will try to come up with better solutions, but lowering to P2,
because workaround exists.


Comment 2 Jaroslav Tulach 2002-07-25 18:09:09 UTC
Created attachment 6903 [details]
Change in order how Lookup.getDefault () is composed, FolderLookup is the first
Comment 3 Jaroslav Tulach 2002-07-25 18:11:31 UTC
Martin, could you please try that the above patch works for you? It
does not hide the instances provided by META-INF/services, but the
content of /Services/ folder now preceeds the other registered services.

Jesse, do you think that this is good change? Or in other words will
it break a lot? Btw. it seems to make the special lookup for error
manager obsolete, but I am not sure?
Comment 4 Jaroslav Tulach 2002-07-26 10:25:35 UTC
Created attachment 6909 [details]
Enhances the MetaInfLookup with remove functionality
Comment 5 Jaroslav Tulach 2002-07-26 10:29:43 UTC
The patch above allows following solution to the problem:

mdr.jar defines META-INF/services/org.nb.api.mdr.MDRManager

that contains to the standalone implementation, say "org.nb.MDRImpl"

mdr-nb-module.jar defines the same file but with content "-org.nb.MDRImpl"

Thus in standalone version everything is defined correctly and the
module removes all such definitions + adds its own - either in
META-INF/services or in module-layer.xml

Comment 6 Jaroslav Tulach 2002-07-26 10:33:45 UTC
Personally I like the second patch, it solves the problem in general,
and our experience with module-layers also implies that sometimes
hiding is needed. So I would like to apply the second patch.

It seems to me that the first patch is also useful - layers are more
powerful and configurable and thus should override the settings of
META-INF. But I do not have such strong opinion here.

Jesse, please do the review and share your opinion.
Comment 7 Tomas Hurka 2002-07-26 11:07:16 UTC
I can confirm that the first patch works fine and solves the problem.
Comment 8 Martin Matula 2002-07-26 14:41:30 UTC
The second patch is great and it not only solves the problems with
lookup when runing with NB, but also it can be very useful for
standalone use of lookup.
But it may still be a good idea to apply also the first patch if there
is no problem with it.
Comment 9 _ briansmith 2002-07-26 20:25:55 UTC
I think that the removal aspect of this patch should be done by adding
an annotation in a comment, instead of extending the syntax of the
file. The reason is that there could be conflicts with other software
that reads meta-inf/services files and expects them to be in the
standard format (the JAXP API stuff does this, I believe). Also, it is
strange that one JAR file controls whether or not another JAR file
provides the service. Instead, I think it would be better if the
mdr.jar's META-INF/services/org.netbeans.api.mdr.MDRManager file
contained something like this:
    org.netbeans.mdr.MDRManagerImpl # [nb-ide-exclude]

The default lookup would ignore the comment, but the IDE lookup would
see the [nb-ide-exclude] annotation inside the comment and know to
ignore the entry.

Alternatively, the information about which META-INF/services entries
need to be removed/replaced could be put alongside the NBM module
manifest information, since NetBeans fully controls the format of that
stuff already.
Comment 10 Martin Matula 2002-07-29 17:28:12 UTC
Hi Brian,
I believe that the extended lookup functionality can be useful also
for standalone application. That's why I don't like the approach
specific to NB (like puting the information into NBMs or having
something like [nb-ide-exclude]). I agree however that it would be
cleaner to put the comment mark ("#") at the beginning of the line
that removes a class from lookup, so that instead of:
-org.netbeans.mdr.NBMDRManagerImpl
we would use:
#-org.netbeans.mdr.NBMDRManagerImpl
Or:
#[remove]org.netbeans.mdr.NBMDRManagerImpl
Or something else similar to that....
What do you think?
Comment 11 _ briansmith 2002-07-29 23:17:44 UTC
Sure, that seems fine. I think the important think is to keep the
files syntactically valid unless they will only be in JAR files that
are 100% Netbeans/OpenIde-specific.
Comment 12 _ hkrug 2002-08-02 07:24:10 UTC
Using todays CVS code (dev trunk) I get an exception probably related
to this issue. Following the comments to the issue I thought, it was
already solved. Are the patches not yet integrated or do I miss
something else ?

MDR seems to work despite the exception, so it should not be to big a
problem.

java.lang.ClassNotFoundException: -org/netbeans/mdr/NBMDRManagerImpl
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:207)
        at
org.openide.util.MetaInfServicesLookup.search(MetaInfServicesLookup.java:144)
        at
org.openide.util.MetaInfServicesLookup.beforeLookup(MetaInfServicesLookup.java:71)
        at
org.openide.util.lookup.AbstractLookup.lookupItem(AbstractLookup.java:250)
        at
org.openide.util.lookup.AbstractLookup.lookup(AbstractLookup.java:241)
        at
org.openide.util.lookup.ProxyLookup.lookup(ProxyLookup.java:128)
        at
org.netbeans.api.mdr.MDRManager.getDefault(MDRManager.java:53)
        at
org.netbeans.modules.mdrexplorer.looks.MDRManagerWrapper.getMDRManager(MDRManagerWrapper.java:37)
Comment 13 Jaroslav Tulach 2002-08-02 10:22:13 UTC
Not yet integrated, I'll work on it now.
Comment 14 Jaroslav Tulach 2002-08-02 10:53:26 UTC
Implemented the solution with #! so if you want to remove a class, do

#-org.netbeans.mdr.NBMDRManagerImpl

Passing to David to decide whether to implement the change in the
default folder.
Comment 15 Martin Matula 2002-08-02 13:36:48 UTC
Hi Jarda,
does it mean that it's been integrated to the main trunk? What is
"default folder"?
Comment 16 David Strupl 2002-10-22 15:15:56 UTC
Jarda - what is status of this? What is "default" folder"? Should this
issue be closed (since removing is already in MetaInfServicesLookup)?
Comment 17 Jaroslav Tulach 2002-10-29 14:28:38 UTC
The issue has been passed to you to decide whether to implement the
change in the default folder.
Comment 18 David Strupl 2002-11-05 09:11:58 UTC
Ok - since the current state hopefully satisfies the needs I am
closing this as fixed. Thanks Jarda.
Comment 19 Martin Matula 2003-07-08 14:34:24 UTC
It works - thanks.