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.
In some cases it is desirable for an SPI-defining module to require that some impl of an interface be available (in cases where the interface module is otherwise useless). The module cannot use OpenIDE-Module-Requires, since that would produce a cyclic dependency. It can be documented that all clients which use the SPI individually require the impl, but this is somewhat cumbersome in practice. Suggest some variant of OIDE-M-R which still requires >=1 provider to be enabled, but which does not count as a dependency for purposes of cycle detection nor topological ordering. TBD whether this intuitive notion can actually be formalized and implemented safely. For example, if A requires T without ordering, B depends on A and provides T, and C depends on A, and C's ModuleInstall M calls T.getDefault(), it could happen that the install order is A-C-B, in which case M might get a null or uninitialized T impl. One possible solution would be to treat the attr as meaning that any module like C with a dep on A would implicitly require T without needing to declare it; this would make the attr just be syntactic sugar (prevents the author of C from forgetting the OIDE-M-R attr). A different approach altogether is to permit the manifest tag to be dropped, and some method in the module system added, e.g.: public static void ModuleInfo.requireDuring(String token, Runnable r) throws NoProviderFoundException; which would guarantee that at least for the duration of the runnable (and possibly beyond it, at the discretion of the impl), one or more providers of the token be enabled. This could permit impls to be left disabled until called. However the implications of encouraging modules being enabled at random points during NB runtime might be bad (e.g. for performance). And the impl could not provide any kind of GUI or the effect would be very disturbing.
*** Issue 56600 has been marked as a duplicate of this issue. ***
*** Issue 72383 has been marked as a duplicate of this issue. ***
Created attachment 31483 [details] Could not this get a little bit more priority? The patch with ok api, ok test and funny and not working impl
*** Issue 22507 has been marked as a duplicate of this issue. ***
I have a better patch that works fine during enablement. Disabling still needs some work...
Created attachment 31485 [details] Relatively nice patch, needs work on disabling
Let's implement this useful improvement.
Created attachment 31501 [details] Reasonable implementation and removal of Requires for openide/io
Reviewers please review the changes: http://www.netbeans.org/nonav/issues/showattachment.cgi/31501/X.diff
Could also be used for: org.openide.execution.ExecutionEngine org.netbeans.modules.project.uiapi.* Impl looks fine. My main concern is whether we even want this issue to be implemented any more, at least in the case of something like IOProvider. Since there is a default impl of the interface in the module, even code calling IOProvider.getDefault() does not absolutely require an IOProvider in lookup. More to the point, consider a unit test which tests code which contains calls to IOProvider. No problem; either you do not care about testing the output (in which case you do nothing) or you do want to check it (in which case you can simply use MockServices etc.). Such tests will run today and they will run with this patch. But what if you initialize the module system in the test later on, e.g. to load layers? Suddenly the test will choke as the module system tells you that IOProvider was not found. Not sure how to solve this. The issue is really that OpenIDE-Module: org.openide.io OpenIDE-Module-Needs: org.openide.windows.IOProvider is a recommendation that the app should contain some impl of this interface, but no code will actually fail without it, while OpenIDE-Module: org.netbeans.modules.projectuiapi/1 OpenIDE-Module-Needs: org.netbeans.modules.project.uiapi.ActionsFactory really states that some impl of that interface has to be in lookup or there will be assertion errors. The latter case is handled well by your impl. Perhaps the former should be written as OpenIDE-Module: org.openide.io OpenIDE-Module-Recommends: org.openide.windows.IOProvider meaning "if you can find a provider of this token, please enable it, but if you can't, don't worry about it". Alternately, perhaps the problem is limited to dummy implementations of interfaces which are only of interest to unit tests (and we really want modules in a live app to require an output window impl). If this is the case, then perhaps we could change the impl so that no error is reported in case a token cannot be found which is mentioned in the OIDE-M-Needs of a fixed (classpath) module. This would mean that just adding org-openide-io.jar to your classpath would never trigger an error, even if you loaded the module system; but if loaded as a real autoload module it would force enablement of some OW impl.
Created attachment 31832 [details] I like the Recommends approach, it can be generally useful
The previous patch implements Jesse's suggestions. If there are no strong objections I'll integrate it early next week.
In + boolean foundOne = false; + Set<Module> possibleModules = new HashSet<Module>(); + for (Module other : providers) { + if (other.isEnabled()) { + foundOne = true; + break LOOP; + } + } did you really mean to break to LOOP? Skipping over other NeedsCheck's? The control flow in the whole LOOP block is rather convoluted and I'm not sure I understand it. Some comments would be nice. Probably needs more thorough tests, I'm guessing; this kind of logic is easy to get wrong in less common cases. E.g. what happens when a module requires one token, needs another, and recommends a third. (Some day core parts of ModuleManager need to be rewritten to use some kind of mathematically sound algorithm.) BTW I don't recommend creating new JARs in the data dir for new tests for ModuleManager. Haven't gotten around to rewriting existing tests yet, but I would much prefer all test module JARs to be created in code on demand. Would make it much easier to add new tests to check subtle variations of different situations. Please file a blocking issue for apisupport/project to have it somehow deal with the new manifest keywords. I'm not really sure what the UI should look like, though. Any particular reason why OpenIDE-Module-Specification-Version is incremented in openide/io/manifest.mf but not in openide/execution/manifest.mf? And why openide/io/api/doc/changes/changes.xml is patched but not openide/execution/api/doc/changes/changes.xml? Please file a dependent issue in projects/code to have projectuiapi/manifest.mf specify OpenIDE-Module-Needs: org.netbeans.modules.project.uiapi.ActionsFactory, org.netbeans.modules.project.uiapi.OpenProjectsTrampoline, org.netbeans.modules.project.uiapi.ProjectChooserFactory and also remove from java/project/manifest.mf OpenIDE-Module-Requires: org.netbeans.modules.project.uiapi.ActionsFactory Or just do that as part of your commit, both as a demonstration of this new tag and a sanity check that it is working in a complex situation. There are probably other prov/req tokens that could better use one of the new tags but we'll find those as we go. Overall, looks good.
Created attachment 31957 [details] Patch ready for integration
I have added another test for chained recommends. That identified the problem with the "break LOOP" and also forced me to fix clearCache() method as well. The apisupport has not issue 80475. Version numbers in openide/io and openide/exeuction has been incremented. Changes written. I usedam suing the "needs" in projects/code and removed that token from java/project/manifest.mf I'd like to integrate tomorrow.
Created attachment 31980 [details] Commit message
Implemented.