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 147096 - The implementation dependencies removal from xml.tools.java
Summary: The implementation dependencies removal from xml.tools.java
Status: RESOLVED FIXED
Alias: None
Product: xml
Classification: Unclassified
Component: Tools (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: Samaresh Panda
URL:
Keywords:
: 77492 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-09-12 14:34 UTC by pgebauer
Modified: 2008-12-01 22:34 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
diff (14.59 KB, text/plain)
2008-09-18 22:54 UTC, Samaresh Panda
Details
new diff (560.92 KB, text/plain)
2008-09-23 22:37 UTC, Samaresh Panda
Details
new diff with --git (61.96 KB, text/plain)
2008-09-24 04:11 UTC, Samaresh Panda
Details

Note You need to log in before you can comment on or make changes to this bug.
Description pgebauer 2008-09-12 14:34:57 UTC
The module org.netbeans.modules.xml.tools.java declares implementation dependency on org.netbeans.modules.xml,
org.netbeans.modules.xml.tax and org.netbeans.modules.xml.tools. Since the property AutoUpdate-Show-In-Client also is
set to true for the module xml.tools.java, it is hard to deliver xml.tools.java bugfixes via AUC whose code changes are
in org.netbeans.modules.xml, org.netbeans.modules.xml.tax or org.netbeans.modules.xml.tools. It could be "mission
impossible" in some cases.

Could implementation dependencies be removed (for example through friend packages)?
Comment 1 Samaresh Panda 2008-09-16 15:57:07 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?
Comment 2 Samaresh Panda 2008-09-16 16:02:29 UTC
Added apireviews@netbeans.org to the CC. Please do comment and suggest any other better approaches to solving this issue.
Comment 3 Jesse Glick 2008-09-16 16:25:32 UTC
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.)
Comment 4 Samaresh Panda 2008-09-16 16:49:59 UTC
OK, let me try that. If it works, I'll send the diff patch for review.
Comment 5 Samaresh Panda 2008-09-16 18:20:03 UTC
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 :(.?
Comment 6 Jesse Glick 2008-09-16 18:33:36 UTC
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
Comment 7 Samaresh Panda 2008-09-16 20:31:55 UTC
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.
Comment 8 Jesse Glick 2008-09-17 17:35:39 UTC
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).
Comment 9 Samaresh Panda 2008-09-17 19:38:42 UTC
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.
Comment 10 Samaresh Panda 2008-09-17 22:46:26 UTC
Seems like the build is working, I'm not sure what was wrong. Let me see if the build works.
Comment 11 Samaresh Panda 2008-09-18 22:53:33 UTC
Please see the attached diff. I'll push the changes after review.
Comment 12 Samaresh Panda 2008-09-18 22:54:45 UTC
Created attachment 70123 [details]
diff
Comment 13 Jesse Glick 2008-09-22 23:24:18 UTC
[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?
Comment 14 Samaresh Panda 2008-09-23 04:43:17 UTC
[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?
Comment 15 Samaresh Panda 2008-09-23 22:36:06 UTC
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.
Comment 16 Samaresh Panda 2008-09-23 22:37:54 UTC
Created attachment 70369 [details]
new diff
Comment 17 Jesse Glick 2008-09-23 23:37:20 UTC
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
Comment 18 Samaresh Panda 2008-09-24 04:10:08 UTC
Pl. see new diff with --git option.
Comment 19 Samaresh Panda 2008-09-24 04:11:38 UTC
Created attachment 70393 [details]
new diff with --git
Comment 20 Samaresh Panda 2008-09-24 22:04:28 UTC
Can I go ahead and commit this change?
Comment 21 Jesse Glick 2008-09-25 01:36:13 UTC
Looks reasonable to me from a general perspective, though I am not qualified to comment on the internals of the XML modules.
Comment 22 Samaresh Panda 2008-09-25 17:53:03 UTC
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.
Comment 23 Quality Engineering 2008-09-27 05:34:38 UTC
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.
Comment 24 Samaresh Panda 2008-12-01 22:34:32 UTC
*** Issue 77492 has been marked as a duplicate of this issue. ***