[cmake-developers] Ninja pools

[Brad King]

On 11/23/2013 09:04 AM, Peter Kümmel wrote:
> OK, I've pushed a patch which adds
> 
> JOB_POOLS
> 
> JOB_POOL_LINK
> JOB_POOL_COMPILE
> 
> CMAKE_JOB_POOL_LINK
> CMAKE_JOB_POOL_COMPILE

Great!  Here are some comments.

Please update the JOB_POOLS parsing to validate each entry instead
of blindly passing the value through to Ninja.  That will catch errors
earlier and allow CMake to do something else with the values later if
we want.  After the find("=") then do something like

 unsigned int v;
 if(sscanf(value, "=%u", &v) == 1)
   ...

The CMAKE_JOB_POOL_(COMPILE|LINK) variables should be implemented
by initializing the corresponding target properties as each target
is created.  See cmTarget::SetMakefile calls to SetPropertyDefault
for the place in the code to do this.  The generator code should
not have to look up the variable values directly.

Please read the cmake-developer.7 manual section on the documentation
cross-referencing syntax and use that to x-ref among the new variables
and properties.  Also in example code like:

+For instance set_property(GLOBAL PROPERTY POOLS "two_jobs=2;ten_jobs=10").

please use a literal block, e.g.

 For instance::

  set_property(GLOBAL PROPERTY POOLS "two_jobs=2;ten_jobs=10")

Note that set_property will combine multiple arguments as a ;-list
anyway so the example could be::

  set_property(GLOBAL PROPERTY POOLS two_jobs=2 ten_jobs=10)

> but without any support for custom commands because it
> is different to the compile/link code changes.
> I think it is cleaner to add an additional patch later.

Agreed.  It is much simpler to do just compile/link first.

Thanks,
-Brad