Community
Participate
Working Groups
Steps to reproduce: 1. install gdb in a path with spaces 2. in eclipse launch configuration specify the full gdb path in 'GDB command' field Debug session will fail because of the unresolved spaces. I noticed this happens only on Linux. The most common case is when eclipse is installed in a path with spaces and 'GDB command' is relative to the eclipse directory, i.e: GDB command: ${eclipse_home}../gdb/bin/gdb Debug session will fail due to the same reason.
Uploaded patch with a proposed fix: https://git.eclipse.org/r/#/c/34052/ If someone can have a look. Thanks!
Committed to master. Thanks!
I missed the backword compatibility regression for a gdb backend that does override deprecated methods. I have pushed another patch that would address this as well: https://git.eclipse.org/r/#/c/34578/
Issues with extenders have come up: https://dev.eclipse.org/mhonarc/lists/cdt-dev/msg28256.html
(In reply to Teodor Madan from comment #3) > I missed the backword compatibility regression for a gdb backend that does > override deprecated methods. > > I have pushed another patch that would address this as well: > > https://git.eclipse.org/r/#/c/34578/ Thanks Teo. I have to admit I am not a big fan of this solution which seems hacky. Can we do better? The issue is that launchGDBProcess(String) is not being called any more. I would like to try a solution that still calls this method and then changes the before, probably splitting the string. I'll try out things on my side as well.
(In reply to Marc Khouzam from comment #5) > (In reply to Teodor Madan from comment #3) > > I missed the backword compatibility regression for a gdb backend that does > > override deprecated methods. > > > > I have pushed another patch that would address this as well: > > > > https://git.eclipse.org/r/#/c/34578/ I've pushed a new patchset which I think solves the issue. For CDT we will use the new API but we still call the old one first.
(In reply to Marc Khouzam from comment #6) > (In reply to Marc Khouzam from comment #5) > > (In reply to Teodor Madan from comment #3) > > > I missed the backword compatibility regression for a gdb backend that does > > > override deprecated methods. > > > > > > I have pushed another patch that would address this as well: > > > > > > https://git.eclipse.org/r/#/c/34578/ > > I've pushed a new patchset which I think solves the issue. For CDT we will > use the new API but we still call the old one first. Actually, this proposed fix works when launchGDBProcess(String) is overridden but not when only getGDBCommandLine() is overridden. I'll make a better fix, but Elena, can you confirm which situation you are in?
(In reply to Marc Khouzam from comment #7) > > > > https://git.eclipse.org/r/#/c/34578/ > > > > I've pushed a new patchset which I think solves the issue. For CDT we will > > use the new API but we still call the old one first. > > Actually, this proposed fix works when launchGDBProcess(String) is > overridden but not when only getGDBCommandLine() is overridden. I pushed another version which I think works for all combinations of overriding. It is not as elegant as I hoped though. It has to rely on comparing the commandLine string with the getGDBCommandLineArray() to see if anything changed. I'm not sure if it is more understandable than Teo's fix (patchset 1). I'm ok with choosing to go back to Teo's fix if you prefer. One advantage of the latest patchset over the first is that for extenders that don't modify the commandLine parameter but do override either method, we will still use the new call to Process.exec which accepts spaces. This is not such a valuable aspect though. What do you guys prefer? (assuming both solution work for Elena)
It is hard to keep compatibility for a method that must be backward compatible with respect to existing callers and to existing overridden cases without resorting to a hacky solution. IMHO, the cleanest code would've been to break compatibility for such cases. Nevertheless, this case does not worth IMO, and cleaning could be done on other occasions. Between java reflection and the commandLine string comparison I think the later is better as it also covers gradual transition to new API. Java reflection case solution would have require GDBBackend extenders to remove old method. To close, it is enough for Elena to verify that latest patch works.
(In reply to Teodor Madan from comment #9) > Between java reflection and the commandLine string comparison I think the > later is better as it also covers gradual transition to new API. Java > reflection case solution would have require GDBBackend extenders to remove > old method. Good point! > To close, it is enough for Elena to verify that latest patch works. Elena confirmed so I committed to master.
Sorry about that and thanks for fixing this!
Please note that this fix has been improved to use shell-parsing syntax. See Bug 458499 for details.
