Community
Participate
Working Groups
As mentioned on wtp-dev, it's time to break the circular dependency between sourceediting and jsdt. I believe history indicates jsdt should be below sourcedeiting on the tree (installable just on platform), but whether that's feasible to maintain at this point depends on the discussion this bug generates.
Repo: git clone ssh://${GITUSER}@git.eclipse.org:29418/jsdt/webtools.jsdt.git Plugins that can move to JSDT: org.eclipse.wst.jsdt.web.core org.eclipse.wst.jsdt.web.support.jsp org.eclipse.wst.jsdt.web.ui
I'm not sure there was agreement on how to break the cycle. If the goal was to make the repositories buildable per-repository in a sane order, wouldn't breaking the small dependency from JSDT to SSE (as I said at https://dev.eclipse.org/mhonarc/lists/wtp-dev/msg10393.html) be the simpler, less disruptive move?
New Gerrit change created: https://git.eclipse.org/r/104394
Created attachment 270087 [details] diff update site before/after applying patch After applying https://git.eclipse.org/r/104394 - which disables building these 5 plugins ... bundles/org.eclipse.wst.jsdt.web.core bundles/org.eclipse.wst.jsdt.web.support.jsp bundles/org.eclipse.wst.jsdt.web.ui tests/org.eclipse.wst.jsdt.web.core.tests tests/org.eclipse.wst.jsdt.web.ui.tests ... - you'll end up with a much smaller update site in webtools.sourceediting/site/target/repository/ since I've removed all the feature (and plugin) inclusions for org.eclipse.wst.jsdt.* This will stop sourceediting from depending on jsdt.
Created attachment 270088 [details] diff update site before/after applying patch (part 2)
In addition to the PR linked on this BZ, I also provided this cleanup fix for .gitignores and storage of generated (binary) files in github: https://git.eclipse.org/r/104393 That should mean that when you build you don't suddenly have dirty changes in your workspace (which could accidentally be committed!). This fix also enables us to use the jgit timestamp provider (should we want that in future), as the workspace remains clean during and after a build.
@Nitin, the problem is that we've dropped JSDT model a while ago, and provided WST based validation for validation job and in reconciler instead. JSDT cannot be used as a standalone product since Neon.
Created attachment 270131 [details] patch removing dependency from jsdt on sourceediting (In reply to Victor Rubezhny from comment #7) > @Nitin, the problem is that we've dropped JSDT model a while ago, and > provided WST based validation for validation job and in reconciler instead. That part's fine, good even since it removed a considerable amount of confusion for users dealing with a builder for *.js files and a validator for web pages. I still contend that the dependency story can be changed back to SSE->JSDT->Common and JSDT->Common by reversing some of the changes in the UI plug-in. JavascriptReconcilingStrategy used to handle source validation, but when the SSE source validation was put in, this was broken. By restoring a little bit of what was deleted then, it's possible to drop the SSE dependency and the new classes that were created to contain it*. I'm not certain about some of the AST flags involved, but this change should break the loop in the repository dependencies. Someone with more recent experience using the editor would be better suited to determining if the functionality that should be there, after the JSDT model was scaled back, is still there with this change. I also removed the validator's membership to the org.eclipse.wst.sse.core.structuredModelGroup group. It's only a benefit for content types that SSE supports, keeping the model in memory while all of the applicable validators are run on those files. It serves no purpose for JavaScript files, and makes JS validation slower than it would otherwise be. I'd suggest this part regardless of the rest.
I have to say, I find this discussion slightly silly. Once SSE repos are merged together, a quick analysis shows that jsdt will only actually depend on sse.core and sse.ui, NOT any of the json / xml / xsl / xpath / web stuff also in the sse repo. I think it is 100% reasonable to have jsdt depend on sse.core / sse.ui (ie the core part of sse). It won't bring in so much of wtp at all. It's in fact quite slim. I still believe strongly that moving the 3 jsdt plugins into the jsdt repo, and having jsdt depend on sse.core/sse.ui, is not that problematic at all. These are just core APIs with very limited effect on a user's installation size.
My last comment was a bit misleading. It all depends how we treat the 3 jsdt plugins that live in sse. Those obviously depend on all of sse, including xml, xpath, etc etc. I still don't really know what those 3 plugins do, but they seem to depend on a lot. But the stuff currently in jsdt repo only depends on sse.core and sse.ui. This doesn't seem so objectionable tome.
(In reply to Nitin Dahyabhai from comment #8) > Created attachment 270131 [details] > patch removing dependency from jsdt on sourceediting > > (In reply to Victor Rubezhny from comment #7) > > @Nitin, the problem is that we've dropped JSDT model a while ago, and > > provided WST based validation for validation job and in reconciler instead. > > That part's fine, good even since it removed a considerable amount of > confusion for users dealing with a builder for *.js files and a validator > for web pages. I still contend that the dependency story can be changed back > to SSE->JSDT->Common and JSDT->Common by reversing some of the changes in > the UI plug-in. > @Nitin I'm not arguing against this, but we aren't able to reverse the ui code... > JavascriptReconcilingStrategy used to handle source validation, but when the > SSE source validation was put in, this was broken. By restoring a little bit > of what was deleted then, it's possible to drop the SSE dependency and the > new classes that were created to contain it*. I'm not certain about some of > the AST flags involved, but this change should break the loop in the > repository dependencies. Someone with more recent experience using the > editor would be better suited to determining if the functionality that > should be there, after the JSDT model was scaled back, is still there with > this change. and here is why: The jsdt model is a completely outdated thing nowadays. We cannot use the following: >>>>> start cut >>>>> bundles/org.eclipse.wst.jsdt.ui/src/org/eclipse/wst/jsdt/internal/ui/text/java/JavascriptReconcilingStrategy.java index 1d16bb5..b588dff 100644 [...] int reconcileFlags= IJavaScriptUnit.FORCE_PROBLEM_DETECTION | (ASTProvider.SHARED_AST_STATEMENT_RECOVERY ? IJavaScriptUnit.ENABLE_STATEMENTS_RECOVERY : 0) | (ASTProvider.SHARED_BINDING_RECOVERY ? IJavaScriptUnit.ENABLE_BINDINGS_RECOVERY : 0); ast[0]= unit.reconcile(ASTProvider.SHARED_AST_LEVEL, reconcileFlags, null, fProgressMonitor); >>>>> end cut >>>>> we should use the only: JavaScriptPlugin.getDefault().getASTProvider().getAST(unit, ASTProvider.WAIT_ACTIVE_ONLY, fProgressMonitor); in order to generate ASTs and this doesn't provides/updates the old jsdt-model. (We just had no time enough to cut and clear the obsolete code) Thus I can accept your patch only if it's modified so it doesn't change JavascriptReconcilingStrategy (see the lines mentioned above), which kills all the validation. The way you proposed is a simple one to split the dependencies (with the mentioned exclusion), but doing it this way the validation is to be re-developed from the scratch.
New Gerrit change created: https://git.eclipse.org/r/106236
To build this locally without using -Pintegration profile (in order to find all the dependencies and resolve them by building locally) you need to build these projects in order: * webtools.common * webtools.servertools (after merging the repos) * webtools.javaee (to resolve webtools.javaee/plugins/org.eclipse.jst.common.frameworks) * webtools.releng/org.eclipse.wtp.releng.versionchecker * webtools.releng/tests/org.eclipse.wtp.releng.tests Then you can build webtools.jsdt from its root pom, like this: `mvn clean install -DskipTests -Dwebtools.buildSite=http://download.eclipse.org/webtools/downloads/drops/R3.9.1/R-3.9.1-20170912000144/ -Dtycho.version=1.0.0 -Dtycho-extras.version=1.0.0` If you don't build all the above first, you need the -Pintegration flag to resolve all the above dynamically. +1 for this patch [1], let's get it merged ASAP. [1] https://git.eclipse.org/r/#/c/106236/1
New Gerrit change created: https://git.eclipse.org/r/106239
New Gerrit change created: https://git.eclipse.org/r/106240
Hi Guys: Nitin's patch cleared up the dependency, but it killed as-you-type validation. Nitin's attempt to fix as-you-type was vetoed by Victor as no longer viable, due to the code base changing too much. So Victor's answer is to simply break the circular dependency, but kill as-you-type validation. I did not like this solution. So I set about to fix it. Rather than deleting the 3 files as Nitin and Victor's patches did, I moved them into jsdt.web.ui, the project inside sse with access to sse APIs AND jsdt api's. The last step was to leave a stub / delegating class in jsdt that could load from an extension point. Then, have jsdt.web.ui implement that extension point with the moved implementation. In this way, as-you-type validation is maintained. SSE depends on JSDT. And everything continues working as today. I strongly support these patches. jsdt patch: https://github.com/robstryker/webtools.jsdt/commits/519774 sse patch: https://github.com/robstryker/webtools.sourceediting/commits/519774 Please give these two patches the full consideration. I believe they are the proper solution and nobody should have any real reason to reject them.
New Gerrit change created: https://git.eclipse.org/r/106303
New Gerrit change created: https://git.eclipse.org/r/106304
Gerrit change https://git.eclipse.org/r/106303 was merged to [master]. Commit: http://git.eclipse.org/c/jsdt/webtools.jsdt.git/commit/?id=95b67eb4698d4d6960fb06aab4c3feffc7fd96b5
Gerrit change https://git.eclipse.org/r/106304 was merged to [master]. Commit: http://git.eclipse.org/c/sourceediting/webtools.sourceediting.git/commit/?id=c2f86a6e13d6635cfb5c827989c421f9b400f9e0
New Gerrit change created: https://git.eclipse.org/r/110946
New Gerrit change created: https://git.eclipse.org/r/111757
Gerrit change https://git.eclipse.org/r/111757 was merged to [master]. Commit: http://git.eclipse.org/c/jsdt/webtools.jsdt.git/commit/?id=1bfac40caddf8bb12fde025635c1d1e5c6689c2b
New Gerrit change created: https://git.eclipse.org/r/111934
Gerrit change https://git.eclipse.org/r/111934 was merged to [master]. Commit: http://git.eclipse.org/c/jsdt/webtools.jsdt.git/commit/?id=06f6e63c80906c26c13003de69eb70a0a635b29b
FYI In the R3_9_maintenance branch of releng.agg, we must now use webtools.jsdt from master branch, since: * sse plugin wst.jsdt.web.ui requires * jsdt plugin org.eclipse.wst.jsdt.ui 2.0.301, which is only in master (not R3_9_1a tag). Thus: http://git.eclipse.org/c/webtools/webtools.releng.aggregator.git/commit/?h=R3_9_maintenance (Aside: why is are there jsdt plugins in the sse repo? Naming conventions in WTP are confusing.)
