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: | The implementation dependencies removal from xml.tools.java | ||
---|---|---|---|
Product: | xml | Reporter: | pgebauer <pgebauer> |
Component: | Tools | Assignee: | Samaresh Panda <samaresh> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | apireviews, sustaining |
Priority: | P2 | ||
Version: | 6.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Attachments: |
diff
new diff new diff with --git |
Description
pgebauer
2008-09-12 14:34:57 UTC
We could add org.netbeans.modules.xml.tools.java as a friend to org.netbeans.modules.xml.tools and org.netbeans.modules.xml.tools. However, it is going to be really hard to make this as a friend to tax. xml.tools.java depends heavily on org-netbeans-tax.jar which is a library and is kept under ide/modules/ext. And I'm not sure how to have intra cluster library dependency. AFAIK, we're supposed to stay away from tax and since no code changes are happening on tax, updating org.netbeans.modules.xml.tools.java via AUC shouldn't cause any issue because of tax. Is it OK if we keep impl dependency on tax and fix the other two? Added apireviews@netbeans.org to the CC. Please do comment and suggest any other better approaches to solving this issue. There should be no special problem with exporting friend packages from xml.tax. It does not matter whether the packages are implemented in the main module sources vs. in a Class-Path extension library. You would just then use a regular module dependency xml.tools.java -> xml.tax. (And apisupport.project could also use a regular dependency - would need to also be listed as a friend.) OK, let me try that. If it works, I'll send the diff patch for review. I made the necessary changes but neither xml.tools.java nor apisupport.project build and I'm not sure why. Here is the friend declaration in xml.tax: <friend-packages> <friend>org.netbeans.modules.apisupport.project</friend> <friend>org.netbeans.modules.xml.tools.java</friend> <package>org.netbeans.xml.tax</package> <package>org.netbeans.xml.tax.decl</package> <package>org.netbeans.modules.xml.tax.cookies</package> <package>org.netbeans.modules.xml.tax.parser</package> </friend-packages> And the projects now have regular (spec) dependency. Is it because xml.tax is in ide cluster and the dependent modules are in java and apisupport cluster :(.? The cluster should not matter. As a matter of discipline we discourage inter-cluster friend dependencies, but they are certainly better than implementation dependencies. What do you mean by "do not build" specifically? Did you remember to rebuild xml.tax first? Does "distilling public package JAR" appear when you first built the new xml.tax, and does it have the correct contents (*.class from the named packages)? Also you can remove the existing implementation version from xml.tax after updating apisupport.project, xml, xml.tools, and xml.tools.java (no one should be using it any more). Cf. http://deadlock.netbeans.org/hudson/job/trunk/lastSuccessfulBuild/artifact/nbbuild/build/generated/impl-deps.txt Make sure to increase the spec version of xml.tax (use the new version in the new deps) and increase the spec versions of the modules depending on it. An example of a complete impl -> friend migration for reference: http://hg.netbeans.org/core-main/rev/a62f9bf68716 Of course I make a clean build of xml.tax fist. One thing I noted is that this jar doesn't appear in nbbuild/build/public-package-jars/. Implies I do NOT see "Distilling...". Shouldn't it go there? Perhaps this is the issue, but I do not know the fix. There are few other unusual things about this module, e.g. these two lines from manifest: OpenIDE-Module-Install: org/netbeans/modules/xml/tax/TAXModuleInstall.class OpenIDE-Module-Requires: org.openide.modules.InstalledFileLocator I suppose it doesn't matter, but I'm not sure. nbbuild/build/public-package-jars/org-netbeans-modules-xml-tax.jar should be generated ("Distilling...") the first time any other module is built which requests a spec version dep on xml.tax (i.e. one of its new friends). xml.tax has an impl dependency on xml support. Do you think that could be the reason why it doesn't go into public-package-jars? Other than that I have no idea why the jar doesn't appear there. Seems like the build is working, I'm not sure what was wrong. Let me see if the build works. Please see the attached diff. I'll push the changes after review. Created attachment 70123 [details]
diff
[JG01] For apisupport.project, you are deleting the last impl dep, so spec.version.base can be deleted and you can switch to OpenIDE-Module-Specification-Version: 1.24 in manifest.mf. For background, see http://wiki.netbeans.org/DevFaqImplementationDependency It seems that you did this reliably in all the other modules, so maybe you just forgot in this case. [JG02] Did you mean to make xml.tools autoload as part of this patch, and delete AutoUpdate-Show-In-Client: false from xml/manifest.mf? [Re:JG01] I may have overlooked. [Re:JG02] It was not intended. There are couple of things I need to comment. [SP01] As a result of this change, xml support module now exposes almost all packages to friends. My experience has been that, the friends list keeps growing and at some point, folks want to make those APIs public. Perhaps we should expose only that is required by the friends and hide everything else inside impl packages. Yes/No? If Yes, I'll work on it. In fact I did some work to some extent and my next comment is based on that. [SP02] AFAIK, the sync packages in xml is obsolete and should be removed. None of the code that I have ever worked with, uses these. The only place these classes are being used is in xml.tax and I do not know much about that. Any ideas?? IMO, we should clean this up, but I would like to discuss that in detail. Things like, if anybody knows the purpose of these classes? How safe it is to clean this up? Cleaning up, sync may not be such a good idea. So, I'll make only the following changes: 1) Allow xml.tools to have friends : [xml.tools.java] 2) Allow xml.tax to have friends : [xml.tools, xml.tools.java, apisupport.project] 3) Allow xml to have friends : [xml.tools, xml.tools.java, xsl, xslt.core, xml.schema, xml.tax] 4) Refactor xml support's implementation classes and hide them. #1 was originally not mentioned in the bug, but we should fix this as well. #4 is same as [SP01]. We could cleanup sync package later. However, I've refactored and exposed those classes, which are ONLY required by friends. Please see the new diff. Created attachment 70369 [details]
new diff
Always use --git when creating a diff that involves file renames; otherwise the patch is unreadable. Probably you want to just add to your ~/.hgrc: [diff] git=1 Pl. see new diff with --git option. Created attachment 70393 [details]
new diff with --git
Can I go ahead and commit this change? Looks reasonable to me from a general perspective, though I am not qualified to comment on the internals of the XML modules. Fix integrated: http://hg.netbeans.org/main/rev/9dcf96a32c32 http://hg.netbeans.org/main/rev/2f4b51caa28c http://hg.netbeans.org/main/rev/943074767a08 http://hg.netbeans.org/main/rev/d7dc311825bb For some reason, IDE kept missing files during commit. As a result there are four changesets. Integrated into 'main-golden', will be available in build *200809270201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/9dcf96a32c32 User: samaresh@netbeans.org Log: #147096 Remove implementation dependencies of various xml modules. *** Issue 77492 has been marked as a duplicate of this issue. *** |