Bugzilla – Full Text Bug Listing
| Summary: | [Gogo] Server console in Eclipse doesn't work in the same way it did with milestone 5 | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] Virgo | Reporter: | Frieder Heugel <frieder.heugel> | ||||||
| Component: | runtime | Assignee: | Lazar Kirchev <l.kirchev> | ||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | glyn.normington, l.kirchev, leo.dos.santos, milesparker, mlippert | ||||||
| Version: | unspecified | ||||||||
| Target Milestone: | 3.5.1.RELEASE | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows 7 | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 351284 | ||||||||
| Attachments: |
|
||||||||
|
Description
Frieder Heugel
Would be nice to have for 3.0, but we are running out of schedule and there is a workaround of using a telnet session to run a shell. Some more details for this issue. Actually, the commands registered by the default Equinox console are missing. The reason is that after introducing the new shell we disabled the default one. The new shell itself registers these commands, but the Virgo tooling uses its own implementation of CommandInterpreter (which actually copies the one from the default equinox console) - org.eclipse.virgo.ide.management.remote.ServerCommandInterpreter in org.eclipse.virgo.ide.management.remote project, and it is used by org.eclipse.virgo.ide.management.remote.StandardBundleAdmin in the same project. This could be fixed in two ways - either the custom CommandInterpreter is removed and a ConsoleSession is used to issue commands (this is the way things should be done with the old equinox console, but the new one also supports it), or the new Gogo API with CommandProcessor and CommandSession should be adopted. Hi Martin, Could you please comment on this bug? We should decide what is the best way for the tooling to adopt the new shell. Hi Martin, As agreed, I checked the code and I suggest the following approach: The project org.eclipse.virgo.ide.management.remote adopts the new Gogo API by using the CommandProcessor and CommandSession instead of the old CommandInterpreter API. For this the class org.eclipse.virgo.ide.management.remote.ServerCommandInterpreter should be completely removed. The org.eclipse.virgo.ide.management.remote.StandardBundleAdmin will lookup CommandProcessor (instead of CommandProvider services), and will create CommandSession from the CommandProcessor. Then StandardBundleAdmin.execute will execute the passed command line by calling CommandSession.execute(cmdLine) instead of ServerCommandInterpreter.exdecute(...). I see that ServerCommandInterpreter.nextArgument() currently removes any quotation marks from the arguments. If the presence of quotation marks in the command line is a real scenario, then they should be removed before callingCommandSession.execute(cmdLine). Martin, I could provide a patch for this, or you could contact me to further discuss the solution when you start refactoring the tooling. Just indicate what you prefer. Miles, I CC you in this bug to be aware of our discussions. Hi Lazar, Sorry to miss this bug. Yes, your solution sounds great. If you can submit a patch for it we'll try to get it in asap. cheers, Miles (In reply to comment #6) > Hi Lazar, > > Sorry to miss this bug. Yes, your solution sounds great. If you can submit a > patch for it we'll try to get it in asap. > > cheers, > > Miles Thanks Miles, I will take care to prepare a patch in the next one or two weeks and attach it here. Regards, Lazar We've decided to hold 3.5.0. for this, since we haven't pushed the release yet anyway. Let us know if you run into any issues that we can collaborate on. Moving to tooling as that is where the code will be. Lazar, do you think you might be able to accomplish this w/in the next week or so? *** Bug 379302 has been marked as a duplicate of this bug. *** Created attachment 218243 [details]
Proposed patch
Patch implementing the proposed solution.
(In reply to comment #11) > Created attachment 218243 [details] > Proposed patch > > Patch implementing the proposed solution. How could I test this locally? I tried to install the tooling from the update site, built locally in the ide project, but the installation fails because of unsatisfied dependencies to virgo kernel. Hi Miles, I think that the proposed patch will not be enough to adopt the new console. I used to think that this StandardBundleAdmin MBean is the connection between the Tooling console and the actual server, but it seems that this was wrong assumption. Do you know where exactly this connection is made? I cannot find it, and I think that this is the place which should be reworked. However, the attached patch also is relevant for the new version of Virgo. I found why the console currently is not working. Actually after all of the changes Miles made it should be working. But in config.ini we have the property gosh.args=--nointeractive, which makes the console not open a session on System.in and System.out, and to listen only for remote connections. If this property is removed and the server started, the console appears. However, its output is not in the Console View, but in the log file, because Virgo redirects the System.out to the log file. Does anybody know how the Eclipse Server Console in the tooling worked with Virgo versions prior to 2.1.1? There the output was still redirected to the log file. Or probably I am looking the wrong Console View?
> Virgo versions prior to 2.1.1? There the output was still redirected to the log
"prior to 2.1.1" should be read "prior to 3.0"
(In reply to comment #12) > (In reply to comment #11) > > Created attachment 218243 [details] > > Proposed patch > > > > Patch implementing the proposed solution. Thanks! I'll take a look now. > > How could I test this locally? I tried to install the tooling from the update > site, built locally in the ide project, but the installation fails because of > unsatisfied dependencies to virgo kernel. Yes, I've fond that the target platform setup is not easy. We have some recent traffic on virgo-dev regarding this. The way I've made it work is to have all of the major dependencies -- including AspectJ and M2e tools installed into your Eclipse platform. Then I put some of the the other bits that I don't want in my Eclipse environment, plus the bits that don't like to live with each other :) into the target platform. You can try: org.eclipse.virgo.ide/SuggestedTargetPlatform.target But no promises! :) (In reply to comment #14) > I found why the console currently is not working. Actually after all of the > changes Miles made it should be working. But in config.ini we have the property > gosh.args=--nointeractive, which makes the console not open a session on > System.in and System.out, and to listen only for remote connections. If this > property is removed and the server started, the console appears. However, its > output is not in the Console View, but in the log file, because Virgo redirects > the System.out to the log file. Argh. Virgo runtime does that, or do you think tooling is? Libra is actually contributing the server console page, so something could be happening there as well. I'll take a look. > > Does anybody know how the Eclipse Server Console in the tooling worked with > Virgo versions prior to 2.1.1? There the output was still redirected to the log > file. Or probably I am looking the wrong Console View? I don't know, its all terribly confusing. What would make me really (not) happy is to discover that some of the crap logging tools that Eclipse seems to be accumulating are stealing this, but that's probably just paranoia on my part. Oh, to be clear..I thin there is a major source of confusion here. The (standard) Console View has always worked properly (hopefully it still does :D) -- what we're referring to hear is the Server Console page. If you double-click on a server and open the editor, that's the last page. It's an *interactive* console. (We have a bug 374628 for a nicer solution to that.) (In reply to comment #18) > Oh, to be clear..I thin there is a major source of confusion here. The > (standard) Console View has always worked properly (hopefully it still does :D) > -- what we're referring to hear is the Server Console page. If you double-click > on a server and open the editor, that's the last page. It's an *interactive* > console. > > (We have a bug 374628 for a nicer solution to that.) I am afraid that for Juno the Libra console has some issues - bug 384304. Miles, I have not looked on the Server Console - I mistakenly tried the Console View. Now I am pretty sure that the proposed patch will work for the Server Console - the patch does not work for the standard Console View, but the Server Console is completely different. I still haven't tested it, but I will shortly try your proposition for org.eclipse.virgo.ide/SuggestedTargetPlatform.target. (In reply to comment #20) > Miles, I have not looked on the Server Console - I mistakenly tried the Console > View. Now I am pretty sure that the proposed patch will work for the Server > Console - the patch does not work for the standard Console View, but the Server > Console is completely different. > > I still haven't tested it, but I will shortly try your proposition for > org.eclipse.virgo.ide/SuggestedTargetPlatform.target. I found a way to try the patch and it is not working. I am investigating why. And one more question arises - for which region of the server should the console work? (In reply to comment #22) > And one more question arises - for which region of the server should the > console work? Obviously the user region. (In reply to comment #21) > I found a way to try the patch and it is not working. I am investigating why. It seems the problem is not in the patch, but in the way I tried to test it - I tried to replace the org.eclipse.virgo.ide.management.remote bundle in the ready installation of Eclipse + Virgo tooling, but this approach seems to not work. I will find a better way to try it. (In reply to comment #24) > (In reply to comment #21) > > > I found a way to try the patch and it is not working. I am investigating why. > > It seems the problem is not in the patch, but in the way I tried to test it - I > tried to replace the org.eclipse.virgo.ide.management.remote bundle in the > ready installation of Eclipse + Virgo tooling, but this approach seems to not > work. I will find a better way to try it. Here's what you need to do there: 1. Make the changes in org.eclipse.virgo.ide.management.remote. This will require switching a different target platform that has the correct dependencies. That should be very simple, but I don't have one pre-made yet, sorry. 2. Export the plugin to o.e.virgo.ide.runtime.core. 3. Change org.eclipse.virgo.ide.runtime.internal.core.runtimes.Virgo35Provider#GEMINI_CONNECTOR_BUNDLE_NAME to point to the new bundle location. 4. Launch Eclipse runtime and start a server from there to get the bundle installed into Virgo. Lazar, Also, please see http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions for the new way of handling patches. You'll need to open a fork on github, which you'll probably want anyway. cheers, Miles (In reply to comment #26) > Lazar, > > Also, please see > http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions for > the new way of handling patches. You'll need to open a fork on github, which > you'll probably want anyway. > > cheers, > > Miles Miles, is this really necessary? I am a committer on the Virgo project. Regards, Lazar (In reply to comment #27) > Miles, is this really necessary? I am a committer on the Virgo project. > > Regards, > Lazar LOL. I'm sorry Lazar. Kind of out of it this am, I think I assumed because of the proposed patch. Actually you should just go ahead an commit. Ideally we'd run it through Gerrit, but I'm not really set up for that. Just let me know when you do that. cheers, Miles (In reply to comment #28) > (In reply to comment #27) > > Miles, is this really necessary? I am a committer on the Virgo project. > > > > Regards, > > Lazar > > LOL. I'm sorry Lazar. Kind of out of it this am, I think I assumed because of > the proposed patch. Actually you should just go ahead an commit. Ideally we'd > run it through Gerrit, but I'm not really set up for that. Just let me know > when you do that. > > cheers, > > Miles I proposed a patch so that you could have a look on it :) I didn't know if Virgo Tooling is set up to work with Gerrit - Virgo itself isn't. I'll let you know when I have verified the patch is working. Regards, Lazar Created attachment 218420 [details]
Proposed patch
I tested the patch and it is working. I had to make e minor change in its MANIFEST.MF though, so a attached the fixed patch. You only will need probably to add Apache Felix Gogo Runtime to the target platform. It is available in the Orbit update site.
if you are fine with the patch, I can submit it.
Lazar, since we don't have to worry about IP approval, just open a git branch using this bug as label. thanks! Miles (In reply to comment #31) > Lazar, since we don't have to worry about IP approval, just open a git branch > using this bug as label. > > thanks! > > Miles Change pushed to branch Bug351328_ServerConsole Lazar Miles, Is there a reason why org.eclipse.virgo.management.remote subproject is not included as a module in the pom of org.eclipse.virgo.ide and respectvily is not built with all other subprojects? Regards, Lazar (In reply to comment #33) > Is there a reason why org.eclipse.virgo.management.remote subproject is not > included as a module in the pom of org.eclipse.virgo.ide and respectvily is not > built with all other subprojects? Yep, a very good one. :) Its one that took Leo and I quite a bit of puzzling to figure out. Turns out that it needs to be built against a totally different set of dependencies as its deployed to the server. It doesn't work to have a shared TP either because there are actually conflicting dependencies. So we really need a separate TP for it. Also process wise we can't have it part of integration build because it needs to actually be pre-built as a jar and then injected by the core.manifest plugin. btw, thanks for your work getting this out. After making it such a rush, I now have to reverse myself and say that I probably won't get to review it right away. Hopefull later this week, but overall my availability won't be great this month and we're going to push off release until next month. Merged with new remote jar. This one was an epic, but works perfectly now. Thanks Lazar. Closing the bug as fixed. |