Bug 271627 - An xsl.jaxp feature
Summary: An xsl.jaxp feature
Status: ASSIGNED
Alias: None
Product: WTP Source Editing
Classification: WebTools
Component: wst.xsl (show other bugs)
Version: 3.2   Edit
Hardware: All All
: P2 enhancement (vote)
Target Milestone: Future   Edit
Assignee: David Carver CLA
QA Contact: David Carver CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 313793
  Show dependency tree
 
Reported: 2009-04-08 10:05 EDT by Doug CLA
Modified: 2012-04-25 20:46 EDT (History)
4 users (show)

See Also:
thatnitind: review+


Attachments
New proposed features (7.49 KB, application/octet-stream)
2011-05-12 14:47 EDT, David Carver CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Doug CLA 2009-04-08 10:05:50 EDT
I think we should separate out the JAXP specific plugins (which currently depend on the JDT) from those that aren't JAXP (and don't require the JDT) by giving them their own feature. This gives us the option of packaging the non-JAXP plugins separately, without requiring the JDT.
Comment 1 David Carver CLA 2009-04-08 10:24:52 EDT
I agree that we probably should do this.  However, I'm not sure this is a 3.1M7 item.  As this is supposed to be for bug fixes and performance enhancements.   I'll add David Williams on this for comment.

I think it is definitely something we should do during the 3.2 time frame.

David comments, thoughts?
Comment 2 David Williams CLA 2009-04-08 10:43:56 EDT
Yes, good idea, but not the right time. 

One we (or, I :) ) get the "xml stand-alone" feature/package, I think you'll only require two JDT plugins anyway. Not the whole JDT. 

Comment 3 David Carver CLA 2009-04-08 11:05:14 EDT
(In reply to comment #2)
> Yes, good idea, but not the right time. 

I'm retargetting the feature for 3.2 (after galileo).  

> One we (or, I :) ) get the "xml stand-alone" feature/package, I think you'll
> only require two JDT plugins anyway. Not the whole JDT. 

Actually, the way the JDT is tied together, we'll end up pulling the JDT.  There is an open bug/enhancement request to allow the launching/debugging runtime to not require some of the jdt.ui items.   As it stands now, since launching/debugging depends on JDT ui, we'll get the full JDT even though we only use a couple of plugins from JDT.

Aren't dependencies a grand thing? :)


