Bug 297222 - Allow temporary build arguments from build console
Summary: Allow temporary build arguments from build console
Status: ASSIGNED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-build (show other bugs)
Version: 7.0   Edit
Hardware: PC Linux
: P3 enhancement with 1 vote (vote)
Target Milestone: ---   Edit
Assignee: cdt-build-inbox@eclipse.org CLA
QA Contact: Jonah Graham CLA
URL:
Whiteboard:
Keywords:
Depends on: 320521
Blocks: 304036
  Show dependency tree
 
Reported: 2009-12-08 11:02 EST by Marc Khouzam CLA
Modified: 2020-09-04 15:25 EDT (History)
9 users (show)

See Also:


Attachments
Run build with custom args (26.13 KB, patch)
2009-12-14 09:18 EST, Dmitry Kozlov CLA
no flags Details | Diff
Enabled image (379 bytes, image/gif)
2009-12-17 09:48 EST, Dmitry Kozlov CLA
no flags Details
Disabled image (359 bytes, image/gif)
2009-12-17 09:49 EST, Dmitry Kozlov CLA
no flags Details
Run build with custom args (83.66 KB, patch)
2010-01-20 15:20 EST, Dmitry Kozlov CLA
no flags Details | Diff
Run build with custom args (84.44 KB, patch)
2010-03-26 10:03 EDT, Dmitry Kozlov CLA
no flags Details | Diff
lost control (12.41 KB, image/png)
2010-06-04 14:56 EDT, Andrew Gvozdev CLA
no flags Details
Same patch updated for HEAD (83.34 KB, patch)
2010-07-12 11:56 EDT, Marc Khouzam CLA
marc.khouzam: iplog-
Details | Diff
Run build with custom args - new - moved console to make.ui (357.21 KB, patch)
2010-07-21 06:05 EDT, Dmitry Kozlov CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Khouzam CLA 2009-12-08 11:02:03 EST
This bug is related to bug 294106 and bug 295626.  I'm opening it in an attempt to separate different features from each other, to simplify the approval process.

This idea of this enhancement is to have a way to temporarily add build arguments.  A suggested way to do this is to have an input textbox in the build console that would except one-time build arguments.

The rational behind this feature is:

In large projects, used by many designers, there could be a large number of different options to build the same code; there could also be a large number of pre-defined build-targets (we have 72 different ones!).  This feature would avoid each developer having to check out and create their own versions of the .project and .cproject file to be able to change a couple of settings, or to select a specific build-target.



