Community
Participate
Working Groups
Example of current situation: DSF-GDB provides: GDBRunControl_7_2_NS extends GDBRunControl_7_0_NS someone specializes this by creating MyPrivateGDBRunControl extends GDBRunControl_7_2_NS The day that CDT adds a new version like GDBRunControl_7_9_NS extends GDBRunControl_7_2_NS the extending class MyPrivateGDBRunControl will not use GDBRunControl_7_9_NS unless the developer notices this new version. Both Mikhail and William confirmed they regularly have this issue. We propose to insert an empty class that will always be at the top. Something like GDBRunControl_NS_Head extends GDBRunControl_7_2_NS Extenders would use MyPrivateGDBRunControl extends GDBRunControl_NS_Head When CDT wants to introduce GDBRunControl_7_9_NS, we would do it like this: GDBRunControl_NS_Head extends GDBRunControl_7_9_NS extends GDBRunControl_7_2_NS which will allow MyPrivateGDBRunControl to automatically inherit from the new version. This change need not break the API and could be done easily anytime.
New Gerrit change created: https://git.eclipse.org/r/52358
(In reply to Eclipse Genie from comment #1) > New Gerrit change created: https://git.eclipse.org/r/52358 I've posted a solution. It also includes a bit of code making use of it part of o.e.cdt.examples.dsf.gdb. What I realized when using it in the example is that the suggested pattern of extending GDB<service>_HEAD will help whenever a new GDB version comes out with a corresponding new service version, but will affect negatively the extender's support of the previous GDB version for that service. I don't think we can help both situations; either we help extenders automatically support the newest GDB version at the expense of the old one, or the extender will (automatically) continue to properly support the old version, at the expense of the new one. Having the different GDB<service>_HEAD available does not force extenders to use them, but gives them the opportunity to do so. It can then be up to the extender to choose if they want to focus on the latest GDB version or not. I'd love to hear from extenders if this can help their situation.
If this pattern is useful, we could also use it for other versioned classes such as: StartOrRestartProcessSequence_7_3 MIRunControlEventProcessor_7_0 FinalLaunchSequence_7_7 DebugNewProcessSequence_7_2 CLIEventProcessor_7_7 although I'm not sure how often these cause problems to extenders, as they don't change nearly as often.
(In reply to Marc Khouzam from comment #2) > (In reply to Eclipse Genie from comment #1) > > New Gerrit change created: https://git.eclipse.org/r/52358 > > I've posted a solution. It also includes a bit of code making use of it > part of o.e.cdt.examples.dsf.gdb. > > What I realized when using it in the example is that the suggested pattern > of extending GDB<service>_HEAD will help whenever a new GDB version comes > out with a corresponding new service version, but will affect negatively the > extender's support of the previous GDB version for that service. > > I don't think we can help both situations; either we help extenders > automatically support the newest GDB version at the expense of the old one, > or the extender will (automatically) continue to properly support the old > version, at the expense of the new one. > > Having the different GDB<service>_HEAD available does not force extenders to > use them, but gives them the opportunity to do so. It can then be up to the > extender to choose if they want to focus on the latest GDB version or not. > > I'd love to hear from extenders if this can help their situation. I can't help with this problem, we always use the latest GDB. IMO, the flexibility provided by this solution is sufficient to cover both cases.
(In reply to Mikhail Khodjaiants from comment #4) > I can't help with this problem, we always use the latest GDB. IMO, the > flexibility provided by this solution is sufficient to cover both cases. Ok, as long as it helps I think it is worth it. I'll wait until early next week to see if maybe William as a comment.
(In reply to Marc Khouzam from comment #5) > (In reply to Mikhail Khodjaiants from comment #4) > > > I can't help with this problem, we always use the latest GDB. IMO, the > > flexibility provided by this solution is sufficient to cover both cases. > > Ok, as long as it helps I think it is worth it. > > I'll wait until early next week to see if maybe William as a comment. I was thinking about this, as we do not always use the latest versions of GDB. It don't think it is a major problem, extenders who need to be able to support older GDB versions (like us) do not need to use the new pattern of “HEAD”, but extenders who don’t no longer need to do as much work. It is probably worth a comment in the example plugin documenting the limitations of "HEAD". I did think of a few ways to make the “HEAD” pattern more tolerant of that, but I’m not sure they really add much other than extra complexity, as it cannot work in all cases without the extender doing the same work they do now. Adding a way for "HEAD" to specify the minimum GDB version required, which could then be checked in the ServicesFactory, would at least allow a cleaner fallback if the extender wants. If not then I think at least an error needs to be logged if "HEAD" is used with an old GDB version. So it is clear what is happening if this causes debugging issues.
WARNING: this patchset contains 1274 new lines of code and may require a Contribution Questionnaire (CQ) if the author is not a committer on the project. Please see:https://wiki.eclipse.org/Project_Management_Infrastructure/Creating_A_Contribution_Questionnaire
(In reply to William Riley from comment #6) > I was thinking about this, as we do not always use the latest versions of > GDB. It don't think it is a major problem, extenders who need to be able to > support older GDB versions (like us) do not need to use the new pattern of > “HEAD”, but extenders who don’t no longer need to do as much work. It is > probably worth a comment in the example plugin documenting the limitations > of "HEAD". I have added a NOTE to every *HEAD file which attempts and pointing this out. I've also put a comment in the example. This is in patchset 2. > Adding a way for "HEAD" to specify the minimum GDB version required, which > could then be checked in the ServicesFactory, would at least allow a cleaner > fallback if the extender wants. > > If not then I think at least an error needs to be logged if "HEAD" is used > with an old GDB version. So it is clear what is happening if this causes > debugging issues. I've tried to do that in patchset 3. It adds some duplicated code to each *_HEAD class which prints to the log when finding an old GDB with a new service version. I've a grown a little hesitant about the real value of this enhancement. Will this really help extenders, or could it be a poisoned gift? You guys have a better appreciation of the reality for extenders, so I'll go with your recommendations. Do you think this is worth going forward with at all?
I've update with patchset 5 which reduces code duplication. Also, to address my concerns about older GDBs it: 1- prints a warning in the log if an older GDB runs with a *HEAD that is newer 2- is asserts in that same case, which should allow extenders to notice this early if they test with older GDBs, while not bother users I've tested this with the DSF-GDB example plugin and it works well. I'm more comfortable with the change as it stands now. I'll commit next week if Mikhail or William don't express concerns.
Gerrit change https://git.eclipse.org/r/52358 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=0d7432318e8098bf8c50bcfedfccfb36cbce94fd
Merged to master. Thanks everyone for the feedback. I've updated https://wiki.eclipse.org/CDT/cdt-debug-dsf-gdb-extensibility and put an note in the N&N: https://wiki.eclipse.org/CDT/User/NewIn88#DSF-GDB_service_extensibility_improvement I hope this helps extenders.
I'm may be late to this, but I've only been able to look into this change now. I don't see how this *_HEAD change is of any significant help. Sure, MyPrivateGDBRunControl will automatically inherit from a new version that gets added to the GDBRunControl_HEAD hierarchy (say 7.9 for example), but the extender IDE still has to customize the code that *creates* MyPrivateGDBRunControl, and that code is still totally sensitive to GDB version changes. For example, if we want to customize GDBControl_7_7 with MyGDBControl, we still have to write code to check against the GDB version, see for example here: https://github.com/RustDT/RustDT/blob/master/plugin_ide.debug/src-lang/melnorme/lang/ide/debug/core/services/LangDebugServicesExtensions.java#L84 In that code, all I want to do is be able to customize the FinalLaunchSequence for GDB versions 7.7 or above. But I have to jump through a lot of hoops to be able to do that, and even so, the way I did things doesn't work if a GDBControl_7_9 is introduced (as in, my customization won't be applied to a 7.9 GDB) . If I try to inherit from GDBControl_HEAD, I still have this line that I have to customize: if(GdbDebugServicesFactory.GDB_7_7_VERSION.compareTo(gdbSvcFactory.getVersion()) <= 0) which in fact will break if a GDBControl_7_9 is introduced, because now GDBControl_HEAD (which now inherits from GDBControl_7_9) will be applied to a debug session using GDB 7.7! Maybe I'm missing something, but I don't see how this solution helps at all, in fact it seems to be begging for errors to be made.
(In reply to Bruno Medeiros from comment #12) > [...] > Maybe I'm missing something, but I don't see how this solution helps at all, > in fact it seems to be begging for errors to be made. If you look at comment #2, it mentions your findings. In the end using the *_HEAD pattern helps extenders that target the latest GDB all the time. It is not as good for extenders that want to support older GDB versions. So the suggested approach is that if you want to keep a service moving upwards with the GDB version, then extend *_HEAD, but for the services that target a specific version, they should not use it. If we can tweak or extend the solution to cover your case, that would be great, but I don't have a good suggestion right now.
It seems to be the whole concept of using class hierarchies to customize the behavior of each GDB service according to the GDB version, is broken. As in, there shouldn't be multiple GDBRunControl/GDBControl/etc for each GDB version, rather there should be just one of each GDBRunControl/GDBControl/etc class, whose *objects* would be configured differently at runtime, according to the actual GDB version. Then, extenders would be able to do stuff like: if( gdbVersion.major == 7 && gdbVersion.minor == 5 ) { // Customize this way for 7.5 } else if( gdbVersion.major == 7 && gdbVersion.minor > 7 ) { // Customize this way for 7.7 or above } And it would work seamlessly when new GDB versions are introduced. Is there any particular reason why things aren't done this way? As it stands, with every new CDT minor version I have to check the GDB support classes of CDT to see if anything new is added. It's not a major problem at the moment for me, but I worry it could become trickier in the future.
(In reply to Bruno Medeiros from comment #14) > there shouldn't be multiple GDBRunControl/GDBControl/etc for each GDB > version, rather there should be just one of each > GDBRunControl/GDBControl/etc class, whose *objects* would be configured > differently at runtime, > > Is there any particular reason why things aren't done this way? When we first started DSF-GDB we made the decision not to go with using versioning -within- a class. The rationale was that adding support for a new version of GDB would then create risk for the support of an older version. By using the class hierarchy as we do, when we create a new service class, it has no impact on the existing base services, thus guaranteeing that we don't break older support. It also comes with its drawbacks, but in the end, I believe it has helped us reduce the maintenance burden for older GDBs, which we naturally tend not to test as much as the most recent one.