Comment 4 David Carver CLA 2009-04-30 11:39:15 EDT
retargeting as an enhancement.
Comment 5 David Carver CLA 2009-08-22 23:24:33 EDT
will work on this along with bug 272790.
Comment 6 David Carver CLA 2009-09-02 15:24:20 EDT
Assigning inbox items to triaged since these have all be triaged.
Comment 7 David Carver CLA 2009-10-18 09:56:39 EDT
retargetting for 3.2M4 due to work load and time constraints.
Comment 8 Martin Oberhuber CLA 2011-05-03 10:29:09 EDT
(In reply to comment #3)
Looking at Helios SR2, I don't quite see how JDT (or even "two JDT plugins") would be dragged in by plain XML / XSL? I have installed the
    eclipse-php-helios-sr2 package and the
    eclipse-javascript-helios-sr2 package
and then installed PDE to open the "plug-in dependencies view" and see who's dragging in jdt.core / jdt.ui / jdt.debug / jdt.debug.ui. I only found the 3 JAXP plugins dragging in JDT.

Separating out JAXP is important for us building a simple Web Development package including PHP (PDT) and Javascript support. We want XSL Editing, and ideally XSL Debug, but no JDT.

Doing our own feature isn't what we want since then we need to patch up the feature toolchain including PDT (which includes XSL).

Splitting the 3 JAXP plugins into a feature of their own which in turn "requirs" the XSL feature seems easy enough to do. Is there a remote chance this could be done for Indigo?
Comment 9 David Carver CLA 2011-05-03 14:18:14 EDT
I'll target for 3.3RC1, but I won't have time to get to this probably for a few more days.  Swamped with other projects now.  Patches are welcome though and will help more things along quicker.
Comment 10 Martin Oberhuber CLA 2011-05-06 08:03:47 EDT
I thought a bit about this, and here is a suggestion with the tree below denoting "feature includes":

o.e.wst.xsl_sdk.feature          (just like today)
  o.e.wst.xsl.feature            (just like today to ensure "update" gets same)
    o.e.wst.xsl_jaxp.feature     (JAXP debug, "requires" xsl_dev and JDT)
    o.e.wst.xsl_dev.feature      (XSL debug, "requires" xsl_edit I'd assume)
    o.e.wst.xsl_edit.feature     (XSL edit as per bug 313793#c4, "req" xpath2)
    o.e.wst.xml.xpath2.processor.feature

So the idea is that the existing XSL features remain unchanged in content, they continue to "include" respective bundles/features as children. This is to ensure that a Helios user who "updates" the existing wst.xsl feature still gets the same content he had before.

What's new is the 3 child features xsl_jaxp, xsl_dev and xsl_edit. These can be leveraged by product builders to get XSL Tooling on a more fine granular level (Edit only, edit+debug or JAXP debug). Since the goal of these is to support fine-granular updating by product builders, I'd also suggest that these don't "include" their respective dependencies but "require" them only. That way a product builder can update/patch eg xpath2 if required without updating/patching respective parents.

Upstream users (such as PHP, see bug 344772; or Javscript Package, see bug 313793) should then be instructed to only require the minimal featureset they really need.

One thing I was not quite sure about is the userdocs.
- Xpath2 / Psychopath docs seem very verbose, are they for ISV's or users?
- Should XSL Tooling docs belong to the edit, dev or jaxp feature?
- Splitting the docs into separate plugins doesn't seem worthwile to me
- Maybe leverage help filters to ensure that eg JAXP related docs are only
  shown when JAXP is installed? This is relatively easy by coding
  <enablement> expressions into the Doc TOC which enable respective nodes
  in the TOC only when the respective plugin is installed.

I could probably try and help out with a patch next week, when there is general consensus about how to approach the feature refactoring. David W's input from his experience refactoring the Webtools features would be very welcome.
Comment 11 David Carver CLA 2011-05-06 10:43:07 EDT
(In reply to comment #10)
> I thought a bit about this, and here is a suggestion with the tree below
> denoting "feature includes":
> 
> o.e.wst.xsl_sdk.feature          (just like today)
>   o.e.wst.xsl.feature            (just like today to ensure "update" gets same)
>     o.e.wst.xsl_jaxp.feature     (JAXP debug, "requires" xsl_dev and JDT)
>     o.e.wst.xsl_dev.feature      (XSL debug, "requires" xsl_edit I'd assume)
>     o.e.wst.xsl_edit.feature     (XSL edit as per bug 313793#c4, "req" xpath2)
>     o.e.wst.xml.xpath2.processor.feature
> 
> So the idea is that the existing XSL features remain unchanged in content, they
> continue to "include" respective bundles/features as children. This is to
> ensure that a Helios user who "updates" the existing wst.xsl feature still gets
> the same content he had before.
> 
> What's new is the 3 child features xsl_jaxp, xsl_dev and xsl_edit. These can be
> leveraged by product builders to get XSL Tooling on a more fine granular level
> (Edit only, edit+debug or JAXP debug). Since the goal of these is to support
> fine-granular updating by product builders, I'd also suggest that these don't
> "include" their respective dependencies but "require" them only. That way a
> product builder can update/patch eg xpath2 if required without
> updating/patching respective parents.
> 
> Upstream users (such as PHP, see bug 344772; or Javscript Package, see bug
> 313793) should then be instructed to only require the minimal featureset they
> really need.


Thanks for doing this Martin.  I'm going to try and get to this this weekend.  If I can't get it for RC1 (I'll need approval to get it in), then I'll try RC2.  The sooner I can get it in the better for EPP packaging to respond to the changes.

> 
> One thing I was not quite sure about is the userdocs.
> - Xpath2 / Psychopath docs seem very verbose, are they for ISV's or users?

There are portions that are for ISV's to use, and there are other portions that describe the XPATH2 functions implemented (there are over 100 functions implemented as part of the specification).  The later is more for users of XPATH2.   We could possibily include the XPath 2 docs from the W3C which describes these in better detail.

In general the documentation needs to be cleaned up as the API described for ISVs is for the old deprecated API and the XPath 2 processor now has a much cleaner and easier to use API that adopters should use.


> - Should XSL Tooling docs belong to the edit, dev or jaxp feature?
> - Splitting the docs into separate plugins doesn't seem worthwile to me
I have these in docbook files, so I can easily split some of this up into other features.  Probably something I'll have to work on for Juno.


> - Maybe leverage help filters to ensure that eg JAXP related docs are only
>   shown when JAXP is installed? This is relatively easy by coding
>   <enablement> expressions into the Doc TOC which enable respective nodes
>   in the TOC only when the respective plugin is installed.
> 
> I could probably try and help out with a patch next week, when there is general
> consensus about how to approach the feature refactoring. David W's input from
> his experience refactoring the Webtools features would be very welcome.

If you can get a patch, I'll give it high priority for a review, and try to get pmc approval.

Dave
Comment 12 David Carver CLA 2011-05-11 09:24:23 EDT
Starting work on this today.
Comment 13 David Carver CLA 2011-05-11 10:29:16 EDT
David, I've created the additional features and testing them during the build.  Question for the WTP build (I have not released these features yet), do I need to add the sourceTemplates to the feature.  These new features do not need individual SDK features created from them.

Martin, you can find a testing update site at:

https://hudson.eclipse.org/hudson/job/cbi-wtp-wst.xsl/ws/sourceediting/development/org.eclipse.wst.xsl.repository/target/site/

Please take a look at this and make sure it meets your needs.

The License information may not be included, because we leave that to the official WTP build which uses the new licensing feature.  I just want to make sure there are no bugs in the installation of the features before asking PMC approval.
Comment 14 David Williams CLA 2011-05-11 10:49:07 EDT
(In reply to comment #13)
> ... do I need
> to add the sourceTemplates to the feature.  These new features do not need
> individual SDK features created from them.
> 

No ... not as far as I know.
Comment 15 David Carver CLA 2011-05-11 17:57:37 EDT
*Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such.

This is a request for the EPP Packages, particularly for the JSDT and PHP packages, that don't want to have full debugging/launching support because of the JDT dependency introduced in the full XSL feature.


*Is there a work-around? If so, why do you believe the work-around is insufficient?

Projects would need to include or construct their own XSL features.  This leads to a fragmentation, and doesn't provide a consistent user experience.  It should be the responsibility of the Source Editing project provide the necessary features for down stream projects to consume.


*How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added?

There is an update site, available for test installation of the features.
https://hudson.eclipse.org/hudson/job/cbi-wtp-wst.xsl/ws/sourceediting/development/org.eclipse.wst.xsl.repository/target/site/


*Give a brief technical overview. Who has reviewed this fix?

Nitin and Nick both have reviewed this addition.

* What is the risk associated with this fix?

Low for the existing XSL project and Source Editing.  The existing XSL feature remains unchanged.  We just create new features.
Comment 16 David Williams CLA 2011-05-12 12:33:20 EDT
Can you provide a patch, or at least a team project set to make the changes easier to review? 

I won't say I object to this change, but do have some concerns, which I'll state here openly just to be honest ... I do not mean them as criticisms or "show stoppers" ... and know there are reasonable answers for each ... but, thought I should say my concerns out loud: 

A. I see this as more an enhancement request, instead of a bug ... and its kind of late for enhancements. 

B. Changing features is normally "frozen" at M6. 

C. Offhand, I wonder how much we in WTP are being expected to fix an issue in JDT (where their coupling is too tight ... and it is their stuff that is "pulling in too much"). 

D. There are countless ways to "slice and dice" features/bundles ... and normally adopters are expected to do it as they like, not to have each project/feature provide scores of possibilities. 

E. Is it reasonable/important to provide XSL Editing without XSL launch and debug? Is there precedent? Can you install Java Editor without launch and debug? Can you install JSP editor without launch and debug? Ant? PHP itself? 

F. At least for JSDT package, I'm not sure it is obvious we'd want to make use of the ability to remove the XSL launch and debug capability ... and certainly not this late in a cycle.

G. How many plugins per feature does this result in? Are we moving to have one feature per bundle? (I can't easily see, see no patch attached). If so, that sort of raises a warning flag for me. 

So, those are my conceptual concerns. I still need to think through the technical implications ... normally its not a good thing to have tightly coupled feature includes. Well, at least not a great thing. 

Is these seen as the "final" fix? It sounds sort of like a "migration step", or something. If it is more a migration step, what is the "final, best fix"? 

I'd like to study the code a bit more before giving any final opinion. 

Overall, though, I definitely like the idea of improving consumabily and loosening coupling. And appreciate Dave and Martin pushing on this. 

Thanks,
Comment 17 David Carver CLA 2011-05-12 14:45:26 EDT
I'm fine if we want to delay this until Juno, just some answers below.

This was a request by the EPP packages for JSDT and PHP so they didn't bring in the 
(In reply to comment #16)
> Can you provide a patch, or at least a team project set to make the changes
> easier to review? 
> 
> I won't say I object to this change, but do have some concerns, which I'll
> state here openly just to be honest ... I do not mean them as criticisms or
> "show stoppers" ... and know there are reasonable answers for each ... but,
> thought I should say my concerns out loud: 
> 
> A. I see this as more an enhancement request, instead of a bug ... and its kind
> of late for enhancements. 
> 
> B. Changing features is normally "frozen" at M6. 
> 
> C. Offhand, I wonder how much we in WTP are being expected to fix an issue in
> JDT (where their coupling is too tight ... and it is their stuff that is
> "pulling in too much"). 

I've opened a bug against JDT to address this but nothing has ever been done about it.  I opened that about 3 years ago, and at this point I'm not holding out much hope that they will do the necessary refactoring to make it right.



> 
> D. There are countless ways to "slice and dice" features/bundles ... and
> normally adopters are expected to do it as they like, not to have each
> project/feature provide scores of possibilities. 

The request came from upstream eclipse projects, so isn't coming from an outside adopter.


> 
> E. Is it reasonable/important to provide XSL Editing without XSL launch and
> debug? Is there precedent? Can you install Java Editor without launch and
> debug? Can you install JSP editor without launch and debug? Ant? PHP itself? 

It is necessary in this case, to still provide editing support.  The xsl_debug feature can still be installed so if an adopter wants to implment launching and debugging they still can.  Just the xsl_jaxp feature contains the necessary support for Xalan and Saxon to allow debugging and launching that way.



> 
> F. At least for JSDT package, I'm not sure it is obvious we'd want to make use
> of the ability to remove the XSL launch and debug capability ... and certainly
> not this late in a cycle.

I'm fine with waiting until Juno.   I'll let Martin chime in in regards to his particular needs.


> 
> G. How many plugins per feature does this result in? Are we moving to have one
> feature per bundle? (I can't easily see, see no patch attached). If so, that
> sort of raises a warning flag for me. 

No this isn't one feature per bundle.   I'll attach the feature.xml files to this bug that show how things break down.

> 
> So, those are my conceptual concerns. I still need to think through the
> technical implications ... normally its not a good thing to have tightly
> coupled feature includes. Well, at least not a great thing. 
> 
> Is these seen as the "final" fix? It sounds sort of like a "migration step", or
> something. If it is more a migration step, what is the "final, best fix"? 

The final best fix is for JDT to fix the underlying issue.
Comment 18 David Carver CLA 2011-05-12 14:47:24 EDT
Created attachment 195525 [details]
New proposed features

Here are the new proposed features for review.
Comment 19 David Williams CLA 2011-05-13 20:04:39 EDT
I don't feel comfortable changing features this late, and from what I've read so far doesn't sound like it "blocks" users or adoption ... would just improve it. Let me know if I've mis-read, but otherwise I think we should not change these features. 

And, sort of independently, I think the original problem should be re-examined. 

My first thought was the original bug Dave references (bug 170394 I believe) could be re-examined, and perhaps an Eclipse PMC Member that was interested in improving the situation could politely ask the JDT team ... ? :) 

But, after reading it, I was curious. So I looked/searched the XSL plugins I have loaded in my workspace, saw one code bundle (and one test bundle) that had o.e.jdt.debug and o.e.jdt.debug.ui in the manifest.mf, I removed them ... and was surprised it caused no compile errors! I also searched through their *.java files, and didn't hit anything, so don't think there's any reflection with strings being used? 

Now, I could easily not have the right bundles loaded, or something, but makes me wonder if original issue and assumptions should be re-examined. 

For one thing, the xsl bundle in question _does_ reference jdt.debug.ui in the plugin.xml ... but, that does not mean it needs to be in the manifest.mf (nor in the classpath). I believe that means if someone happens to have jdt.debug.ui installed, the extended xsl debug functionality will work. If they happen to not have it installed, then it would not work -- all through the magic of the Eclipse extension registry. That sounds like exactly what you want. Makes a loose coupling. Adopters or packagers that want the debugging support, can then control what's installed via feature or product definitions. 

Any traction here? Or am I seeing things wrong? I'd probably approve a change to the manifest.mf :)  ... even this late, and even though it might "change behavior" in the JavaScript IDE EPP package, if indeed less JDT stuff was pulled in. 

And, this doesn't address o.e.jdt.launching ... that is used in XSL java code, but a) might pull in less without jdt.debug? and b) makes me think that usage should be reexamined too, if we can get rid of the hard jdt.debug.ui dependency? 

I don't mean to be a "stick in the mud" but seems a feature reorganization is premature.
Comment 20 David Carver CLA 2011-05-14 11:08:09 EDT
Re-targeting for 3.4 to investigate further.
Comment 21 Martin Oberhuber CLA 2011-05-16 03:44:15 EDT
Hm...

I don't know the inner workings of XSL tooling and to what respect it could potentially do JAXP with or without JDT. 

What counts for me as adopter is that I want XSL Tooling for web development without Java. "Roll your own feature" is not a good solution for 2 reasons:
  - It fails to work if I'm 2 levels up (I integrate PHP, PHP requires XSLT).
  - It makes us fork from Eclipse, which causes problems in "check for updates"
    and "install add-on from Eclipse" scenarios.

I also don't see an immediate need for splitting out the "XSL Edit" feature, but the JAXP thing is a very separate ting for consumer communities, so that should be reflected in the feature structure. I really can't see what's so big a problem with introducing a nested feature for JAXP, especially when semantics of the containing feature remains exactly the same. Independent of "what JDT dependency problem there is", it sounds logical for me that JAXP requires Java. But I don't want no JAXP nor Java, I want plain XSLT. So why not make it a separate feature.

I remember that a while back the WTP features were reorganized pretty late in the game - I don't remember how much of disruption and grief that really caused, but I'm very happy with the result (ie we consume webtools without the server_ui feature now, to support web development with PHP but without java based appservers).

There's a good chance that my immediate problem is fixed by bug 344772 - PHP not including the XSLT feature any more at all - so I'm not going to push much more for this. Though given there's much web development without Java, I find it sad that it's impossible to consume XSLT without JAXP/Java.
Comment 22 Martin Oberhuber CLA 2011-05-16 03:59:09 EDT
I think the bottomline is: I see no value in trying to decouple JAXP from Java on a fine granluar base, when it is likely that anybody who does JAXP needs Java as well. To me, it makes more sense to decouple on a coarse granular base (feature level).
Comment 23 David Carver CLA 2011-07-07 09:13:13 EDT
*** Bug 351431 has been marked as a duplicate of this bug. ***
Comment 24 Alex Blewitt CLA 2011-07-07 09:52:09 EDT
JAXP and XSL are two different things though; you can have XSL transformations quite outside of the JAXP space (e.g. FO).

At the very least, decoupling from the UI components makes sense, because then it doesn't appear (to the user) that JDT has been consumed, even if it's there under the covers.

I think these dependencies are being pulled in via the kind of Manifest headers that David was talking about in comment 14, and whilst getting rid of the launching might be an end goal, there are other parts that bring in jdt.launching in the XML package (possibly through the same mechanism).

Cleaning up the manifest sounds like an initial step in the right direction to prevent the XSL package introducing dependencies on the UI components of JDT which would be a good initial fix. Subsequent re-organisation of the features might be desirable.

Can the manifest cleanup be implemented for targetting 3.7.1?
Comment 25 Martin Oberhuber CLA 2011-07-07 10:08:09 EDT
(In reply to comment #24)

> At the very least, decoupling from the UI components makes sense, because then
> it doesn't appear (to the user) that JDT has been consumed, even if it's there
> under the covers.

Unfortunately that's not true, since the non-UI o.e.jdt.launching provides the launchConfigurationTypes extension, which is sufficient to make the launch JDT configuration type visible in the UI -- although not functional. See also
bug 350438 .
Comment 26 Alex Blewitt CLA 2011-07-07 10:12:20 EDT
Those without Java (JDT UI) installed won't be trying to launch a Java application, because they won't have any Java applications to launch. Although a bug, it shouldn't be a requirement to keep JDT UI around just because of a bug elsewhere.