Bugzilla – Full Text Bug Listing
| Summary: | Move remote launch/debug to DSF | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Anna Dushistova <anna.dushistova> | ||||||||||||||||||||
| Component: | cdt-debug-dsf-gdb | Assignee: | Ted Williams <ted> | ||||||||||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Marc Khouzam <marc.khouzam> | ||||||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||||||
| Priority: | P3 | CC: | marc.khouzam, mober.at+eclipse, pawel.1.piech, ted | ||||||||||||||||||||
| Version: | 7.0 | ||||||||||||||||||||||
| Target Milestone: | 8.0 | ||||||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||
| Whiteboard: | |||||||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||||||
| Bug Blocks: | 318214 | ||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||
|
Description
Anna Dushistova
You mean the TargetManagement Remote launch? We already have a remote launch in DSF-GDB but maybe we should align the two Yes, I mean target management remote launch, its name is org.eclipse.cdt.launch.remote. Right now it's based on CDI. I would be happy to move it to DSF. (In reply to comment #2) > Yes, I mean target management remote launch, its name is > org.eclipse.cdt.launch.remote. > Right now it's based on CDI. I would be happy to move it to DSF. That sounds good. I'm just curious to know how that launch is meant to be used compared to our existing remote launch which uses the launchDelegate org.eclipse.cdt.dsf.gdb.launch.remoteCLaunch Mark, the idea behind that launch (except for being an example) is to reuse previously defined connection information for launch/debug. We also allow downloading executable on target. Created attachment 170794 [details]
first draft
Here is my first attempt. Any feedback is appreciated.
What plugins do I have to check out to try this? (e.g., RSE) the patch is to be applied to org.eclipse.cdt.launch.remote plugin. As for RSE, you can install either SDK or Runtime. 3.1 version is fine, no need to take the latest one. Looks very good. First set of comments:
1- Do we want to keep the CDI-based launcher? For our other launch config types, the user can choose CDI, DSF-GDB or EDC. All you need to do is add the 'debug' mode in the launch delegate extension that already points to RemoteRunLaunchDelegate, and then give each of the launch delegate a different name (which should be in plugin.properties)
2- if you decide to do point #1, we also have to set a default to the DSF launch, which you can do like it is done in CDebugCorePlugin.setDefaultLaunchDelegates()
3- if you don't decide to do pont #1, then RemoteRunLaunchDelegate could be cleaned up. I believe everything part of
if (mode.equals(ILaunchManager.DEBUG_MODE)) {
will no longer be called.
4- we must update tabs to match the DSF-GDB options, mostly for the Debugger tab and Main tab.You can look at bug 248593 comment #14 which describes how to handle it for JTag. Note that DSF-GDB no longer shows a debugger combobox if there is only a single choice. This was done in bug 281970 comment #8
5- For the Run launch configuration, we no longer show the debugger tab or source tabs since they didn't make sense for Run. You may want to consider doing the same. (bug 281970 comment #28)
6- when I create a launch config for a remote launch I see an exception. Is it my setup maybe?
!ENTRY org.eclipse.osgi 2 0 2010-06-03 15:51:49.868
!MESSAGE While loading class "org.eclipse.rse.ui.SystemPreferencesManager$ModelChangeListener", thread "Thread[Worker-1,5,main]" timed out waiting (5000ms) for thread "Thread[Worker-7,5,main]" to finish starting bundle "org.eclipse.rse.ui_3.1.0.v200905272300 [1350]". To avoid deadlock, thread "Thread[Worker-1,5,main]" is proceeding but "org.eclipse.rse.ui.SystemPreferencesManager$ModelChangeListener" may not be fully initialized.
!STACK 0
org.osgi.framework.BundleException: State change in progress for bundle "reference:file:/local/lmckhou/eclipse.3.6RC3/plugins/org.eclipse.rse.ui_3.1.0.v200905272300.jar" by thread "Worker-7".
at org.eclipse.osgi.framework.internal.core.AbstractBundle.beginStateChange(AbstractBundle.java:1077)
at org.eclipse.osgi.framework.internal.core.AbstractBundle.start(AbstractBundle.java:282)
at org.eclipse.osgi.framework.util.SecureAction.start(SecureAction.java:417)
at org.eclipse.osgi.internal.loader.BundleLoader.setLazyTrigger(BundleLoader.java:265)
at org.eclipse.core.runtime.internal.adaptor.EclipseLazyStarter.postFindLocalClass(EclipseLazyStarter.java:106)
at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.findLocalClass(ClasspathManager.java:453)
at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.findLocalClass(DefaultClassLoader.java:216)
at org.eclipse.osgi.internal.loader.BundleLoader.findLocalClass(BundleLoader.java:393)
at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:469)
at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:422)
at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:410)
at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.loadClass(DefaultClassLoader.java:107)
at java.lang.ClassLoader.loadClass(Unknown Source)
at java.lang.ClassLoader.loadClassInternal(Unknown Source)
at org.eclipse.rse.ui.SystemPreferencesManager$1.run(SystemPreferencesManager.java:510)
at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
Caused by: org.eclipse.osgi.framework.internal.core.AbstractBundle$BundleStatusException
... 16 more
Root exception:
org.eclipse.osgi.framework.internal.core.AbstractBundle$BundleStatusException
at org.eclipse.osgi.framework.internal.core.AbstractBundle.beginStateChange(AbstractBundle.java:1077)
at org.eclipse.osgi.framework.internal.core.AbstractBundle.start(AbstractBundle.java:282)
at org.eclipse.osgi.framework.util.SecureAction.start(SecureAction.java:417)
at org.eclipse.osgi.internal.loader.BundleLoader.setLazyTrigger(BundleLoader.java:265)
at org.eclipse.core.runtime.internal.adaptor.EclipseLazyStarter.postFindLocalClass(EclipseLazyStarter.java:106)
at org.eclipse.osgi.baseadaptor.loader.ClasspathManager.findLocalClass(ClasspathManager.java:453)
at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.findLocalClass(DefaultClassLoader.java:216)
at org.eclipse.osgi.internal.loader.BundleLoader.findLocalClass(BundleLoader.java:393)
at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:469)
at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:422)
at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:410)
at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.loadClass(DefaultClassLoader.java:107)
at java.lang.ClassLoader.loadClass(Unknown Source)
at java.lang.ClassLoader.loadClassInternal(Unknown Source)
at org.eclipse.rse.ui.SystemPreferencesManager$1.run(SystemPreferencesManager.java:510)
at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
I still need to go through the details of the new FinalLaunchSequence, but the approach is perfect.
Nice work.
Mark, I haven't seen that exception with my configuration. Does it happen every time you create a new config? Or just the first time after you started Eclipse? I think I'll go with having 2 debug launch delegates available for now. In the plugin.xml, we define our own debugger and the debug settings page is contributed as a CDebuggerPage. Does this approach still make sense or should I change the way it's done? (In reply to comment #9) > Mark, I haven't seen that exception with my configuration. Does it happen every > time you create a new config? Or just the first time after you started Eclipse? I'll try it when I get to the office. > I think I'll go with having 2 debug launch delegates available for now. > In the plugin.xml, we define our own debugger and the debug settings page is > contributed as a CDebuggerPage. Does this approach still make sense or should I > change the way it's done? That will have to be changed. DSF-GDB does not make use of those debugger extensions. I think you'll need your own launch delegate. (In reply to comment #9) > Mark, I haven't seen that exception with my configuration. Does it happen every > time you create a new config? Or just the first time after you started Eclipse? This happens only once, when I select an existing launch config for this remote launch, or when I create the first one. I think it must be because the RSE plugins are being activated. It could be my setup... Created attachment 171806 [details]
corrected patch
I think this patch contains fixes for the comments Marc made. There is one problem with this patch though. If I use DSF debug and then switch to the CDI debug, I'm getting the "Connection timeout" MI exception. I'm not sure yet why this is happening...
(In reply to comment #12) > Created an attachment (id=171806) [details] > corrected patch > > I think this patch contains fixes for the comments Marc made. Looks good. A couple comments on the new code: plugin.xml 1- The org.eclipse.rse.remotecdt.RemoteApplicationLaunch no longer needs to have the 'modes' specified 2- the two CDI tabs for main, arg and common tabs should not have the same id 3- The placement entry for the DSF Args tab must the the id returned by getId() of the Main tab instead (maybe override getId() in RemoteCDSFMainTab) plugin.properties 4- Can you rename "C/C++(DSF) Remote Debugging" to "GDB (DSF) Remote Debugging", because other debugger integration are based on DSF (like EDC) 5- Can you also change the matching description to something like "Start new application on a remote system under control of GDB debugger integrated using the Debugger Services Framework (DSF)." RemoteLaunchConfigurationTabGroup.java 6- Maybe cleanup that file and put a comment to explain why we don't actually set any tabs (like in PlaceHolderLaunchConfigurationTabGroup.java) RemoteCDSFMainTab.java 7- It would be more correct to override setDefaults() to set our own four properties as well. 8- Can we unify RemoteCDSFMainTab.java and RemoteCMainTab.java? RemoteCDSFDebuggerTab.java 9- In the constructor, use SessionType.REMOTE instead. 10- Can you put a comment along with the DEFAULTS_SET constant. I had trouble remembering what this was, although I'm the one that wrote it :-) 11- To be proper, we should do the same DEFAULTS_SET for the CDI RemoteCDebuggerTab.java GDBServerDSFFinalLaunchSequence.java (12- In the constructor, we could assert that the session is REMOTE and that 'attach' is false, since that is what we expect. Then you could get rid of all those checks in the sequence steps.) While looking at this file I got an idea. If we know exactly what this launch is doing more than the exiting remote launch, then we could simply do those things, and then fall back to the existing FinalLaunchSequence using the existing remote launch. That would greatly simply the code and would cause a lot more code-reuse so would be safer. So, just like you did in RemoteGdbLaunchDelegate.java where you check if the RSE initialization is complete, can we also do the part where we upload the binary and start gdbserver? After that, we are doing normal Remote debugging, I think, no? If there is a problem in doing the upload at the very beginning of the launch, we may be able to integrate that step in the FinalLaunchSequence by still having a new GDBServerDSFFinalLaunchSequence, but to have it extend FinalLaunchSequence and add steps at the beginning (adding steps in the middle is not possible). What do you think of that? Another value of this is that the maintenance would be almost free since any changes to FinalLaunchSequence would automatically be propagated to this remote launch. I haven't tried it though, so there may be some limitation I didn't forsee. I think prototyping this should be very quick. > There is one > problem with this patch though. If I use DSF debug and then switch to the CDI > debug, I'm getting the "Connection timeout" MI exception. I'm not sure yet why > this is happening... We can look at this once the patch has settled down. Created attachment 172052 [details]
patch that has some issues addressed
I fixed issues 1-7,9. (In reply to comment #13) > RemoteCDSFMainTab.java > 8- Can we unify RemoteCDSFMainTab.java and RemoteCMainTab.java? How do you suggest we do it? > RemoteCDSFDebuggerTab.java > 10- Can you put a comment along with the DEFAULTS_SET constant. I had trouble > remembering what this was, although I'm the one that wrote it :-) I don't know what it is too, I just used JTAG as an example. :) > GDBServerDSFFinalLaunchSequence.java > > (12- In the constructor, we could assert that the session is REMOTE and that > 'attach' is false, since that is what we expect. Then you could get rid of all > those checks in the sequence steps.) > > While looking at this file I got an idea. If we know exactly what this launch > is doing more than the exiting remote launch, then we could simply do those > things, and then fall back to the existing FinalLaunchSequence using the > existing remote launch. That would greatly simply the code and would cause a > lot more code-reuse so would be safer. > > So, just like you did in RemoteGdbLaunchDelegate.java where you check if the > RSE initialization is complete, can we also do the part where we upload the > binary and start gdbserver? After that, we are doing normal Remote debugging, > I think, no? I'll see if it is possible, the idea makes sense to me. (In reply to comment #15) > I fixed issues 1-7,9. Thanks. > > RemoteCDSFMainTab.java > > > > 7- It would be more correct to override setDefaults() to set our own four > > properties as well. You should use the proper defaults for this, instead of EMPTY_STRING for everything, e.g., SKIP_DOWNLOAD_TO_REMOTE_DEFAULT > (In reply to comment #13) > > RemoteCDSFMainTab.java > > 8- Can we unify RemoteCDSFMainTab.java and RemoteCMainTab.java? > > How do you suggest we do it? I forgot that they extend a different version of CMainTab. Forget this. > > RemoteCDSFDebuggerTab.java > > 10- Can you put a comment along with the DEFAULTS_SET constant. I had trouble > > remembering what this was, although I'm the one that wrote it :-) > > I don't know what it is too, I just used JTAG as an example. :) /* * When the launch configuration is created for Run mode, * this Debugger tab is not created because it is not used * for Run mode but only for Debug mode. * When we then open the same configuration in Debug mode, the launch * configuration already exists and initializeFrom() is called * instead of setDefaults(). * We therefore call setDefaults() ourselves and update the configuration. * If we don't then the user will be required to press Apply to get the * default settings saved. * Bug 281970 */ > > So, just like you did in RemoteGdbLaunchDelegate.java where you check if the > > RSE initialization is complete, can we also do the part where we upload the > > binary and start gdbserver? After that, we are doing normal Remote debugging, > > I think, no? > > I'll see if it is possible, the idea makes sense to me. Great, I'll wait for that update. I think we are almost there. Marc, I can easily move downloading a binary on target into RemoteGdbLaunchDelegate, but launching it on target requires knowing program arguments, and it seems like the only way to get them is via IGDBBackend, which is accessible in the LaunchSequence but is not in the delegate. It looks like if I am able to get the arguments, I can implement our plan. (In reply to comment #17) > Marc, I can easily move downloading a binary on target into > RemoteGdbLaunchDelegate, but launching it on target requires knowing program > arguments, and it seems like the only way to get them is via IGDBBackend, which > is accessible in the LaunchSequence but is not in the delegate. > It looks like if I am able to get the arguments, I can implement our plan. How about implementing the code that is run in IGDBBackend directly in the delegate? String args = config.getAttribute(ICDTLaunchConfigurationConstants.ATTR_PROGRAM_ARGUMENTS, (String)null); if (args != null) { args = VariablesPlugin.getDefault(). getStringVariableManager(). performStringSubstitution(args); } return args; Created attachment 172258 [details]
patch that has all the issues addressed but doesn't work
I seem to have implemented all the suggestions, but that lead to getting MI exception in every session even with DSF.
Message:
Failed to execute MI command:-target-select remote 127.0.0.1:2345
Error message from debugger back end:
127.0.0.1:2345: Connection timed out.
Stack:
java.lang.Exception: 127.0.0.1:2345: Connection timed out.
at org.eclipse.cdt.dsf.mi.service.command.AbstractMIControl$RxThread.processMIOutput(AbstractMIControl.java:824)
at org.eclipse.cdt.dsf.mi.service.command.AbstractMIControl$RxThread.run(AbstractMIControl.java:662)
Never mind, I'm on a new system and didn't have gdbserver installed. It works fine. I'll get it cleaned up a bit though. (In reply to comment #19) > Created an attachment (id=172258) [details] > patch that has all the issues addressed but doesn't work > > I seem to have implemented all the suggestions, but that lead to getting MI > exception in every session even with DSF. > Message: > Failed to execute MI command:-target-select remote 127.0.0.1:2345 > Error message from debugger back end: > 127.0.0.1:2345: Connection timed out. > Stack: > java.lang.Exception: 127.0.0.1:2345: Connection timed out. > at > org.eclipse.cdt.dsf.mi.service.command.AbstractMIControl$RxThread.processMIOutput(AbstractMIControl.java:824) > at > org.eclipse.cdt.dsf.mi.service.command.AbstractMIControl$RxThread.run(AbstractMIControl.java:662) Created attachment 172260 [details]
cleaned patch
(In reply to comment #16) > > > RemoteCDSFMainTab.java > > > > > > 7- It would be more correct to override setDefaults() to set our own four > > > properties as well. > > You should use the proper defaults for this, instead of EMPTY_STRING for > everything, e.g., SKIP_DOWNLOAD_TO_REMOTE_DEFAULT - IRemoteConnectionHostConstants.DEFAULT_SKIP_DOWNLOAD is a different property and not a value. I think you must use SKIP_DOWNLOAD_TO_REMOTE_DEFAULT. The other three defaults seem ok. - The new @since 7.0 tag in Activator.java is not right. You can increase the plugin to 7.1 and change the tag. - I think you must override getPluginID() in RemoteGdbLaunchDelegate - Why is the CDI Delegate doing DebugPlugin.newProcess(launch, remoteShellProcess, Messages.RemoteRunLaunchDelegate_RemoteShell); and not the new DSF Delegate? - In the Manifest file the line Export-Package: org.eclipse.cdt.launch.remote;x-internal:=true make the entire package internal. Why is that? This also turns off all API tooling things, which is why we are not getting errors for missing @since tags. - Please export the other package as well. We just found out that _every_ package should be exported. How do you want to proceed when it is time to commit? We can go through the IP approval process or we could have Ted commit it. It would be easier to have Ted commit it... (In reply to comment #22) > - In the Manifest file the line > Export-Package: org.eclipse.cdt.launch.remote;x-internal:=true > make the entire package internal. Why is that? This also turns off all API > tooling things, which is why we are not getting errors for missing @since tags. > > - Please export the other package as well. We just found out that _every_ > package should be exported. Marc, do you want me to remove x-internal:=true? I want to have it for the internal package for sure. > How do you want to proceed when it is time to commit? We can go through the IP > approval process or we could have Ted commit it. It would be easier to have > Ted commit it... Agreed. We'll go the least painful route and ask Ted to commit it when ready. Created attachment 172376 [details]
patch that has some issues addressed (except x-internal)
(In reply to comment #23) > Marc, do you want me to remove x-internal:=true? I want to have it for the > internal package for sure. I guess this is not your responsibility since you didn't put it there :-) I've opened Bug 317536 to track this. Removing the x-internal is not as simple as it seems for two reasons: 1- I guess someone should confirm that those API are ok to be made public 2- the API Tooling will complain about all the missing @since so all those will have to be fixed (pretty trivial, but still requires time, and I don't feel right asking you to do it) (In reply to comment #22) > - The new @since 7.0 tag in Activator.java is not right. You can increase the > plugin to 7.1 and change the tag. I was wrong about this. 7.1 is for most CDT plugins but not this one. This one actually has a version of 2.2 in the Manifest file. Please bump it to 2.3. But remove the @since tag, since everything is internal, none of this is official API, so let's leave it for Bug 317536. > - Why is the CDI Delegate doing > DebugPlugin.newProcess(launch, remoteShellProcess, > Messages.RemoteRunLaunchDelegate_RemoteShell); > and not the new DSF Delegate? Was this an over-sight? I believe this line adds a 'remote shell' process to our launch. I guess that is what we want? So, everything looks good besides the one point above about plugin version. I haven't actually run it yet, so let me do that, to get a feel for it, and then we can involve Ted. I'll get back to you. (In reply to comment #26) > (In reply to comment #22) > > Was this an over-sight? I believe this line adds a 'remote shell' process to > our launch. I guess that is what we want? Well, we want to show the output of the shell, and this is the reason it is there. This is very nice. It really makes remote launching much smoother.
I ran into a couple of issues though:
1- why does RSEHelper.remoteFileDownload return a Process? In the code, it only returns null.
2- The terminate action of the debug view does not work very well with this new launch.
a- terminating when the launch is selected (top element) does not work. It works in CDI. We should fix this for this bug.
b- terminating when the Remote Shell is selected does not work. This also does not work in CDI. We should fix this, but it could tracked by another bug.
c- when terminating with other elements selected, it works, except that the launch is not marked as terminated and the 'remove terminated launch' button remains disabled. This could be an unrelated issue see Bug 312267. This could also be tracked by another bug.
3- When doing an actual remote debugging session to another machine, I don't get the permissions of the downloaded file properly set:
> gdbserver.7.1 :2345 /home/lmckhou/DSFTestApp;exit
Process /home/lmckhou/DSFTestApp created; pid = 19249
Cannot exec /home/lmckhou/DSFTestApp: Permission denied.
The reason seems to be that the code
Thread.sleep(500);
p.destroy();
happens too fast. With a longer sleep, I'm ok. But I think this solution is risky. I wonder if we need this at all? I've noticed that the command "chmod +x" is followed by ";exit", so I wonder, do we need to call p.destroy() at all?
4- I wonder why we need the inferior process at all, shown in the launch? All input/output to the debugged application seems to be from this Remote Shell entry, so the other one seems useless. This could tracked by another bug though, since CDI does the same.
5- In the Main tab, what is the Properties... button next to the connection? I tried to use it (selected "skip download...") but it didn't seem to do anything.
If you don't want to address all issues as part of this bug, go ahead an open other bugs and we can get this patch committed. But 1, 2a and 3 should be fixed before.
Thanks
(In reply to comment #28) > This is very nice. It really makes remote launching much smoother. > > I ran into a couple of issues though: > 4- I wonder why we need the inferior process at all, shown in the launch? All > input/output to the debugged application seems to be from this Remote Shell > entry, so the other one seems useless. This could tracked by another bug > though, since CDI does the same. > What do you mean by "inferior process"? > 5- In the Main tab, what is the Properties... button next to the connection? I > tried to use it (selected "skip download...") but it didn't seem to do > anything. See Bug 231827 for details. (In reply to comment #28) > 2- The terminate action of the debug view does not work very well with this new > launch. > a- terminating when the launch is selected (top element) does not work. It > works in CDI. We should fix this for this bug. > b- terminating when the Remote Shell is selected does not work. This also > does not work in CDI. We should fix this, but it could tracked by another bug. The problem a) seems to be caused by the line DebugPlugin.newProcess(launch, remoteShellProcess, Messages.RemoteRunLaunchDelegate_RemoteShell); After commenting it out "Terminate" started working as expected. Maybe the remoteShellProcess needs to be added somehow differently so that DSF knows it is there? (In reply to comment #29) > (In reply to comment #28) > > This is very nice. It really makes remote launching much smoother. > > > > I ran into a couple of issues though: > > 4- I wonder why we need the inferior process at all, shown in the launch? All > > input/output to the debugged application seems to be from this Remote Shell > > entry, so the other one seems useless. This could tracked by another bug > > though, since CDI does the same. > > > What do you mean by "inferior process"? The application that is being debugged is called 'inferior' by GDB. So, under the launch, the process and the threads, you see three entries, one "Remote Shell", "gdb" and the last one is the inferior. That is the one that I wonder if we need at all. But that is not a big deal. We can leave it for now. > > 5- In the Main tab, what is the Properties... button next to the connection? I > > tried to use it (selected "skip download...") but it didn't seem to do > > anything. > See Bug 231827 for details. thanks (In reply to comment #30) > > a- terminating when the launch is selected (top element) does not work. It works in CDI. We should fix this for this bug. > > The problem a) seems to be caused by the line > DebugPlugin.newProcess(launch, remoteShellProcess, > Messages.RemoteRunLaunchDelegate_RemoteShell); > After commenting it out "Terminate" started working as expected. > Maybe the remoteShellProcess needs to be added somehow differently so that DSF > knows it is there? There is probably something obvious here, but I don't see it. All this terminate stuff is handled by platform code (Launch and RuntimeProcess) for both CDI and DSF, so if it works for CDI it should work for DSF. I'll debug it a big tomorrow. Created attachment 172758 [details]
patch with 1 and 3 fixed
(In reply to comment #32) > Created an attachment (id=172758) [details] > patch with 1 and 3 fixed Please up the plugin version in MANIFEST.MF to 2.3 > 1- why does RSEHelper.remoteFileDownload return a Process? In the code, it > only returns null. Ok, this is fixed. > 2- The terminate action of the debug view does not work very well with this new > launch. > a- terminating when the launch is selected (top element) does not work. It > works in CDI. We should fix this for this bug. This is caused because the RemoteShell process takes longer to be terminated. The platform should still detect it through a DebugEvent.TERMINATE but for some reason it does not. I don't have more time to spend on it so let's open a new bug for this issue and keep moving forward. > b- terminating when the Remote Shell is selected does not work. This also > does not work in CDI. We should fix this, but it could tracked by another bug. Please open a bug. > c- when terminating with other elements selected, it works, except that the > launch is not marked as terminated and the 'remove terminated launch' button > remains disabled. This could be an unrelated issue see Bug 312267. This could > also be tracked by another bug. I believe this is the same issue as 2a, so let's but both in the same bug. > 3- When doing an actual remote debugging session to another machine, I don't > get the permissions of the downloaded file properly set: > > gdbserver.7.1 :2345 /home/lmckhou/DSFTestApp;exit > Process /home/lmckhou/DSFTestApp created; pid = 19249 > Cannot exec /home/lmckhou/DSFTestApp: Permission denied. > > The reason seems to be that the code > Thread.sleep(500); > p.destroy(); > happens too fast. With a longer sleep, I'm ok. But I think this solution is > risky. I wonder if we need this at all? I've noticed that the command "chmod > +x" is followed by ";exit", so I wonder, do we need to call p.destroy() at all? Works well with the latest patch. > 4- I wonder why we need the inferior process at all, shown in the launch? All > input/output to the debugged application seems to be from this Remote Shell > entry, so the other one seems useless. This could tracked by another bug > though, since CDI does the same. Let's forget about this until someone else agrees that it is something to change. > 5- In the Main tab, what is the Properties... button next to the connection? I > tried to use it (selected "skip download...") but it didn't seem to do > anything. So, this Properties.. is a global setting, right? Well, when I use it, it does not work. I set the skip download to enabled by default and the binary still gets downloaded based on the launch checkbox. If I create a new launch, it does not take the global property either. Anyway, this is not related to this bug, so let's open a new one. == I'm happy with this patch. Can you open the three bugs that are mentioned in this comment? And up the plugin version. After that, if you can get Ted to commit the patch then you can resolve this. Thanks a lot for the good work. Created attachment 172819 [details]
patch with updated version
(In reply to comment #35) > Marc, I created bugs 318050, 318051, 318052 per your request. Thanks for creating bug 318050, bug 318051, bug 318052. I leave the rest up to you and Ted. Created attachment 172913 [details]
the whole patch
For some reason, only some changes got into previous patch, so I've redone it.
Any update on getting this committed? Marc, for some reason Ted's committer account is suspended. He is trying to get it resolved now. (In reply to comment #38) > Any update on getting this committed? Committed. *** cdt cvs genie on behalf of tewillia *** [314659] Move remote launch/debug to DSF [*] .classpath 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/.classpath?root=Tools_Project&r1=1.1&r2=1.2 [*] plugin.properties 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/plugin.properties?root=Tools_Project&r1=1.3&r2=1.4 [*] plugin.xml 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/plugin.xml?root=Tools_Project&r1=1.1&r2=1.2 [*] org.eclipse.jdt.core.prefs 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/.settings/org.eclipse.jdt.core.prefs?root=Tools_Project&r1=1.1&r2=1.2 [*] RSEHelper.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/launch/remote/RSEHelper.java?root=Tools_Project&r1=1.2&r2=1.3 [+] RemoteGdbLaunchDelegate.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/launch/remote/RemoteGdbLaunchDelegate.java?root=Tools_Project&revision=1.1&view=markup [+] RemoteCDSFMainTab.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/launch/remote/RemoteCDSFMainTab.java?root=Tools_Project&revision=1.1&view=markup [+] RemoteDSFGDBDebuggerPage.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/launch/remote/RemoteDSFGDBDebuggerPage.java?root=Tools_Project&revision=1.1&view=markup [*] RemoteCDebuggerTab.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/launch/remote/RemoteCDebuggerTab.java?root=Tools_Project&r1=1.2&r2=1.3 [*] RemoteLaunchConfigurationTabGroup.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/launch/remote/RemoteLaunchConfigurationTabGroup.java?root=Tools_Project&r1=1.2&r2=1.3 [*] RemoteCMainTab.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/launch/remote/RemoteCMainTab.java?root=Tools_Project&r1=1.3&r2=1.4 [+] RemoteCDSFDebuggerTab.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/launch/remote/RemoteCDSFDebuggerTab.java?root=Tools_Project&revision=1.1&view=markup [*] RemoteRunLaunchDelegate.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/launch/remote/RemoteRunLaunchDelegate.java?root=Tools_Project&r1=1.3&r2=1.4 [*] Activator.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/src/org/eclipse/cdt/internal/launch/remote/Activator.java?root=Tools_Project&r1=1.2&r2=1.3 [*] MANIFEST.MF 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/cross/org.eclipse.cdt.launch.remote/META-INF/MANIFEST.MF?root=Tools_Project&r1=1.2&r2=1.3 |