[cmake-developers] [wip/patch] Expose Ninja console pool feature for custom commands/targets

[Peter Collingbourne]

On Tue, Nov 04, 2014 at 12:53:07AM -0500, Ben Boeckel wrote:
> On Mon, Nov 03, 2014 at 16:22:56 -0800, Peter Collingbourne wrote:
> > This patch exposes the Ninja console pool feature via the add_custom_command
> > and add_custom_target commands. Specifically, it introduces a USE_CONSOLE
> > flag which can be used to communicate to the generator that the command
> > would prefer to use the console. It has no effect on generators other than
> > the Ninja generator.
> > 
> > The patch also causes the feature to be used with the built-in cache editor.
> 
> +1 to the feature. I think there are some bugs open on the tracker for
> this we should ping when appropriate.

There's http://public.kitware.com/Bug/view.php?id=14915 which I will ping
when I get a chance.

> > Docs/tests to come. Any comments on the proposed approach are appreciated.
> 
> See inline below.
> 
> > -    doing_verbatim
> > +    doing_nothing
> 
> A separate commit to rename this would be nice.

Will do.

> > +    else if(copy == "USE_CONSOLE")
> > +      {
> > +      doing = doing_nothing;
> > +      console = true;
> > +      }
> 
> Is there any reason this could not be implemented with a JOB_POOL target
> property? This would allow custom commands to be placed into a pool with
> JOB_POOLS other than the console pool.

I think that any feature which allows custom commands to be placed in arbitrary
pools should be exposed separately from this feature, the reason being that
a build system may choose to implement the features orthogonally.

Having the features be separate is also useful for supporting older versions
of Ninja (1.1 to 1.4) that support the pool feature but not console pools.

> >    cmTarget CreateGlobalTarget(const std::string& name, const char* message,
> >      const cmCustomCommandLines* commandLines,
> > -    std::vector<std::string> depends, const char* workingDir);
> > +    std::vector<std::string> depends, const char* workingDir,
> > +    bool console);
> 
> A separate commit to add this parameter would be nice so that the "make
> current things console'd" is separate from "allow the user to make
> targets console'd". This parameter might not even be necessary with a
> generic JOB_POOL target property.

Will do.

Thanks,
-- 
Peter