Bugzilla – Full Text Bug Listing
| Summary: | Build before launch should be cleverer about which configuration is built | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | James Blackburn <jamesblackburn+eclipse> | ||||||||||
| Component: | cdt-debug | Assignee: | James Blackburn <jamesblackburn+eclipse> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Ken Ryall <ken.ryall> | ||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | CC: | acollins, john.cortell, pawel.1.piech | ||||||||||
| Version: | 7.0 | Flags: | john.cortell:
review+
jamesblackburn+eclipse: review? (ken.ryall) |
||||||||||
| Target Milestone: | 8.0 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
James Blackburn
Created attachment 176043 [details]
patch 1
The attached patch adds a checkbox to the launch configuration dialog, enabling or disabling automatic choosing of the configuration to be built, based on the path to the program.
When the checkbox is enabled the build config drop down is greyed out, and the configuration that is automatically chosen is selected. This shows the user which configuration chosen before running the launch.
The choice of build config is updated whenever the launch is run, to respect any intervening changes to the build configurations.
A couple issues:
- java.lang.NullPointerException
at org.eclipse.cdt.launch.LaunchUtils.getBuildConfigByProgramPath(LaunchUtils.java:178)
at org.eclipse.cdt.launch.ui.CAbstractMainTab.updateBuildConfigCombo(CAbstractMainTab.java:287)
at org.eclipse.cdt.launch.ui.CAbstractMainTab.updateBuildOptionFromConfig(CAbstractMainTab.java:417)
at org.eclipse.cdt.dsf.gdb.internal.ui.launching.CMainTab.initializeFrom(CMainTab.java:276)
when opening the dialog (ACPathEntry#getLocation() returns null if the directory doesn't exist).
- private CAbstractMainTac#fBuildConfigAuto perhaps make this protected to be consistent with the other GUI members.
- 'Auto' not checked by default
- Checkbox label "Choose automatically using program name" perhaps better that the label contains the text field's name: "Select configuration using 'C/C++ Application'"?
- LaunchUtils#getBuildConfigByProgramPath. Is findFilesByName the right thing to do for relative paths? Aren't relative paths always relative to the project? Probably something like IProject.getFile(path).getLocation()
+ '// Matched more than one, so use the active configuration' this is unlikely to happen, perhaps log this?
- What happens when a non-CDT project is used?
Created attachment 176324 [details]
patch 2
Thanks for the feedback James; the attached patch resolves these issues.
Created attachment 177203 [details]
patch 3
A few tweaks to Alex's patch:
- LaunchUtils#getBuildConfigByProgramPath: should use core model for project description, not property manager (which can only be safely used from UI thread).
- AbstractCLaunchDelegate2#buildForLaunch: should also check the ATTR_PROJECT_BUILD_CONFIG_AUTO flag and select the appropriate build configuration
- dsf.gdb.ui CMainTab: set auto config as default for new configurations here too
It's slightly odd that LaunchUtils#getBuildConfigByProgramPath converts resources to location for comparison rather than sticking to IResources, but this looks consistent with the CMainTab. Given the launches only work on local resources, I guess this is ok for now.
John / Ken I think you were involved in the build before launch functionality. If you get a chance, review would be appreciated. Created attachment 177205 [details]
patch 4
Don't change the underlying launch configuration (i.e. don't apply the auto attribute) if the user hasn't explicitly clicked the checkbox. This is to prevent unnecessary deltas of launch configs held in a repository.
Thanks for the patch Alex. Committed to HEAD (8.0). *** cdt cvs genie on behalf of jblackburn *** Bug 317796 - Build before launch should be cleverer about which configuration is built. Add option to LaunchConfig Main Tab to automatically choose a build configuration appropriate to the binary being debugged. [*] CMainTab.java 1.13 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/launching/CMainTab.java?root=Tools_Project&r1=1.12&r2=1.13 [*] ICDTLaunchConfigurationConstants.java 1.34 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/ICDTLaunchConfigurationConstants.java?root=Tools_Project&r1=1.33&r2=1.34 [*] LaunchMessages.properties 1.36 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/internal/ui/LaunchMessages.properties?root=Tools_Project&r1=1.35&r2=1.36 [*] CMainTab.java 1.87 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/ui/CMainTab.java?root=Tools_Project&r1=1.86&r2=1.87 [*] CAbstractMainTab.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/ui/CAbstractMainTab.java?root=Tools_Project&r1=1.2&r2=1.3 [*] LaunchUtils.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/LaunchUtils.java?root=Tools_Project&r1=1.9&r2=1.10 [*] AbstractCLaunchDelegate2.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate2.java?root=Tools_Project&r1=1.9&r2=1.10 [*] AbstractCLaunchDelegate.java 1.73 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-launch/org.eclipse.cdt.launch/src/org/eclipse/cdt/launch/AbstractCLaunchDelegate.java?root=Tools_Project&r1=1.72&r2=1.73 [*] MANIFEST.MF 1.18 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-launch/org.eclipse.cdt.launch/META-INF/MANIFEST.MF?root=Tools_Project&r1=1.17&r2=1.18 Nice patch. Looks good to me. |