Bugzilla – Full Text Bug Listing
| Summary: | CDT Global Build Console created for every CDT CBuildConsole extension | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Jeff Johnston <jjohnstn> | ||||||||||
| Component: | cdt-build | Assignee: | Andrew Gvozdev <angvoz.dev> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Andrew Gvozdev <angvoz.dev> | ||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | CC: | acollins, cdtdoug, jamesblackburn+eclipse | ||||||||||
| Version: | 8.0 | ||||||||||||
| Target Milestone: | 8.0 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Linux | ||||||||||||
| Whiteboard: | |||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 342069 | ||||||||||||
| Attachments: |
|
||||||||||||
Can you attach a small plugin defining console like that to demonstrate the issue? (In reply to comment #1) > Can you attach a small plugin defining console like that to demonstrate the > issue? I tried, but unfortunately, it is a bit complicated. There needs to be a builder set up in addition to the regular CDT Common Builder for the project. I couldn't get it to invoke my builder so as to open the console and write to it which would demonstrate what is happening under Autotools. The easy way to demonstrate this is to download the Autotools plug-in from the Indigo site (in the Programming Languages section I believe). Create a new C Project and choose GNU Autotools -> Hello ANSI C program. Build the project. Look at the consoles. There will be two CDT Global Consoles. With the fix, there is just the one. I noticed upon testing just a regular Managed Make hello world executable that the build console can be empty while the global console is full. Some kind of timing glitch to keep an eye on (perhaps the async startup() doesn't complete in time???) (In reply to comment #1) > Can you attach a small plugin defining console like that to demonstrate the > issue? Were you able to reproduce with the Autotools plug-in installed? Yes I am looking into this. I am trying to define console like that myself to understand how all the pieces work together. It aligns with another project where I'd like to create a new console. Any tip is appreciated. Maybe I should look at Autotools code how it is done there. (In reply to comment #4) > Yes I am looking into this. I am trying to define console like that myself to > understand how all the pieces work together. It aligns with another project > where I'd like to create a new console. Any tip is appreciated. Maybe I should > look at Autotools code how it is done there. Ok, I've made it easier. I am including a new version of sampleConsole. In its Activator, it creates a sample console and the default build console. To use it, create a C project of some sort. Then, go to Window->Preferences and click on the preference that says "Click here to ...". This will run the Activator for sampleConsole which will show you the bug. For some reason, when I use my patch and run the test, the global console is not getting updated with either of the build messages so something is wrong with my implementation. Only one global console gets created so that part is at least correct. Created attachment 192589 [details]
Plug-in that will show problem.
(In reply to comment #5) > > For some reason, when I use my patch and run the test, the global console is > not getting updated with either of the build messages so something is wrong > with my implementation. Only one global console gets created so that part is > at least correct. Hmm, it appears to be a display update issue. If I click on the C project, the CDT Global Build Console magically shows the lines we wrote to it concatenated together since there aren't any newlines in the output. Maybe my GlobalBuildConsoleManager is not receiving an event properly. (In reply to comment #6) > Created attachment 192589 [details] > Plug-in that will show problem. Thanks that was helpful. I was able to wire the sample console to my project in no time. Having another global console as forced extra of course. One question about your requirements. From the patch I believe you want to add contents of your console to the global console, is that right? I do not want to litter the global console in my case, especially considering that it might happen in concurrent thread. (In reply to comment #8) > (In reply to comment #6) > > Created attachment 192589 [details] > > Plug-in that will show problem. > Thanks that was helpful. I was able to wire the sample console to my project in > no time. Having another global console as forced extra of course. > > One question about your requirements. From the patch I believe you want to add > contents of your console to the global console, is that right? I do not want to > litter the global console in my case, especially considering that it might > happen in concurrent thread. The patch was just making the global console actually global as opposed to one per console. It was our doing that got the CBuildConsole extension added. In the real-world case of Autotools, the configuration builder runs before the common builder as part of a single build so it would seem to make sense to show the combined output in the Global console; however, we could certainly live with the CBuildConsole extension output not being shown in the global console if you want to go another route. The key issue to solve is the multiple global consoles. Created attachment 193202 [details] rebased patch Here is original patch attachment 191933 [details] rebased against HEAD. I hope I resolved conflicts correctly. (In reply to comment #10) > Created attachment 193202 [details] > rebased patch > Here is original patch attachment 191933 [details] rebased against HEAD. I hope I resolved > conflicts correctly. I should say that it is not intended for commit, just to make it easier to review. Created attachment 193457 [details] patch The global console was added in bug 309113 in CDT 8.0. It looks like it was under assumption that there is only one console manager in CDT. Starting with Jeff's patch, I changed IBuildConsoleManager (which is a @noimplement interface btw) to remove methods related to global console such as startGlobalConsole(), and moved them to GlobalBuildConsoleManager. Also a few related changes. Jeff, please take a look if it works for you. James, the global console was your request. Do you have any issues with this approach? In short, this leads to one truly global console instead one "global" console for each console manager. (In reply to comment #12) > Created attachment 193457 [details] > patch > > The global console was added in bug 309113 in CDT 8.0. It looks like it was > under assumption that there is only one console manager in CDT. Starting with > Jeff's patch, I changed IBuildConsoleManager (which is a @noimplement interface > btw) to remove methods related to global console such as startGlobalConsole(), > and moved them to GlobalBuildConsoleManager. Also a few related changes. > > Jeff, please take a look if it works for you. Yes it works in that there is only the one Global Console. When I do a reconfigure, the output ends up appended to the CDT Global Console which has the last build still in it. I assume that the Autotools Configure builder just needs to manually clear the global console when it starts a build. (In reply to comment #14) > Yes it works in that there is only the one Global Console. When I do a > reconfigure, the output ends up appended to the CDT Global Console which has the > last build still in it. I assume that the Autotools Configure builder just > needs to manually clear the global console when it starts a build. In CDT Global console is cleaned when a new build is initiated in UI, you can look where calls to CUIPlugin.getDefault().startGlobalConsole() are placed. If you are using your own to initiate the build you'd need to call it too. Note that there is preference "Always clear console before building", it may be looked up too. A builder itself is not a good place to clear the console, some other plugin can insert own builder before yours so it has to be done before any builder is run. I wonder how it plays with autobuild enabled or build on save. I want to get this patch in, if you have issues with clearing console please open a new task. (In reply to comment #15) > A builder itself is not a good place to clear the console, some other plugin > can insert own builder before yours so it has to be done before any builder is > run. I wonder how it plays with autobuild enabled or build on save. For 3.7 this can be fixed by cdt.core registering a PRE_BUILD listener that clears the global console if the preference says it should. PRE_BUILD is called once per top-level build. (In reply to comment #13) > James, the global console was your request. Do you have any issues with this > approach? In short, this leads to one truly global console instead one "global" > console for each console manager. Yep, sounds good. The intention was that the global build console is truly global, so I'm not sure why it was attached to the console manager. (In reply to comment #16) > (In reply to comment #15) > > A builder itself is not a good place to clear the console, some other plugin > > can insert own builder before yours so it has to be done before any builder is > > run. I wonder how it plays with autobuild enabled or build on save. > For 3.7 this can be fixed by cdt.core registering a PRE_BUILD listener that > clears the global console if the preference says it should. PRE_BUILD is called > once per top-level build. I agree with that but not sure how easy it is to figure out if CDT (and other possible builders contributing to CDT console) in fact is going to run the build. We do not want to clear console on any autobuild event, only if there is an action. (In reply to comment #18) > I agree with that but not sure how easy it is to figure out if CDT (and other > possible builders contributing to CDT console) in fact is going to run the > build. We do not want to clear console on any autobuild event, only if there is > an action. I think you can just check the build kind: IResourceChangeEvent#getBuildKind() ignoring AUTO_BUILD At the moment CDT doesn't respond to AUTO_BUILD, so this is a reasonable heuristic. We shouldn't require (or allow?) clients outside of CDT to clear the global console as it could lead to issues in the future if the console mysteriously clears itself when multiple builders are attached... (In reply to comment #19) > (In reply to comment #18) > > I agree with that but not sure how easy it is to figure out if CDT (and other > > possible builders contributing to CDT console) in fact is going to run the > > build. We do not want to clear console on any autobuild event, only if there > is > > an action. > I think you can just check the build kind: IResourceChangeEvent#getBuildKind() > ignoring AUTO_BUILD OK thanks for the pointer. I would still rather have another task to consider the clearing issue separately. There is multitude of ways to build, does every path goes through platform build? > At the moment CDT doesn't respond to AUTO_BUILD, so this is a reasonable > heuristic. We shouldn't require (or allow?) clients outside of CDT to > clear the global console as it could lead to issues in the future if the console > mysteriously clears itself when multiple builders are attached... I committed the patch to the HEAD. (In reply to comment #20) > OK thanks for the pointer. I would still rather have another task to consider > the clearing issue separately. Sounds good. > There is multitude of ways to build, does every > path goes through platform build? Yes, the platform is the central coordinator of all builds (which use platform build infrastructure and build commands). *** cdt cvs genie on behalf of agvozdev *** bug 340995: CDT Global Build Console created for every CDT CBuildConsole extension [*] CopyBuildLogAction.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/CopyBuildLogAction.java?root=Tools_Project&r1=1.4&r2=1.5 [*] BuildConsolePartitioner.java 1.18 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/BuildConsolePartitioner.java?root=Tools_Project&r1=1.17&r2=1.18 [*] ShowErrorAction.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/ShowErrorAction.java?root=Tools_Project&r1=1.4&r2=1.5 [+] GlobalBuildConsoleManager.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/GlobalBuildConsoleManager.java?root=Tools_Project&revision=1.1&view=markup [*] GlobalBuildConsole.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/GlobalBuildConsole.java?root=Tools_Project&r1=1.1&r2=1.2 [*] BuildConsolePage.java 1.31 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/BuildConsolePage.java?root=Tools_Project&r1=1.30&r2=1.31 [*] BuildConsoleManager.java 1.30 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/BuildConsoleManager.java?root=Tools_Project&r1=1.29&r2=1.30 [*] IBuildConsoleManager.java 1.11 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/IBuildConsoleManager.java?root=Tools_Project&r1=1.10&r2=1.11 [*] CUIPlugin.java 1.102 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/CUIPlugin.java?root=Tools_Project&r1=1.101&r2=1.102 [*] GlobalBuildLogPreferencePage.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/preferences/GlobalBuildLogPreferencePage.java?root=Tools_Project&r1=1.2&r2=1.3 [*] BuildLogPreferencePage.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/preferences/BuildLogPreferencePage.java?root=Tools_Project&r1=1.3&r2=1.4 [*] BuildGroup.java 1.20 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/cview/BuildGroup.java?root=Tools_Project&r1=1.19&r2=1.20 [*] TargetBuild.java 1.22 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.make.ui/src/org/eclipse/cdt/make/ui/TargetBuild.java?root=Tools_Project&r1=1.21&r2=1.22 [*] BuildConsoleTests.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/buildconsole/BuildConsoleTests.java?root=Tools_Project&r1=1.6&r2=1.7 [*] BuildAllConfigurationsAction.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/internal/ui/actions/BuildAllConfigurationsAction.java?root=Tools_Project&r1=1.2&r2=1.3 [*] CleanAllConfigurationsAction.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/internal/ui/actions/CleanAllConfigurationsAction.java?root=Tools_Project&r1=1.2&r2=1.3 [*] BuildConfigurationsJob.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/internal/ui/actions/BuildConfigurationsJob.java?root=Tools_Project&r1=1.3&r2=1.4 |
Created attachment 191933 [details] proposed patch to fix problem The CDT Global Build Console gets created for every CBuildConsole extension specified. This is because there is one BuildConsoleManager per console id as dictated by the CUIPlugin getConsole(name, id) method and each BuildConsoleManager creates a global build console with the same name. This should be changed so there is one global build console for all the various CBuildConsoles, including the default one. I am providing a proposed patch which fixes this by having a GlobalBuildConsoleManager which is the only one that creates the GlobalBuildConsole and global console partitioner. It doesn't bother with listening for resource changes. Each BuildConsoleManager is passed the GlobalBuildConsoleManager in the startup() method and it uses it to refer to the global console and partitioner where needed. I chose to pass the global console manager rather than to refer to a CUIPlugin public method so that the API would be private in CUIPlugin and in the future, consoles could be grouped globally if desired.