With a way to specify temporary build arguments or targets, a developer could build something extra (like change a default-setting or choose a specific target) very easily.
Comment 1 Andrew Gvozdev CLA 2009-12-08 11:33:44 EST
(In reply to comment #0)
> In large projects, used by many designers, there could be a large number of
> different options to build the same code; there could also be a large number of
> pre-defined build-targets (we have 72 different ones!). 
Can you give an example of temporary target that you want to type in the textbox? If your 72 targets are anything like ours it would be a long string to type.

> With a way to specify temporary build arguments or targets, a developer could
> build something extra (like change a default-setting or choose a specific
> target) very easily.
Comment 2 Jonas Jonsson CLA 2009-12-09 04:56:18 EST
(In reply to comment #1)
> Can you give an example of temporary target that you want to type in the
> textbox? If your 72 targets are anything like ours it would be a long string to
> type.

OK, the default configuration (we have 18 different to chose from) is that the build target is 'all', then we add six variables to the make-command, example:

make -f Makefile CONFIG=DEBUG TYPE=X CPUTYPE=Y SIM_HW=NO SILENT_MAKE=YES USE_COVERITY=YES

The 18 different default configs comes out of having six different TYPE, and all of these are posssible to build with CONFIG=[DEBUG|RELEASE|PRODUCT].  But then as a developer you might want to speed up the build a little, thus you don't want Coverity to run on each build so you want to set USE_COVERITY=NO.  And perhaps you're building something that doesn't need to run on real HW, then you set SIM_HW=YES. There are of course other build targets than all,clean,clobber as well as other variables that you can play with.

With 50 developers messing with the same code, version handled, you don't want to check out and create private versions of the .cproject & .project files.  We also build different versions (four currently, soon five) of our product where the source code lives in different branches, but we still want to use the same .cproject & .project files, now imagine that each developer must check out at least one of those two files just for changing a 'YES' to a 'NO', we would have thousands of instances of these files in our version control system within a week our two.....  Usability!

Thus there must be a mechanism for adding parameters to the build command that is run by eclipse, may it be 'make' or something else.  The stuff that is entered into the text-box could populate some environment variable that is feed to the what-ever-command-line that actually builds the project.
Comment 3 Dmitry Kozlov CLA 2009-12-14 09:18:27 EST
Created attachment 154405 [details]
Run build with custom args

This patch adds text field to build console toolbar to enable user specify build arguments and couple of buttons: to run build using specified arguments and to stop previously run build. User-specified arguments are transient and never stored in project, so doesn't change existing build configurations.
Comment 4 Dmitry Kozlov CLA 2009-12-17 00:28:09 EST
Andrew, could you pay attention to this patch.
Comment 5 Marc Khouzam CLA 2009-12-17 08:44:58 EST
I've applied the patch and from what I see it looks really nice.

Can you please attach the two icons that you've added as separate attachments? As you saw before, patches don't handle them properly.

I'm not very knowledgeable about the build system of CDT, so this question may not be relevant, but here goes: how to I access the build console (to use the new temp args text box) before I do my very first build?  Currently, I have to build my project, to see the build console.
Comment 6 Dmitry Kozlov CLA 2009-12-17 09:48:21 EST
Created attachment 154663 [details]
Enabled image
Comment 7 Dmitry Kozlov CLA 2009-12-17 09:49:08 EST
Created attachment 154664 [details]
Disabled image
Comment 8 Andrew Gvozdev CLA 2009-12-17 23:15:34 EST
(In reply to comment #4)
> Andrew, could you pay attention to this patch.
Yeah, I am paying attention to what's going on. I already voiced my opinion that console should have "Run" rather than "Build Make Target" command in bugs mentioned in description. Thinking about it as I write this post, it may be appropriate in Make Targets View though. Would you agree with moving it there?

Jonas makes interesting case in comment#2. This case is close to the case developers of our company are facing. Thinking about this use case, the patch solves a problem with not saving transient build commands in VSC. But otherwise I am not sure how handy it is for this case. The patch provides only 5 characters in the input control while Jonas' example needs to type 85. I tried it and the line is hard to edit. After you labored a few of these you can choose them from the drop-down box, yes. It does not quite solve the problem with combinatorial explosion of targets, after a while all used combinations are going to be accumulated in the box. I think there could be some other ideas besides temporary command how to handle that. For example, ability to hide not wanted make targets in Make Targets View. Or enhanced targets with configurable arguments such as CONFIG or USE_COVERITY being set via checkboxes in the bottom part of the view for the selected target. The ideas gravitate to Make Targets View.

(In reply to comment #5)
> I'm not very knowledgeable about the build system of CDT, so this question may
> not be relevant, but here goes: how to I access the build console (to use the
> new temp args text box) before I do my very first build?  Currently, I have to
> build my project, to see the build console.
A good question, perhaps strengthening my point. As far as I know, you have to do a build before you can get the console. Make Targets View is available all the time.
Comment 9 Jonas Jonsson CLA 2009-12-18 04:58:09 EST
(In reply to comment #8)
> ....
> Jonas makes interesting case in comment#2. This case is close to the case
> developers of our company are facing. Thinking about this use case, the patch
> solves a problem with not saving transient build commands in VSC. But otherwise
> I am not sure how handy it is for this case. The patch provides only 5
> characters in the input control while Jonas' example needs to type 85. I tried
> it and the line is hard to edit. After you labored a few of these you can
> choose them from the drop-down box, yes. It does not quite solve the problem
> with combinatorial explosion of targets, after a while all used combinations
> are going to be accumulated in the box. I think there could be some other ideas
> besides temporary command how to handle that. For example, ability to hide not
> wanted make targets in Make Targets View. Or enhanced targets with configurable
> arguments such as CONFIG or USE_COVERITY being set via checkboxes in the bottom
> part of the view for the selected target. The ideas gravitate to Make Targets
> View.

Actually, it's very rare that we must type 85 chars before building.  We set up different default configurations and then call the build-command with parameters from the chosen configuration.  The normal use-cases are handled with the default configurations. If something is added to the build command, it's generally not longer than 30 chars.

BUT: if something is added into the text-box, it should be appended to the build-command that is given in the project configuration.

I can't see any need for check-boxes, and I can imagine that creating a good solution for that would be ... tricky :)
 
> (In reply to comment #5)
> > I'm not very knowledgeable about the build system of CDT, so this question may
> > not be relevant, but here goes: how to I access the build console (to use the
> > new temp args text box) before I do my very first build?  Currently, I have to
> > build my project, to see the build console.
> A good question, perhaps strengthening my point. As far as I know, you have to
> do a build before you can get the console. Make Targets View is available all
> the time.

The 'Make Targets View' doesn't show any targets at all for me, since we're not using 'Makefile' as our primary build-control file on the developer machines. The Makefile is used on the build servers, but we're using 'Makefile.rsync' on our machines (that might be Vista/Linux/Solaris)....  Actually, the 'Make Targets View' is totally unusable for me.  If it would be possible to tell it to show the targets in an other file than 'Makefile', it would perhaps be another story.
Comment 10 Andrew Gvozdev CLA 2009-12-18 10:12:37 EST
(In reply to comment #9)
> The 'Make Targets View' doesn't show any targets at all for me, since we're not
> using 'Makefile' as our primary build-control file on the developer machines.
> The Makefile is used on the build servers, but we're using 'Makefile.rsync' on
> our machines (that might be Vista/Linux/Solaris)....  Actually, the 'Make
> Targets View' is totally unusable for me.  If it would be possible to tell it to
> show the targets in an other file than 'Makefile', it would perhaps be another
> story.
It is pretty flexible to adjust to custom workflow and it does not have to be targets from makefile. We have I think a similar needs. The compilation is always done on the remote box and we run not make targets from makefile but the commands to run make targets on the remote box. The build command in project configuration is set to "ssh remote-box 'make " and when you run "Make Target" in the view corresponding target is appended to it. We got "compile", "cxxtest", "flexelint", "package" targets, all run remotely. The error parsers are set up so any error markers appear in the editor and Problems View for any of compilation, cxxtest or flexelint errors. We got a custom eclipse plugin which generates predefined targets in "Make Targets View" when the wizard creates our project type.
Comment 11 Dmitry Kozlov CLA 2010-01-20 15:19:55 EST
Next version of patch which fixes two things:
- build arguments text field now has border,
- build arguments specified in build console now are added to build arguments, specified in build command in Preferences. Before they were overriding all previously specified arguments.
Comment 12 Dmitry Kozlov CLA 2010-01-20 15:20:56 EST
Created attachment 156702 [details]
Run build with custom args
Comment 13 Doug Schaefer CLA 2010-01-20 16:48:15 EST
What does this look like if the user is using the internal builder. Do these things still show up?
Comment 14 Dmitry Kozlov CLA 2010-01-20 17:16:42 EST
(In reply to comment #13)
> What does this look like if the user is using the internal builder. Do these
> things still show up?
Yes, it is shown, and you can run internal build using Run build button in build console. But parameters are ignored. This is discussabble, if you are going to upstream the patch.

Doug, Andrew Gvozdev is fighting against adding this feature for several months. Can you tell if there is a chance to upstream this patch, and what else should be done to upstream it.
Comment 15 Marc Khouzam CLA 2010-02-18 10:23:52 EST
This bug has been pretty quiet, although we were making good progress.  Anything needed to make this progress?
Comment 16 Dmitry Kozlov CLA 2010-03-04 02:15:27 EST
Guys can we return to this patch and discuss the strategy once more. I'd prefer not to waste my time on patch that never will be applied. So given the problem, Andrew, what are your suggestions how we can apply this patch or implement another which solves stated problem?
Comment 17 Andrew Gvozdev CLA 2010-03-06 14:49:51 EST
OK, yielding to popular demand I'll take this one for a ride. Dmitry, could you resubmit the patch ensuring IP cleanness so all new files have proper license owner, author, etc? I'll send it to IP review in hope we can make it for Helios. Could you also make a statement same as in bug 295625 comment 45?
Comment 18 Andrew Gvozdev CLA 2010-03-06 15:10:40 EST
Wait, there is one problem that must be resolved. The patch introduces dependency from org.eclipse.cdt.ui to org.eclipse.cdt.make.core. We should find a solution which does not.
Comment 19 Marc Khouzam CLA 2010-03-06 19:38:06 EST
(In reply to comment #17)
> OK, yielding to popular demand I'll take this one for a ride

Thanks Andrew.
Comment 20 Doug Schaefer CLA 2010-03-06 22:43:30 EST
(In reply to comment #18)
> Wait, there is one problem that must be resolved. The patch introduces
> dependency from org.eclipse.cdt.ui to org.eclipse.cdt.make.core. We should find
> a solution which does not.

That was my point. You have no idea what build system is being used with build console. And not all build systems allow temporary arguments. The internal builder especially. It will be hard to do without being confusing to someone.
Comment 21 Andrew Gvozdev CLA 2010-03-07 12:29:33 EST
(In reply to comment #20)
> (In reply to comment #18)
> That was my point. You have no idea what build system is being used with build
> console. And not all build systems allow temporary arguments. The internal
> builder especially. It will be hard to do without being confusing to someone.
Ouch, did I just put myself in the position to defend the feature :-Q?
OK, let me spin it this way then - in general a build system allows you to specify build targets one way or another (build targets in more general sense than make targets). Internal builder can take targets "Build", "Clean", "Rebuild", "Build selected Files" which are activated via project context menu. You could as well have a mechanism to pass those from console.

Disclaimer: please don't count me as a supporter or opponent of this feature just yet. I may try to rock the boat left or upside down and will try to take it where I subjectively think it is most usable for the users.
Comment 22 Doug Schaefer CLA 2010-03-07 13:48:01 EST
(In reply to comment #21)
> (In reply to comment #20)
> > (In reply to comment #18)
> > That was my point. You have no idea what build system is being used with build
> > console. And not all build systems allow temporary arguments. The internal
> > builder especially. It will be hard to do without being confusing to someone.
> Ouch, did I just put myself in the position to defend the feature :-Q?
> OK, let me spin it this way then - in general a build system allows you to
> specify build targets one way or another (build targets in more general sense
> than make targets). Internal builder can take targets "Build", "Clean",
> "Rebuild", "Build selected Files" which are activated via project context menu.
> You could as well have a mechanism to pass those from console.

That may be a stretch. But I'll hold judgment until I see what it looks like.

> Disclaimer: please don't count me as a supporter or opponent of this feature
> just yet. I may try to rock the boat left or upside down and will try to take
> it where I subjectively think it is most usable for the users.

That's all I ask :). We need to keep a keen eye on usability. I want to make sure all the features we do can be adopted by all products that build on CDT.
Comment 23 Dmitry Kozlov CLA 2010-03-26 10:03:14 EDT
Created attachment 163079 [details]
Run build with custom args

I'm reposting the patch aligned against fresh HEAD. It is still references make.core and this the thing that we should discuss, but before let's make a common opinion about including functionality provided by this patch into upstream. Or declare some functional modifications which must be done to include this upstream.

Andrew, you asked for confirmation:

I, Dmitry Kozlov, declare that I developed attached code from scratch,
without referencing any 3rd party materials except material licensed under the
EPL. I am authorized by my employer CodeSourcery to make this contribution
under the EPL.
Comment 24 Andrew Gvozdev CLA 2010-05-08 13:40:29 EDT
(In reply to comment #23)
> Created an attachment (id=163079)
> Run build with custom args
> I'm reposting the patch aligned against fresh HEAD. It is still references
> make.core and this the thing that we should discuss, but before let's make a
> common opinion about including functionality provided by this patch into
> upstream. Or declare some functional modifications which must be done to include
> this upstream.
After some more thinking about it, I came to agree with the approach in general. I still think that console should employ "Run command" (bug 295626) but "Build command" is important special case and that special case should be implemented in terms of build component - which the patch does.
Regarding Internal builder, it is not a fault of the patch that it does not work properly. This patch uses mechanism of Make Targets and Make Targets do not work with Internal builder properly which is a different issue bug 304774. There just should be a way to pass parameters (those it can take, like "Build Selected Files") to Internal builder in Make Targets.

But the patch cannot be accepted because of the dependency from cdt.ui to cdt.make.core plugin. This must be resolved. I suggest to extract and move build console to build component where it belongs. Formally it is internal package now in cdt.ui. The appropriate location would be a common (or core) build component, it could probably be Doug's new build plugin in CDT 8.0.  Before it is available we could place the build console to cdt.make.ui temporarily - to test and discuss the approach.
Comment 25 Dmitry Kozlov CLA 2010-05-20 04:28:27 EDT
(In reply to comment #24)
> (In reply to comment #23)
> > Created an attachment (id=163079) [details]
> > Run build with custom args
> > I'm reposting the patch aligned against fresh HEAD. It is still references
> > make.core and this the thing that we should discuss, but before let's make a
> > common opinion about including functionality provided by this patch into
> > upstream. Or declare some functional modifications which must be done to include
> > this upstream.
> After some more thinking about it, I came to agree with the approach in
> general.
> But the patch cannot be accepted because of the dependency from cdt.ui to
> cdt.make.core plugin.
> I suggest to extract and move
> build console to build component where it belongs.
Can you guarantee that after moving build console this patch will be accepted?
Comment 26 Andrew Gvozdev CLA 2010-05-20 09:51:27 EDT
(In reply to comment #25)
> (In reply to comment #24)
> > (In reply to comment #23)
> > > Created an attachment (id=163079) [details]
> > > Run build with custom args
> > > I'm reposting the patch aligned against fresh HEAD. It is still references
> > > make.core and this the thing that we should discuss, but before let's make a
> > > common opinion about including functionality provided by this patch into
> > > upstream. Or declare some functional modifications which must be done to
> include
> > > this upstream.
> > After some more thinking about it, I came to agree with the approach in
> > general.
> > But the patch cannot be accepted because of the dependency from cdt.ui to
> > cdt.make.core plugin.
> > I suggest to extract and move
> > build console to build component where it belongs.
> Can you guarantee that after moving build console this patch will be accepted?
I can promise my support for the patch. There seem to be different opinions where build console should reside and we'll have to make the case. The patch would strengthen the case considerably I think.
On the other hand, I can guarantee that the patch won't be accepted having this dependency.
Comment 27 Dmitry Kozlov CLA 2010-05-26 02:46:15 EDT
(In reply to comment #26)
> On the other hand, I can guarantee that the patch won't be accepted having this
> dependency.
Could you perform more detailed review of this patch and say if this dependency is the only problem that should be fixed.
Comment 28 Andrew Gvozdev CLA 2010-06-04 14:56:31 EDT
Created attachment 171149 [details]
lost control

(In reply to comment #27)
> Could you perform more detailed review of this patch and say if this dependency
> is the only problem that should be fixed.

Alright, here is more detailed review.

- Dependency from org.eclipse.cdt.ui to org.eclipse.cdt.make.core. Stating it here for completeness.
- After opening workspace the command window is not available. I think it is important use case for this functionality but it should be probably addressed as a separate task.

- After the first build, the drop-down gets populated with the make targets from the project. This list stays there even if I switch to a different project with a different set of targets.
- The drop-down box is filled with existing make targets from Make Target view. However, there is an inconsistency as the list is not updated when targets added/removed.
- Make Targets from MTV using custom build command should be treated in special way. If I select such one from drop-down menu a wrong build command is run.
As a note, connection to targets from Make Target view could cause quite a few issues. I likely haven't spotted all of them yet. You may be better off putting it off for another separate patch.

- The icon represents wrong metaphor. It should be "Build" metaphor (the hammer). I think "Build Make Target" icon from Make Targets view represents best what is happening here.
- "Stop project build" works only with console-initiated builds, does not work with regular one or with MTV.
- While another build is running "Build..." - starting another build will put the job in queue background but there is no cue for a user. It just looks like nothing happens. Note that I tested with older CDT version, I believe the build queue is broken on current CDT HEAD.
- AFAICS the command will run all builders each time (i.e. invoke scanner discovery command in the shell), is it how it is intended to be?
- It does not work with internal builder as Doug pointed out but that's bug 304774.

- Ctrl-A, Ctrl-C and some others in the input control are applied to text from console. We positively need common keys such as Ctrl-C, Ctrl-V working properly in the control.
- While testing with multiple consoles I had a case when the drop-down box lost its drop-down control [v]. It looks like the input control got way too long. When I type very long string the cursor goes outside of visible area. Can't reproduce it though.
- Again, while playing with multiple consoles sometimes the build opens a second console spontaneously. Hmm, same happens with MTV and regular build. Nevermind.

- Attempt to run while non-CDT project is selected leads to an exception:
java.lang.ArrayIndexOutOfBoundsException: 0
at org.eclipse.cdt.internal.ui.buildconsole.RunBuildAction.run(RunBuildAction.java:62)
at org.eclipse.jface.action.Action.runWithEvent(Action.java:498)

From the initial look at the code:
- Do we have it in console preferences anymore? ConsolePreferencePage.consoleBuildToolbar.label is still there in the code.
- BuildConsoleCCombo looks like something that might belong to Platform. Did you try to submit it there? If they stall, it should probably go to one of packages in org.eclipse.cdt.internal.ui eventually. But while we work on the patch - let's keep it contained here for the time being.
- All words in tooltips should be capitalized per UI guidelines. The tooltips are a bit too verbose for my taste.
- Compilation warnings, please
- TargetBuild copy will need to be addressed after we find home for build console package.

Thanks
Comment 29 Marc Khouzam CLA 2010-06-17 15:46:24 EDT
We noticed that in the last posted patch, in TargetBuild.java, the line

Job targetJob = new Job("TargetBuild.backgroundTask.name") { //$NON-NLS-1$

should be something in the line of

Job targetJob = new Job(MakeUIPlugin.getResourceString("TargetBuild.backgroundTask.name")) { //$NON-NLS-1$
Comment 30 Marc Khouzam CLA 2010-06-17 15:54:46 EDT
(In reply to comment #29)
> We noticed that in the last posted patch, in TargetBuild.java, the line
> 
> Job targetJob = new Job("TargetBuild.backgroundTask.name") { //$NON-NLS-1$
> 
> should be something in the line of
> 
> Job targetJob = new
> Job(MakeUIPlugin.getResourceString("TargetBuild.backgroundTask.name")) {
> //$NON-NLS-1$

This one too, also in org.eclipse.cdt.internal.ui.buildconsole.TargetBuild.java:
				monitor.beginTask("TargetBuild.monitor.beginTask", targets.length); //$NON-NLS-1$
Comment 31 Marc Khouzam CLA 2010-07-12 11:56:43 EDT
Created attachment 174056 [details]
Same patch updated for HEAD

I've had to apply this patch so I've updated it for HEAD.
I don't know the code, so I didn't verify the validity of the changes, I simply applied it and resolved a couple of conflicts.

I've also hacked the issue reported in comment #29 and comment #30, which should eventually be properly fixed.
Comment 32 Marc Khouzam CLA 2010-07-12 11:58:01 EDT
(In reply to comment #31)
> Created an attachment (id=174056) [details]
> Same patch updated for HEAD
> 
> I've had to apply this patch so I've updated it for HEAD.

Just in case: I meant that I needed to apply it to my own workspace.  I didn't commit anything of course.
Comment 33 Dmitry Kozlov CLA 2010-07-21 06:05:21 EDT
Created attachment 174829 [details]
Run build with custom args - new - moved console to make.ui
Comment 34 Dmitry Kozlov CLA 2010-07-21 06:11:41 EDT
> - Dependency from org.eclipse.cdt.ui to org.eclipse.cdt.make.core. Stating it
> here for completeness.
Resolved

> - After opening workspace the command window is not available. I think it is
> important use case for this functionality but it should be probably addressed
> as a separate task.
I agree that this is a separate task. Not addressed in this patch.

> - After the first build, the drop-down gets populated with the make targets
> from the project. This list stays there even if I switch to a different project
> with a different set of targets.
Resolved.
> - The drop-down box is filled with existing make targets from Make Target view.
> However, there is an inconsistency as the list is not updated when targets
> added/removed.
Resolved

> - Make Targets from MTV using custom build command should be treated in special
> way. If I select such one from drop-down menu a wrong build command is run.
> As a note, connection to targets from Make Target view could cause quite a few
> issues. I likely haven't spotted all of them yet. You may be better off putting
> it off for another separate patch.
Agree. Not in this patch.

> - The icon represents wrong metaphor. It should be "Build" metaphor (the
> hammer). I think "Build Make Target" icon from Make Targets view represents
> best what is happening here.
Fixed.

> - "Stop project build" works only with console-initiated builds, does not work
> with regular one or with MTV.
This is separate bug (enhancement request) somewhere in bugzilla. While discussing this I desided that it is too tricky to do it because Project.build doesn't return Job, thus to catch it we need to monitor all jobs and find build job to cancel it. I suggest we don't mixin this to this patch.

> - While another build is running "Build..." - starting another build will put
> the job in queue background but there is no cue for a user. It just looks like
> nothing happens. Note that I tested with older CDT version, I believe the build
> queue is broken on current CDT HEAD.
I suggest not to mix build queue improvements and this patch. KISS

> - AFAICS the command will run all builders each time (i.e. invoke scanner
> discovery command in the shell), is it how it is intended to be?
Set runAllBuilders to false.

> - It does not work with internal builder as Doug pointed out but that's bug
> 304774.
As Daniel Jacobowitz pointed recently in cdt-dev internal builder is broken in a lot of another ways. I suggest to separate internal builder fixes from this patch.
 
> - Ctrl-A, Ctrl-C and some others in the input control are applied to text from
> console. We positively need common keys such as Ctrl-C, Ctrl-V working properly
> in the control.
Ctrl-C, Ctrl-V definitely works.

> - While testing with multiple consoles I had a case when the drop-down box lost
> its drop-down control [v]. It looks like the input control got way too long.
> When I type very long string the cursor goes outside of visible area. Can't
> reproduce it though.
> - Again, while playing with multiple consoles sometimes the build opens a
> second console spontaneously. Hmm, same happens with MTV and regular build.
> Nevermind.
I don't.

 
> - Attempt to run while non-CDT project is selected leads to an exception:
> java.lang.ArrayIndexOutOfBoundsException: 0
> at
> org.eclipse.cdt.internal.ui.buildconsole.RunBuildAction.run(RunBuildAction.java:62)
> at org.eclipse.jface.action.Action.runWithEvent(Action.java:498)
Fixed.
> - Do we have it in console preferences anymore?
> ConsolePreferencePage.consoleBuildToolbar.label is still there in the code.
We don't. Cleaned up.

> - BuildConsoleCCombo looks like something that might belong to Platform. Did
> you try to submit it there? If they stall, it should probably go to one of
> packages in org.eclipse.cdt.internal.ui eventually. But while we work on the
> patch - let's keep it contained here for the time being.
Kept. Didn't submit upstream. Marc says there is no such problem (why custom components was created) on Vista.

> - All words in tooltips should be capitalized per UI guidelines. The tooltips
> are a bit too verbose for my taste.
Resolved both: capitalized, shorter.

> - Compilation warnings, please
Resolved as much as possible.

> - TargetBuild copy will need to be addressed after we find home for build
> console package.
Resolved.
Comment 35 Marc Khouzam CLA 2010-07-27 14:14:40 EDT
I'm hoping that with Bug 320765, we will be able to open the build console before doing a build.  This is important for the feature of the current bug because we should be able to add arguments before having to do a build.

The problem is that with Bug 320765, it is possible to see the build console when no project has been selected.  The patch of this bug is not prepared for that.

I suggest disabling the new text box and the new run button, until a project is first selected.  Once the project is selected by the user, we can enable both, and everything should be fine.
Comment 36 Dmitry Kozlov CLA 2010-08-30 08:16:27 EDT
Andrew, have you give up with this issue? I see that the only reply to latest patch is from Marc. Is it ok to commit this patch and then fix what Marc talk about in his comment?
Comment 37 Marc Khouzam CLA 2010-08-30 08:43:49 EDT
(In reply to comment #36)
> Andrew, have you give up with this issue? I see that the only reply to latest
> patch is from Marc. Is it ok to commit this patch and then fix what Marc talk
> about in his comment?

The fix for my issue is a one-liner that adds:

if ( fProject == null ) return;

in RunBuildAction#run() right after we check for
if ( targetManager == null ) return;
Comment 38 Andrew Gvozdev CLA 2010-08-30 10:58:41 EDT
(In reply to comment #36)
> Andrew, have you give up with this issue? I see that the only reply to latest
> patch is from Marc. Is it ok to commit this patch and then fix what Marc talk
> about in his comment?
This patch requires an agreement to move console from cdt.ui to cdt.make.ui (bug 320521). That requires an agreement from CDT development team (at least no veto) to proceed. I've compiled relevant responses from cdt-dev mailing and posted in bug 320521. There is disagreement from Doug on the proposal so we cannot move with it. Also, there were some objections from the list regarding this bug:

On Thu, Jul 22, 2010 at 9:28 AM, <Warren.Paul@nokia.com> wrote:
> > There is a concern about the new proposed functionality.  It?s not very generic and won?t work with any builder, so I don?t think it should be added to the generic build console.  At least not without more checks in place.  For example, if the project in context has this build nature then show the new UI, otherwise hide it.

On Thu, Jul 22, 2010 at 12:46 PM, Doug Schaefer <cdtdoug@gmail.com> wrote:
> This has always been and still is may main issue with this feature.
> I'm getting really tired of things in the Eclipse UI that aren't
> applicable to the workflows I'm using. I do not want to see this UI if
> I'm not using a builder that can use it. Either make it optional with
> the default being off, or make it look at the current builders to see
> if it makes sense.
> > Just my $.02.
> I'm going to put more that $.02 on this. We need to keep the CDT UI
> clean and I will stand up to ensure we fix usability issues and not
> introduce them.

If you still want to move with this request, maybe some other options could be explored, for example as an idea making console extendible which would allow to extend it in make.ui plugin.
Comment 39 Andrew Gvozdev CLA 2011-03-15 09:40:45 EDT
returning to the pool
Comment 40 Marc Khouzam CLA 2011-03-15 09:58:02 EDT
(In reply to comment #39)
> returning to the pool

I am still hoping to eventually get this feature into CDT (probably after Indigo).  Would it be possible to discuss what could be done to make this solution proper for CDT?
Comment 41 Andrew Gvozdev CLA 2011-03-15 10:08:41 EDT
It should always be possible to discuss more. I posted my take on it and Doug expressed his. We need to find a solution acceptable for the different workflows that are in use and inside the constraints imposed by plugin dependencies.
Comment 42 Marc Khouzam CLA 2011-10-04 14:38:36 EDT
(In reply to comment #41)
> It should always be possible to discuss more. I posted my take on it and Doug
> expressed his. We need to find a solution acceptable for the different
> workflows that are in use and inside the constraints imposed by plugin
> dependencies.

Trying to figure out what can be done to make the "Temporary build arguments in the build console" feature acceptable for open-source.

Looking at the email exchange:
http://dev.eclipse.org/mhonarc/lists/cdt-dev/msg19921.html

is it correct to say that the issue is that the new UI (e.g., the temp arg box) is not applicable to all build systems and therefore should not always be visible?

If that is the issue, would it be sufficient to choose a way to make that UI only visible when relevant, or when selected explicitly by the user?  If we enhance the patch to have this, will it be good for stock CDT?
Comment 43 Marc Khouzam CLA 2013-07-09 10:52:49 EDT
I've pushed Dmitry's feature to gerrit to allow for easier tracking:
https://git.eclipse.org/r/14407
Comment 44 Marc Khouzam CLA 2013-07-09 10:58:06 EDT
(In reply to comment #43)
> I've pushed Dmitry's feature to gerrit to allow for easier tracking:
> https://git.eclipse.org/r/14407

I've pushed a small update in a second patchset to fix a couple of bugs we found along the way.