On Wed, Nov 05, 2014 at 04:49:52PM -0500, Ben Boeckel wrote: > On Wed, Nov 05, 2014 at 13:12:13 -0800, Peter Collingbourne wrote: > > I've added documentation and tests and addressed Ben's comments, and > > published > > the staging branch 'console-pool'. PTAL. > > Thanks for splitting the commits. > > For the testing of it, since only Ninja supports it, maybe it would be > possible to run a program which exits testing for isatty(STDIN_FILENO) > or isatty(STDOUT_FILENO).
I considered that, and indeed this is how I tested the console pool feature in Ninja's test suite [1]. (That test disables itself if the test suite itself does not have access to the console.) The problem is that CTest does not give Ninja direct access to the console, so a test like the one Ninja uses wouldn't work, or at least it wouldn't provide coverage if the test suite is invoked in the normal way. I don't know enough about CTest to know if there is some way to get it to pass through console access. > Looking at it again, it strikes me that maybe USES_CONSOLE would be > better since it's more declarative (telling CMake about the command) > than imperative (telling the command it may use the console). > USES_CONSOLE may also be better and generators such as VS might know to > error with "I don't offer a terminal to use" or tools such as Eclipse to > hide these targets from their lists (though how CMake communicates the > requirement is an open question). ...Which makes me think that > MAY_USE_CONSOLE would be useful, but I don't think CMake should be > saddled with the job of telling the command whether a terminal exists or > not (and documented as such). "TERMINAL" rather than "CONSOLE" may also > be more accurate (I feel "console" is being used because that happens to > be the Ninja builtin pool name). > > Thoughts? I would rather keep things simple and have this flag always be opportunistic. Remember that the build tool could be invoked with or without a terminal, so I would expect commands to normally do something sensible if it turns out not to be available. We might want to have a mechanism to control whether targets appear in IDEs, but that seems like it should be a separate feature. I'm happy to change the name to USES_CONSOLE (or USES_TERMINAL, I don't really mind which). Thanks, -- Peter [1] https://github.com/martine/ninja/blob/master/src/subprocess_test.cc#L102 -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
