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 77489 - jsch cannot depend on ant
Summary: jsch cannot depend on ant
Status: RESOLVED FIXED
Alias: None
Product: ide
Classification: Unclassified
Component: libs (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: Jaroslav Tulach
URL:
Keywords: API_REVIEW_FAST
Depends on:
Blocks:
 
Reported: 2006-06-07 07:19 UTC by Jaroslav Tulach
Modified: 2009-09-02 22:16 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Implementation of Jesse's idea with factory method (11.85 KB, patch)
2006-06-13 17:59 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Tulach 2006-06-07 07:19:16 UTC
jsch is a library and as such it shall not depend on ant module. This has been 
observed during the work on split of ideX cluster into pieces 
(http://openide.netbeans.org/proposals/arch/clusters.html), but in spite of 
that it is generally insane to have library that depends on most of 
implementation in other modules.

The reason why lib/jsch depends on Ant is due to its subclass of 
org.apache.tools.ant.module.spi.AutomaticExtraClasspathProvider
this shall be either moved to another module or there should be some 
declarative API (project/libraries?) to make the dependency "weak".
Comment 1 Jesse Glick 2006-06-09 00:47:17 UTC
Suggest that this dep be moved to some module in the nb* cluster.
Comment 2 Jaroslav Tulach 2006-06-09 08:09:08 UTC
As far as I know the need of adding jsch as an ant library comes from 
mobility. If I realize Jesse's suggestion and register jsch to ant by some 
(new?) module in nbX.Y cluster, will not mobility have to depend on this 
module? 

Btw. why cannot mobility register this library themselves?
Comment 3 Jesse Glick 2006-06-09 17:22:17 UTC
"As far as I know the need of adding jsch as an ant library comes from
mobility." - well I don't know who originally requested it but it is valid to
make such an association independently of any mobility module.

If a dep from mobility to whoever registers this lib is in fact required, it
should be expressed as a token require dep.
Comment 4 Jaroslav Tulach 2006-06-12 07:42:34 UTC
Requesting token from one of nbX.Y modules is as bad as having dep on that 
cluster. So I am afraid this is not the right solution either.

Could not the ant module read something from layers? For example a library 
definition describing the jsch library as "Ant" volume type? Or anything more 
simple - reading nbinst://... from somewhere?

Jesse, if you've been asked to convert the contract between ant and jsch into 
declarative, so modules do not need to depend on each other, what would you 
do?
Comment 5 Jesse Glick 2006-06-12 18:28:32 UTC
A new library type may be a possibility. It seems that we could not add a volume
type to the j2se library type; would have to define a separate library type for
Ant. I'm scared of this option because it means that Ant libs would appear in
the Library Manager; I don't think you can hide them. New users might be
confused about why there are "Class Libraries", which everyone needs to see, vs.
"Ant Libraries", which almost no one needs to see. This is a UI change and not
one we really want. For consistency we would need to then also remove the
Classpath section from Ant options and use Lib Mgr to define Ant libs instead.

A new declarative SPI using nbinst URLs would be possible of course, but
requires a new layer area, a new API review, probably a new style of format
since we have no precedent that I know of for registering bare URLs.

Or we could make a new static method in Ant like:

public class AutomaticExtraClasspathProviderFactory {
  public static AutomaticExtraClasspathProvider create(Map config);
}

which would read a URL (or several??) from the config file, which could then be
a Services/**/*.instance. A bit ugly, I think, but could work. You could use
instanceOf to ensure that the instance was ignored if the Ant module was not
present.

Or we could simply split off the dep into a separate eager module, which could
be put into whichever cluster you prefer. Of course it means a new module which
we might want to avoid.

Or the registration could be moved into the mobility module which needs it, and
just assume that few non-J2ME users of the IDE needed that lib registered anyway
(they can always add it themselves in the Options dialog).
Comment 6 Jaroslav Tulach 2006-06-13 17:59:00 UTC
Created attachment 31008 [details]
Implementation of Jesse's idea with factory method
Comment 7 Jaroslav Tulach 2006-06-13 18:02:53 UTC
I really like Jesse's suggestion to create the factory method. I know it 
depends on semi-deprecated Services/ folder, but at least it shows what is the 
functionality that is missing in metaInfServices...

I've attached the patch that solves the problem and seems to work fine. The 
mobility team shall probably depends on org.apache.ant.tools > 3.18 as well as 
the lib.jsch lib...
Comment 8 Jesse Glick 2006-06-13 18:19:00 UTC
Looks OK to me. Minor issues:


nbinst URL protocol should specify a module name as the "host".
(libs/jsch/src/org/netbeans/libs/jsch/mf-layer.xml,
ant/src/org/apache/tools/ant/module/spi/AutomaticExtraClasspathProvider.java)


Obviously ide/golden/deps.txt will need a patch.


You could consider making the same decl registration from the junit module,
though that needs to depend on the Ant module anyway.


In url method, IOException should probably be IllegalArgumentException.


AutomaticExtraClasspath should find the File for the URL in the url method and
throw IAE if it fails, rather than silently producing nothing.


In actual layer, instanceClass attr is redundant.


In sample layer in Javadoc, instanceOf attr should be specified.


AntLibrary.org.netbeans.lib.jsch.instance could be renamed for stylistic reasons
to e.g. org-netbeans-lib-jsch-antlibrary.instance.


s/2005/2006


Spec version in apichanges.xml looks wrong. 3.25, right?
Comment 9 Jaroslav Tulach 2006-06-19 07:35:15 UTC
I'll update the patch according to Jesse's comments and integrate tomorrow.
Comment 10 Jesse Glick 2006-06-20 22:47:14 UTC
Looks to be FIXED, right?

Still outstanding (not sure if you disagree or just forgot):

"nbinst URL protocol should specify a module name..."

Also minor comments:

IllegalStateException is a bit weird for bad args; prefer IllegalArgumentException.

AutomaticExtraClasspath can be simplified to just keep the File as an instance
field, rather than the URL.
Comment 11 Jaroslav Tulach 2006-06-21 06:53:04 UTC
It is fixed, just my changes to the issue got lost. Only non-fixed things are:

"nbinst URL protocol should specify a module name as the host" - I do not know 
what it means, the only use of nbinst I found in java and it does the same 
thing as my code. If you know what you want, change it.

"junit changes" - not done, out of scope.

"instanceClass" - kept in layer.

Comment 12 Jesse Glick 2006-06-21 18:47:45 UTC
Never mind, I will fix it myself.
Comment 13 _ tboudreau 2007-11-15 11:21:43 UTC
Note this solution is pretty undebuggable if you include the CVS modules in your platform app, but do not know you should also include the jsch library.  I 
just had some correspondence with someone using the platform who had this problem - something triggered an attempt to load the classes (even without the 
Ant modules installed), and they were not there, which cause the app not to start.

Why not move registration of the Ant task to an eager module that is only enabled if mobility and ant are present?
Comment 14 Jesse Glick 2007-11-15 15:17:36 UTC
Tim - I don't know what you are talking about. If you are seeing some exception, please file a fresh bug with *exact*
steps to reproduce and someone will look at it. Your problem may or may not be related to this issue, so instead of
proposing solutions please describe the problem enough that it can be evaluated.