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

2014-11-14 Thread Peter Collingbourne
On Wed, Nov 05, 2014 at 06:35:25PM -0500, Ben Boeckel wrote:
> On Wed, Nov 05, 2014 at 15:03:02 -0800, Peter Collingbourne wrote:
> > On Wed, Nov 05, 2014 at 04:49:52PM -0500, Ben Boeckel wrote:
> > > 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.
> 
> Hrm. Forgot about that :( . The best I can think of is to have certain
> dashboards run it separately, but that means we still need some way to
> get it to the dashboard and for it *not* to run on the other dashboards
> as a always-fail (maybe WILL_FAIL TRUE to make sure it actually
> *detects* it?). Anyways, I wouldn't worry about it too much due to the
> many issues.
> 
> > 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.
> 
> Good point. It was more to get the ideas out in the air than a formal
> request for change.

Okay, I've renamed the new flag to 'USES_TERMINAL' and merged to next.

Thanks,
-- 
Peter
-- 

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


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

2014-11-05 Thread Peter Collingbourne
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


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

2014-11-05 Thread Peter Collingbourne
On Mon, Nov 03, 2014 at 04:22:56PM -0800, Peter Collingbourne wrote:
> Hi all,
> 
> 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.

I've added documentation and tests and addressed Ben's comments, and published
the staging branch 'console-pool'. PTAL.

Thanks,
-- 
Peter
-- 

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


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

2014-11-04 Thread 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 depends, const char* workingDir);
> > +std::vector 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
-- 

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


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

2014-11-03 Thread Peter Collingbourne
Hi all,

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.

Docs/tests to come. Any comments on the proposed approach are appreciated.

Thanks,
-- 
Peter
>From eaebc04718f1a96c59ac03d21ebb4bb95db4475f Mon Sep 17 00:00:00 2001
From: Peter Collingbourne 
Date: Tue, 4 Nov 2014 00:05:51 +0100
Subject: [PATCH] console pools

---
 Source/cmAddCustomCommandCommand.cxx |  9 +++--
 Source/cmAddCustomTargetCommand.cxx  | 13 ++---
 Source/cmCustomCommand.cxx   | 16 +++-
 Source/cmCustomCommand.h |  6 ++
 Source/cmGlobalGenerator.cxx | 25 +
 Source/cmGlobalGenerator.h   |  3 ++-
 Source/cmGlobalNinjaGenerator.cxx| 16 +---
 Source/cmGlobalNinjaGenerator.h  |  4 
 Source/cmLocalNinjaGenerator.cxx |  1 +
 Source/cmMakefile.cxx| 18 --
 Source/cmMakefile.h  | 12 
 Source/cmNinjaTargetGenerator.h  |  1 -
 Source/cmNinjaUtilityTargetGenerator.cxx |  5 +
 13 files changed, 96 insertions(+), 33 deletions(-)

diff --git a/Source/cmAddCustomCommandCommand.cxx b/Source/cmAddCustomCommandCommand.cxx
index 2d19610..e035195 100644
--- a/Source/cmAddCustomCommandCommand.cxx
+++ b/Source/cmAddCustomCommandCommand.cxx
@@ -35,6 +35,7 @@ bool cmAddCustomCommandCommand
   std::vector depends, outputs, output;
   bool verbatim = false;
   bool append = false;
+  bool console = false;
   std::string implicit_depends_lang;
   cmCustomCommand::ImplicitDependsList implicit_depends;
 
@@ -102,6 +103,10 @@ bool cmAddCustomCommandCommand
   {
   append = true;
   }
+else if(copy == "USE_CONSOLE")
+  {
+  console = true;
+  }
 else if(copy == "TARGET")
   {
   doing = doing_target;
@@ -312,7 +317,7 @@ bool cmAddCustomCommandCommand
 this->Makefile->AddCustomCommandToTarget(target, no_depends,
  commandLines, cctype,
  comment, working.c_str(),
- escapeOldStyle);
+ escapeOldStyle, console);
 }
   else if(target.empty())
 {
@@ -321,7 +326,7 @@ bool cmAddCustomCommandCommand
  main_dependency,
  commandLines, comment,
  working.c_str(), false,
- escapeOldStyle);
+ escapeOldStyle, console);
 
 // Add implicit dependency scanning requests if any were given.
 if(!implicit_depends.empty())
diff --git a/Source/cmAddCustomTargetCommand.cxx b/Source/cmAddCustomTargetCommand.cxx
index 3235502..2d4ad48 100644
--- a/Source/cmAddCustomTargetCommand.cxx
+++ b/Source/cmAddCustomTargetCommand.cxx
@@ -48,6 +48,7 @@ bool cmAddCustomTargetCommand
   std::vector depends;
   std::string working_directory;
   bool verbatim = false;
+  bool console = false;
   std::string comment_buffer;
   const char* comment = 0;
   std::vector sources;
@@ -59,7 +60,7 @@ bool cmAddCustomTargetCommand
 doing_working_directory,
 doing_comment,
 doing_source,
-doing_verbatim
+doing_nothing
   };
   tdoing doing = doing_command;
 
@@ -90,9 +91,14 @@ bool cmAddCustomTargetCommand
   }
 else if(copy == "VERBATIM")
   {
-  doing = doing_verbatim;
+  doing = doing_nothing;
   verbatim = true;
   }
+else if(copy == "USE_CONSOLE")
+  {
+  doing = doing_nothing;
+  console = true;
+  }
 else if (copy == "COMMENT")
   {
   doing = doing_comment;
@@ -226,7 +232,8 @@ bool cmAddCustomTargetCommand
   cmTarget* target =
 this->Makefile->AddUtilityCommand(targetName, excludeFromAll,
   working_directory.c_str(), depends,
-  commandLines, escapeOldStyle, comment);
+  commandLines, escapeOldStyle, comment,
+  console);
 
   // Add additional user-specified source files to the target.
   target->AddSources(sources);
diff --git a/Source/cmCustomCommand.cxx b/Source/cmCustomCommand.cxx
index c161eb6..46d3cf5 100644
--- a/Source/cmCustomCommand.cxx
+++ b/Source/cmCustomCommand.cxx
@@ -34,7 +34,8 @@ cmCustomCommand::cmCustomCommand(const cmCustomCommand& r):
   WorkingDirectory(r.WorkingDire

Re: [cmake-developers] -GNinja on Windows

2012-06-13 Thread Peter Collingbourne
On Wed, Jun 13, 2012 at 05:34:14PM +0200, Peter Kümmel wrote:
> On 13.06.2012 15:31, Amine Khaldi wrote:
>>
>> Please don't consider Windows as "done" until proper dependency tracking
>> is in place (it's lacking so far for rc files).
>>
>
> Please test stage. I've added rc file dependency tracking.
> It's a bit "brute force" because I feed cl.exe with the
> .rc file and collect the included files.
>
> This works as long as all #includes are done before
> cl.exe stops because of rc specific stuff.
>
> I don't know if cmake could do it better.

You could invoke cl.exe in preprocessor-only mode (/P /DRC_INVOKED),
and then supply its output to rc.exe.   This also prevents cl.exe
from uselessly trying to compile the resource file as a C program.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Review request: Ninja-EXPORT_COMPILE_COMMANDS

2012-05-16 Thread Peter Collingbourne
On Sun, May 13, 2012 at 11:59:20PM +0200, Stephen Kelly wrote:
> Stephen Kelly wrote:
> 
> >>
> >> Furthermore, there is no need to make paths within the build directory
> >> absolute.  Each command must be invoked from the home output directory
> >> (i.e. the build "root" directory), so each command's "directory"
> >> attribute should be set to that directory, rather than the start
> >> output directory as in your patch.
> > 
> > Fixed now, If I understood you correctly.
> 
> I've pushed this to the CMake stage repo now.
> 
> Peter, could you verify that I should always use the same directory for the 
> directory JSON property as I do in the patch?

Yes, this looks fine now.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Review request: Ninja-EXPORT_COMPILE_COMMANDS

2012-05-09 Thread Peter Collingbourne
On Sun, May 06, 2012 at 03:20:59PM +0200, Stephen Kelly wrote:
> 
> Hi, 
> 
> I've pushed a new branch to my clone for review to make the 
> CMAKE_EXPORT_COMPILE_COMMANDS option work with the Ninja generator.
> 
> https://gitorious.org/~steveire/cmake/steveires-cmake
> 
> Thanks,
> 
> Steve.

Hi Steve,

Thanks for working on this. 

I think the main problem with your changes is that they do not respect
the CMAKE_*_COMPILE_OBJECT variable, so the output will be wrong for
non-C++ compilers or compilers that do not take the conventional set
of command line parameters.

Furthermore, there is no need to make paths within the build directory
absolute.  Each command must be invoked from the home output directory
(i.e. the build "root" directory), so each command's "directory"
attribute should be set to that directory, rather than the start
output directory as in your patch.

I am not sure if this is the best approach for building JSON files
from Ninja CMake builds.  A few weeks ago I spoke with the folks at
Google who are working on Clang tooling, and I believe the current
thinking is that it may be best to teach Ninja to output JSON files
based on the provided build.ninja files.  The advantage of this is
that it will work for any build system with a Ninja generator, and
there would be no need to add functionality to CMake which may impose
an additional maintenance burden and get out of sync with the
"real" build commands.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] ninja status

2012-04-07 Thread Peter Collingbourne
On Sat, Apr 07, 2012 at 10:00:41PM +0200, Peter Kümmel wrote:
> On 07.04.2012 21:56, Peter Kümmel wrote:
>>
>> By default Ninja support is not enabled on Windows and Mac, somehow it was 
>> build
>> the last times but I touched the cmake code there and introduced a FORCE:
>> http://cmake.org/gitweb?p=stage/cmake.git;a=commitdiff;h=f93e81858b5e1243714ed7f26aadfc791a7b0ff0
>
> I wonder how Ninja was enabled on the build server? Manually by editing 
> CMakeCache.txt?

I believe they just use -DCMAKE_ENABLE_NINJA=ON.  I don't see why
your change is necessary at all, please revert it.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] ninja command line options ?

2012-04-03 Thread Peter Collingbourne
On Tue, Apr 03, 2012 at 08:02:38PM +0200, Alexander Neundorf wrote:
> Hi,
> 
> I'm thinking about giving ninja + Eclipse a try.
> Short question:
> * how do I tell ninja to build a specific target ?

ninja $name_of_target

> * is there a special way to "clean" ?

ninja -t clean

> * how do I tell ninja to use multiple threads/processes (like -j for make) ?

-j, but the default -j value is the number of cores in your machine
plus one.

> * is the syntax the same for Windows and Linux ?

Yes.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja: Recognize and in CMAKE_DEPFILE_FLAGS_${lang}

2012-03-25 Thread Peter Collingbourne
On Fri, Mar 23, 2012 at 08:52:42AM -0400, Brad King wrote:
> On 3/22/2012 12:23 PM, Amine Khaldi wrote:
>> I've attached a patch that allows us (ReactOS) ... to properly track 
>> dependencies in rc files.
>
> Peter, as maintainer of the Ninja generator how does this look
> to you?

I think this is fine, as long as we stay consistent with the set of
substitutions offered by ExpandRuleVariables so we have the option
of switching to that later.  Committed at 6b5614f together with a
modified version of the change suggested by Alexander Usov to work
around the incorrect handing of depfile options by distcc.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] depend problem on windows

2012-03-17 Thread Peter Collingbourne
On Thu, Mar 15, 2012 at 10:24:49AM -0400, Bill Hoffman wrote:
> So, I did a git branch switch to next and ran ninja on my build tree. It 
> correctly re-ran cmake.  But I found a link error building cmake-gui:
>
>
> CMakeLib.lib(cmLocalVisualStudio10Generator.cxx.obj) : error LNK2001:  
> unresolved external symbol "public: virtual void __thiscall  
> cmLocalVisualStudio7Generator::GetTargetObjectFileDirectories(class  
> cmTarget *,class std::vector std::char_traits,class std::allocator >,class  
> std::allocator std::char_traits,class std::allocator > > > &)"  
> (?GetTargetObjectFileDirectories@cmLocalVisualStudio7Generator@@UAEXPAVcmTarget@@AAV?$vector@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@V?$allocator@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@2@@std@@@Z)
> bin\cmake-gui.exe : fatal error LNK1120: 1 unresolved externals
> LINK Pass 1 failed. with 2
>
>
> I did a ninja -t clean, and then ninja and everything worked.  So, it  
> looks like some of the object files did not rebuild correctly after the  
> branch switch.  Same source tree worked with a gmake build tree, so  
> there were no odd file time things going on.

Most likely some object files were not rebuilt as a result of header
file changes.  This will happen if the compiler does not support
emitting dependency files, such as cl.exe.  I think there are a couple
of ways we could consider supporting cl.exe here:

1) Integrate CMake's built in dependency scanner.  This will require
   some thought because the dependency scanner works on a per-target
   basis and Ninja reads dependency files on a per-file basis.

2) Come up with a way to use cl.exe's /showIncludes option to build a
   dependency list.  There has been some discussion of this topic on
   the Ninja mailing list:
   
https://groups.google.com/group/ninja-build/search?group=ninja-build&q=showincludes&qt_g=Search+this+group

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja rebuild_cache fails with spaces in the path (windows and linux)

2012-03-17 Thread Peter Collingbourne
On Wed, Mar 14, 2012 at 11:03:09AM -0400, Bill Hoffman wrote:
> $ ninja rebuild_cache
> [1/1] Running CMake to regenerate build system...
> FAILED: cmd.exe /c cd "C:\Users\hoffman\Work\My Builds\cmake-ninja" &&  
> "C:\Users\hoffman\Work\My Builds\cmake-gmake\bin\cmake.exe"  
> -HC:/Users/hoffman/Work/My Builds/cmake -BC:/Users/hoffman/Work/My  
> Builds/cmake-ninja
>
> CMake Error: The source directory "C:/Users/hoffman/Work/My  
> Builds/cmake-ninja/Builds/cmake-ninja" does not exist.
> Specify --help for usage, or press the help button on the CMake GUI.
> ninja: build stopped: subcommand failed.
>
>
> This also fails on linux.

Fixed as of 60bec54ef644e5c85578ec4a9b24dd2b2cb11e2b.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] ninja bug on windows

2012-03-17 Thread Peter Collingbourne
On Wed, Mar 14, 2012 at 03:43:53PM +, Peter Collingbourne wrote:
> On Wed, Mar 14, 2012 at 10:52:22AM -0400, Bill Hoffman wrote:
> > On 3/13/2012 10:39 PM, Peter Collingbourne wrote:
> >> Maybe another time we can think about ways to improve PCH support,
> >> but for now I guess we can add an OR Ninja to this test and add
> >> OBJECT_OUTPUTS support to the Ninja generator.
> > I tried making that blog active for ninja and a I get a new error:
> >
> >
> > 122: ninja: ERROR: 'PCH\foo_precompiled.pch', needed by  
> > 'CMakeFiles\foo.dir\foo1.c.obj', missing and no known rule to make it
> >
> >
> > So, Ninja is not handling OBJECT_OUTPUTS and OBJECT_DEPENDS correctly?
> >
> >
> >   SET_SOURCE_FILES_PROPERTIES(foo_precompile.c PROPERTIES
> > OBJECT_OUTPUTS "${PCH_DIR}/foo_precompiled.pch")
> >
> >   # These source files use the precompiled header.
> >   SET_SOURCE_FILES_PROPERTIES(${foo_SRCS} PROPERTIES
> > OBJECT_DEPENDS "${PCH_DIR}/foo_precompiled.pch")
> 
> The Ninja generator is handling OBJECT_DEPENDS correctly but not
> OBJECT_OUTPUTS.  Adding support shouldn't be too difficult -- we
> just need to output a phony build statement creating a dependency
> from any OBJECT_OUTPUTS to the object file.

Fixed as of commit 9077adfc528957a929b19f0375752a46ec74ceb9.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] new ninja rebuild too much issue

2012-03-14 Thread Peter Collingbourne
On Wed, Mar 14, 2012 at 10:45:39AM -0400, Bill Hoffman wrote:
> On 3/14/2012 7:34 AM, Nicolas Desprès wrote:
>> Nope if you like in the generated build.ninja "build.ninja" appears
>> only once as an output using the RERUN_CMAKE rule.
>>
>> I rather think it is a bug in ninja of the restat implementation.
> What is the way to debug this?  Can you get ninja to tell you why it is  
> choosing to do a build of a file?  This is quite reproducible.

Most likely, this is due to the compile command line changing between
runs.  If any command line changes, Ninja will need to re-run that
command.  In particular, toggling CMAKE_ENABLE_NINJA would affect
whether CMAKE_USE_NINJA is defined on the command line for anything
under Source.  Ninja stores previously run commands in .ninja_log so
you can examine that to verify that the command line is changing.

We can probably reduce the amount of recompilation required by only
defining CMAKE_ENABLE_NINJA for cmake.cxx (the only source file that
uses it).

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] ninja bug on windows

2012-03-14 Thread Peter Collingbourne
On Wed, Mar 14, 2012 at 10:52:22AM -0400, Bill Hoffman wrote:
> On 3/13/2012 10:39 PM, Peter Collingbourne wrote:
>> Maybe another time we can think about ways to improve PCH support,
>> but for now I guess we can add an OR Ninja to this test and add
>> OBJECT_OUTPUTS support to the Ninja generator.
> I tried making that blog active for ninja and a I get a new error:
>
>
> 122: ninja: ERROR: 'PCH\foo_precompiled.pch', needed by  
> 'CMakeFiles\foo.dir\foo1.c.obj', missing and no known rule to make it
>
>
> So, Ninja is not handling OBJECT_OUTPUTS and OBJECT_DEPENDS correctly?
>
>
>   SET_SOURCE_FILES_PROPERTIES(foo_precompile.c PROPERTIES
> OBJECT_OUTPUTS "${PCH_DIR}/foo_precompiled.pch")
>
>   # These source files use the precompiled header.
>   SET_SOURCE_FILES_PROPERTIES(${foo_SRCS} PROPERTIES
> OBJECT_DEPENDS "${PCH_DIR}/foo_precompiled.pch")

The Ninja generator is handling OBJECT_DEPENDS correctly but not
OBJECT_OUTPUTS.  Adding support shouldn't be too difficult -- we
just need to output a phony build statement creating a dependency
from any OBJECT_OUTPUTS to the object file.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] ninja bug on windows

2012-03-13 Thread Peter Collingbourne
On Tue, Mar 13, 2012 at 07:08:31PM -0400, Bill Hoffman wrote:
> PrecompiledHeader - This one seems to be a dependency problem.
>
> The first time you run it you get this:
>
> 122: Run Build Command:C:/cygwin/bin/ninja.exe
> [3/4] Building C object CMakeFiles\foo.dir\foo1.c.obj
> 122: FAILED: C:\PROGRA~2\MICROS~1.0\VC\bin\cl.exe   /nologo /DWIN32  
> /D_WINDOWS /W3 /Zm1000 /D_DEBUG /MDd /Zi  /Ob0 /Od /RTC1  
> /Yufoo_precompiled.h /FIfoo_precompiled.h "/FpC:/Users/hoffman/Work/My  
> Builds/cmake-ninja/Tests/PrecompiledHeader/PCH/foo_precompiled.pch"  
> /FoCMakeFiles\foo.dir\foo1.c.obj /Fdfoo.pdb -c "C:\Users\hoffman\Work\My  
> Builds\cmake\Tests\PrecompiledHeader\foo1.c"
> 122:
> 122: foo1.c
> 122: C:\Users\hoffman\Work\My  
> Builds\cmake\Tests\PrecompiledHeader\foo1.c : fatal error C1083: Cannot  
> open precompiled header file: 'C:/Users/hoffman/Work/My  
> Builds/cmake-ninja/Tests/PrecompiledHeader/PCH/foo_precompiled.pch':  
> Permission denied
> [3/4] Building C object CMakeFiles\foo.dir\foo_precompile.c.obj  out:  
> foo_precom[3/4] Building C object CMakeFiles\foo.dir\foo2.c.obj   
> out: foo2.c
> 122: ninja: build stopped: subcommand failed.
> 1/1 Test #122: PrecompiledHeader ***Failed1.96 sec
>
> If I run the test again, it passes.   Might be that ninja is doing more  
> in parallel than gmake, nmake or visual studio.

Yes, it is a dependency problem.  Neither the Makefile nor Ninja
generators can easily generate correct dependencies for PCH,
because the PCH output file name only appears in the compiler flags.
The CMakeLists.txt for this test contains a hack to add correct
dependency information for the Makefile generator:

> # Setup dependencies for precompiled header creation and use.  The VS
> # IDE takes care of this automatically.
> IF("${CMAKE_GENERATOR}" MATCHES "Makefile")
>   # This source file creates the precompiled header as a side-effect.
>   SET_SOURCE_FILES_PROPERTIES(foo_precompile.c PROPERTIES
> OBJECT_OUTPUTS "${PCH_DIR}/foo_precompiled.pch")
> 
>   # These source files use the precompiled header.
>   SET_SOURCE_FILES_PROPERTIES(${foo_SRCS} PROPERTIES
> OBJECT_DEPENDS "${PCH_DIR}/foo_precompiled.pch")
> ENDIF("${CMAKE_GENERATOR}" MATCHES "Makefile")

Maybe another time we can think about ways to improve PCH support,
but for now I guess we can add an OR Ninja to this test and add
OBJECT_OUTPUTS support to the Ninja generator.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja/VS: fixed PDB name

2012-03-13 Thread Peter Collingbourne
On Wed, Mar 14, 2012 at 02:45:44AM +0100, Óscar Fuentes wrote:
> The compile and link commands executed by ninja contains the same
> option: /FdTARGET_PDB (TARGET_PDB here is literal text, not a
> placeholder.)  This is a simplified excerpt from a .ninja.log file:
> 
> 5969  11219   0   driver\CMakeFiles\foo.dir\foo.cpp.obj   "C:/Archivos de 
> programa/Microsoft Visual Studio 10.0/VC/bin/cl.exe"   /nologo /DWIN32 
> /D_WINDOWS /W3 /Zm1000 /EHsc /GR /MD /O2 /Ob2 /D NDEBUG /TP 
> /Fodriver\CMakeFiles\foo.dir\foo.cpp.obj /FdTARGET_PDB -c 
> ..\..\..\..\driver\foo.cpp
> 
> 11219 11922   0   foo.exe cmd.exe /c cd . && c:/apps/cmake/bin/cmake.exe 
> -E vs_link_exe "C:/Archivos de programa/Microsoft Visual Studio 
> 10.0/VC/bin/cl.exe"  /nologo driver\CMakeFiles\foo.dir\foo.cpp.obj /Fefoo.exe 
> /FdTARGET_PDB -link  /version:0.0  /STACK:1000 /machine:X86  
> /INCREMENTAL:NO  /subsystem:windows && cd .
> 
> 
> Please note how the compile command also has /FdTARGET_PDB, which makes
> no sense.
> 
> For executables, using the same /FdTARGET_PDB causes build failures when
> two or more executables are linked at the same time and the linker tries
> to create and lock the file TARGET_PDB for each of those executables.

Which version of cmake are you using?  Older versions of the ninja-generator
branch had this bug but it should now be fixed as of commit ae41323.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] ninja bug on windows

2012-03-10 Thread Peter Collingbourne
On Fri, Mar 09, 2012 at 02:10:45PM -0500, Bill Hoffman wrote:
> I am seeing that ninja always wants to relink the executables for me  
> every time it is run.  The output is this:
>
> $ ninja
> [1/9] Linking C static library  
> Utilities\cmlibarchive\libarchive\cmlibarchive.lib
> [2/9] Linking CXX executable bin\cmake.exe
> [3/9] Linking CXX executable bin\cmw9xcom.exe
> [4/9] Linking CXX executable bin\cpack.exe
> [5/9] Linking CXX executable bin\ctest.exe
> [6/9] Linking CXX executable Tests\CMakeLib\CMakeLibTests.exe
> [7/9] Generating ../Docs/ctest.txt
> [8/9] Generating ../Docs/cpack.txt
> [9/9] Generating ../Docs/cmake.txt
>
> Looks like the problem is libarchive getting recreated each time.

This seems to be a problem with the response file support in the
Windows branch of Ninja.  That branch of Ninja will always relink
any target (in this case cmlibarchive.lib) in the case where the
list of object files exceeds the command line length limit, because
it causes Ninja to use a response file with a different generated
name every time.  You can see this for yourself by examining the
.ninja_log file which contains the commands invoked by Ninja:

40  351 0   Utilities\cmlibarchive\libarchive\cmlibarchive.lib  
cmd.exe /c cd. && C:\PROGRA~1\MICROS~1.0\VC\bin\link.exe /lib /nologo  
@C:\DOCUME~1\peter\LOCALS~1\Temp\nja390.tmp&& cd.
751 54480   Utilities\cmlibarchive\libarchive\cmlibarchive.lib  
cmd.exe /c cd. && C:\PROGRA~1\MICROS~1.0\VC\bin\link.exe /lib /nologo  
@C:\DOCUME~1\peter\LOCALS~1\Temp\njaC0F.tmp&& cd.
681 69000   Utilities\cmlibarchive\libarchive\cmlibarchive.lib  
cmd.exe /c cd. && C:\PROGRA~1\MICROS~1.0\VC\bin\link.exe /lib /nologo  
@C:\DOCUME~1\peter\LOCALS~1\Temp\nja4F81.tmp&& cd.
616916264   0   Utilities\cmlibarchive\libarchive\cmlibarchive.lib  
cmd.exe /c cd. && C:\PROGRA~1\MICROS~1.0\VC\bin\link.exe /lib /nologo  
@C:\DOCUME~1\peter\LOCALS~1\Temp\nja4F95.tmp&& cd.
50  24430   Utilities\cmlibarchive\libarchive\cmlibarchive.lib  
cmd.exe /c cd. && C:\PROGRA~1\MICROS~1.0\VC\bin\link.exe /lib /nologo  
@C:\DOCUME~1\peter\LOCALS~1\Temp\nja8.tmp&& cd.
51  361 0   Utilities\cmlibarchive\libarchive\cmlibarchive.lib  
cmd.exe /c cd. && C:\PROGRA~1\MICROS~1.0\VC\bin\link.exe /lib /nologo  
@C:\DOCUME~1\peter\LOCALS~1\Temp\nja11.tmp&& cd.
50  351 0   Utilities\cmlibarchive\libarchive\cmlibarchive.lib  
cmd.exe /c cd. && C:\PROGRA~1\MICROS~1.0\VC\bin\link.exe /lib /nologo  
@C:\DOCUME~1\peter\LOCALS~1\Temp\nja19.tmp&& cd.
40  381 0   Utilities\cmlibarchive\libarchive\cmlibarchive.lib  
cmd.exe /c cd. && C:\PROGRA~1\MICROS~1.0\VC\bin\link.exe /lib /nologo  
@C:\DOCUME~1\peter\LOCALS~1\Temp\nja21.tmp&& cd.

The Ninja master branch avoids this problem by using a fixed name per
target for response files.  So I guess we will need to wait until we
move to master.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Rename Ninja generator?

2012-03-08 Thread Peter Collingbourne
On Thu, Mar 08, 2012 at 09:25:37PM -0500, Bill Hoffman wrote:
> On 3/8/2012 8:37 PM, Alan W. Irwin wrote:
>>
>> I think that would be confusing to users.
>>
>> Could you define an internal variable (to identify those generators
>> that have Makefile-like characteristics) that could be tested instead
>> of CMAKE_GENERATOR?
>
> I suppose you could, but it would not be backwards compatible. My  
> concern is all the projects that already exist that have code like this:
> if(CMAKE_GENERATOR MATCHES "Makefiles")
>
> A google search of that string gives my 5600 hits...
>
> Just trying to make as many projects work with ninja out of the box as  
> possible.

I did an exact search for that string:

http://www.google.co.uk/search?sourceid=chrome&ie=UTF-8&q=%22if(CMAKE_GENERATOR+MATCHES+Makefiles%22

and received only 36 hits, most of which seem to be matching
CMake's own code, which we can certainly change if necessary.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Rename Ninja generator?

2012-03-08 Thread Peter Collingbourne
On Thu, Mar 08, 2012 at 05:44:09PM -0500, Bill Hoffman wrote:
> I am wondering if we want to call the Ninja generator something like:
>
> Ninja Makefiles...  It is not totally accurate, but it avoids code like  
> this:
>
>
> IF(CMAKE_GENERATOR MATCHES "Makefiles" OR CMAKE_GENERATOR MATCHES "Ninja")
>
> Also, it is more likely to work with existing cmake build files.  I had  
> to do the same thing with jom.  Although with jom, it pretty much is a  
> makefile.
>
>
> Thoughts?

Hi Bill,

I thought about this for a while and I decided that this probably
isn't a good idea for a few reasons:

1) It is inaccurate.  The Ninja generator uses a fundamentally different
   approach to generating build files and I think that should be
   reflected in the name.

2) If we go with your suggestion, things will continue to work if what
   the project does for Make happens to also be appropriate for Ninja.
   But that isn't always the case -- if the project cares about
   which generator is being used then it may well be doing something
   specific to the Makefile generator (such as relying on the fact
   that CMake builds recursive makefiles by running the build program
   in a subdirectory, or using "$(MAKE)" -- you can see an example
   of that in ExternalProject.cmake).  In that case things would break
   unless the conditional excludes Ninja:

   IF(CMAKE_GENERATOR MATCHES "Makefiles" AND NOT CMAKE_GENERATOR MATCHES 
"Ninja")

   Things like that need to be decided on a case by case basis by
   the project developer and I don't see any reason to favour one
   scenario over the other.  All things being equal I would err on
   the conservative side and say that Ninja shouldn't receive any
   special treatment from projects by default -- if projects need
   the Ninja generator to get the same special treatment, they can
   match Ninja explicitly.

3) Less typing :) (Though one idea I had for improving the
   user interface for the -G option was to allow the argument to
   be matched case insensitively, and for unambiguous prefixes to
   be expanded.  So for example you could say -G nmake which would
   be disambiguated to "NMake Makefiles", but -G n would return an
   error since it matches both Ninja and NMake).

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja windows

2012-03-08 Thread Peter Collingbourne
On Tue, Mar 06, 2012 at 03:56:24PM -0500, Bill Hoffman wrote:
> On 3/6/2012 1:29 PM, Peter Collingbourne wrote:
>
>> We can certainly do that, but the ninja binary would need to be
>> switched as soon as we get around to using rspfile in Ninja (and
>> no sooner).
>>
>
> OK, so is there a ninja binary that I can download right now that will  
> work if I change the cmake files for cmake to build the ninja generator  
> on windows?  If so, where is it?  My idea is to try it on my machine. If 
> it works I will push the change into next that enables ninja on windows 
> and setup a dashboard that pulls ninja from that spot each night. When 
> cmake is changed to work with master ninja if someone updates that 
> binary, then it will pull that and just work.  That is what I have been 
> doing with jom.

That would be Peter Kummel's ninja.  But what I would suggest we do,
as I suggested before, is to use an internal variable to turn on the
Ninja generator on Windows and Mac.  This way, we can safely merge
ninja-generator into master without confusing users on Windows or
Mac by shipping a non-working Ninja generator in the next release,
while keeping the ability to test (on the dashboard and otherwise).
I have no idea what the timeframe for the Windows and Mac people
is, but I really would like the Ninja generator to be available on
non-Windows/Mac platforms in 2.8.8.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja windows

2012-03-06 Thread Peter Collingbourne
n Tue, Mar 06, 2012 at 12:43:29PM -0500, Bill Hoffman wrote:
> On 3/6/2012 12:17 PM, Peter Collingbourne wrote:
>
>> The Ninja generator is disabled on Windows as it still doesn't yet
>> work correctly with Ninja's master branch.  It works to some degree
>> with someone else's branch, but one of the required features on that
>> branch (response file support) was recently merged into Ninja master,
>> so hopefully someone on Windows will be able to add the necessary
>> support to CMake soon.  I would hold off on setting up a dashboard
>> for Windows until we can at least build CMake with Ninja master.
>>
>
> I am a bit confused.   What are the features on someone else's branch  
> that the CMake generator needs?

I think the only missing feature was response file support, but I am
not entirely sure.

> What will it take to get this:
>
>
> http://sourceforge.net/projects/cmakescript/files/cmake-2.8.7.20120202-Ninja.exe
>  
> 
>
>
> into the CMake source tree?
>
>
> I would be OK, with a non-standard ninja as long as those patches are  
> eventually going to be in ninja.

Well, the way that the branch supports response files is different to
the way that Ninja master supports them (the branch uses some kind of
hack to figure out what the command arguments are and copies those
into a response file, while on master the generator is responsible
for deciding on the contents of the response file using the "rspfile"
variable as documented here [1]).  The branch happens to work without
any modifications to CMake but we will need to add support for rspfile
for things to work on master.

> I don't like having ninja windows  
> cmake support outside of the main cmake dev tree.

All the code is already in ninja-generator in the staging repository,
it just needs to be turned on (i.e. by commenting out the if
statement).

>  Could we do this:
>
> 1. integrate cmake windows ninja support into next
> 2. run a dashboard that uses a given ninja binary?
>
> My goal would be to make sure cmake windows ninja keeps building and  
> working and other ninja work does not break the existing cmake windows  
> support.

We can certainly do that, but the ninja binary would need to be
switched as soon as we get around to using rspfile in Ninja (and
no sooner).

>> I have been asking Windows developers to enable the generator locally
>> for their own development by commenting out the IF(NOT WIN32) block.
>> Perhaps once we set up Windows dashboards it would be better to
>> introduce an internal variable (CMAKE_USE_NINJA?) which can be used
>> to enable this generator on Windows, and have the dashboards use it.
>>
> I would rather have it on by default and print some warning if it finds  
> the "wrong" ninja, one that does not have the required patches.

I don't think it's worth adding detection support for a now-obsolete
version of Ninja, but we can certainly check for rspfile support once
we start using that in CMake.

Thanks,
-- 
Peter

[1] http://martine.github.com/ninja/manual.html#_rule_variables
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja windows

2012-03-06 Thread Peter Collingbourne
On Tue, Mar 06, 2012 at 11:44:46AM -0500, Bill Hoffman wrote:
> What is the current status of ninja windows?  I would like to setup a  
> dashboard for this, however, the ninja code for windows does not seem to  
> be on the stage for CMake right now.   Is there a way to get it on the  
> stage?  Or is it too far from being useful to be on the stage at this 
> point?

The Ninja generator is disabled on Windows as it still doesn't yet
work correctly with Ninja's master branch.  It works to some degree
with someone else's branch, but one of the required features on that
branch (response file support) was recently merged into Ninja master,
so hopefully someone on Windows will be able to add the necessary
support to CMake soon.  I would hold off on setting up a dashboard
for Windows until we can at least build CMake with Ninja master.

I have been asking Windows developers to enable the generator locally
for their own development by commenting out the IF(NOT WIN32) block.
Perhaps once we set up Windows dashboards it would be better to
introduce an internal variable (CMAKE_USE_NINJA?) which can be used
to enable this generator on Windows, and have the dashboards use it.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja generator on Windows

2012-02-18 Thread Peter Collingbourne
On Wed, Feb 01, 2012 at 10:19:15PM +0100, Peter Kümmel wrote:
> On 31.01.2012 23:40, Peter Collingbourne wrote:
>> On Tue, Jan 31, 2012 at 10:32:23PM +0100, Peter Kümmel wrote:
>>> On 31.01.2012 16:19, Peter Collingbourne wrote:
>>>> On Tue, Jan 31, 2012 at 12:15:59AM +0100, Peter Kümmel wrote:
>>>>> - Paths like 'c:\' - Ninja now supports colon escaping "c:" ->   "c$:"
>>>>> Is there a single place where the escaping could be done?
>>>>
>>>> ':' only needs to be escaped in identifiers.  So
>>>> cmGlobalNinjaGenerator::EncodeIdent is the correct place.
>>>
>>> Also the colons in drive names must be "escaped". For instance,
>>> when you wanna configure ninja to build the tests with gtest
>>> at C:\gtest-1.6.0, you have to call
>>>
>>>   python configure.py --with-gtest=c$:\gtest-1.6.0
>>>
>>> this generates the rule:
>>>
>>> build $builddir\gtest_main.o: cxx $
>>>   c$:\gtest-1.6.0\src/gtest_main.cc
>>>
>>> Without the $: the build script does not work.
>>
>> Sorry if I wasn't clear here.  When I wrote "identifiers" I was
>> referring to lexicographical identifiers in the Ninja grammar.
>> Path names in build statements are lexed as identifiers, and so must
>> be escaped.  But variable values (including those containing paths)
>> are not lexed as identifiers so there is no need to escape anything
>> other than '$'.  All path names destined to be output as identifiers
>> should already be going through cmGlobalNinjaGenerator::EncodeIdent.
>
> Ok, but I also had to "EncodeLiteral" the commands, but this function
> is obsolete when we maybe use KWSys.

Apologies for not replying to this sooner.

This caused custom commands to be double encoded, causing a test failure
on *nix.  I reverted part of your changes to fix the test failures and
also had a look at implementing the escaping rules correctly.  With the
changes I made, I was able to configure CMake successfully using the
token-splitter branch.

I found that on Windows (on my machine at least), some try_compile
runs were non-deterministic, and this caused build failures in CMake.
I also encountered the issue Oscar mentioned in the other thread
regarding the pdb file being locked (maybe the two issues are related).
But I was able to successfully compile most of LLVM/Clang on Windows.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] ninja spaces in the path do not work on linux

2012-02-18 Thread Peter Collingbourne
On Fri, Feb 17, 2012 at 09:16:31PM +0100, Nicolas Desprès wrote:
> 2012/2/17 Bill Hoffman 
> 
> > On 2/17/2012 12:26 PM, Nicolas Desprès wrote:
> >
> >>
> >> I think it is a bug in the generator which do not escape the space
> >> properly using the $ character as supported by Ninja.
> >>
> >> --
> >> Nicolas Desprès
> >>
> >>  Will you be able to fix that?
> >
> 
> I think yes. It is just a matter of time. My weekend is already overloaded.
> I'll try to do it. If Peter or someone else in the community comes up with
> a patch before me everybody would be happy :-)

I decided to look into escaping rules this weekend.  Now with the
code on the ninja-generator branch, CMake can build itself with both
source and build directories containing spaces, and pass all tests.
LLVM/Clang can also build with both directory names containing spaces.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja help

2012-02-15 Thread Peter Collingbourne
On Wed, Feb 15, 2012 at 10:47:33AM -0500, Bill Hoffman wrote:
> Where at are the versions of Ninja that I need to use with CMake for:
>
> Windows:
> Mac:
> Linux:
>
>
> I have seen several git branches mentioned in emails, and it is not  
> clear to me where to get the right Ninja for CMake on all platforms.  To  
> setup nightly testing, is there a master git branch that we should test  
> against?  This would let us know if Ninja breaks CMake support right 
> away.

Hi Bill,

The correct "mainline" branch to use for all platforms (and the one
I have been testing against) is the master branch from this repository:

git://github.com/martine/ninja.git

which is maintained by Evan Martin.  I don't think any other branches
are maintained/code-reviewed/updated as frequently.  I know that
people have been experimenting with improved Windows support elsewhere
but I think that we should go with whatever is eventually merged
into mainline.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja generator on Windows

2012-02-01 Thread Peter Collingbourne
On Tue, Jan 31, 2012 at 10:58:32PM +, Peter Collingbourne wrote:
> On Tue, Jan 31, 2012 at 05:06:28PM -0500, Brad King wrote:
> > On 1/31/2012 4:44 PM, Peter Kümmel wrote:
> >> OK, thanks. I already noticed the usage of OutputFormat::SHELL.
> >> Then I have to extend cmLocalGenerator by a Ninja case. It will be
> >> similar to NMake but additionally replaces colons by '$:'.
> >>
> >> Should I add kwsysSystem_Shell_Flag_Ninja in System.h?
> >
> > Yes.  Unfortunately KWSys was not really the right place to have all
> > the platform-specific shell escaping code, but it ended up that way
> > historically.  Please keep any commits that touch KWSys mutually
> > exclusive from commits that touch files outside KWSys.
> 
> I disagree.  The escaping rules are already implemented correctly
> and are only required in the specific case where a path name needs
> to appear as a (lexicographical) identifier.  Also, it is impossible
> to represent certain strings as identifiers without writing some
> text beforehand.  For example, the generator will build things like
> this for paths containing the '@' character:
> 
> ident0 = a@b
> 
> build foo: CUSTOM_COMMAND $ident0
>   [...]
> 
> Therefore, the best place to keep this code is in the part of the Ninja
> generator that is responsible for writing rules/build statements to
> a file.

Having studied the situation a bit further, I think that
kwsysSystem_Shell_Flag_Ninja should be used to request the Ninja
encoding for variable values on top of the normal shell encoding
scheme (the identifier encoding rules don't really fit into the
existing scheme for shell encoding for the above reasons).  The only
encoding required for Ninja variable values (on top of standard shell
encoding) is to replace '$' by '$$' ('$' is the only special character
in variable values).

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja generator on Windows

2012-02-01 Thread Peter Collingbourne
On Tue, Jan 31, 2012 at 10:42:40PM -0500, Bill Hoffman wrote:
> On 1/31/2012 5:58 PM, Peter Collingbourne wrote:
>>> >  That will
>>> >  be handy when it comes time to integrate this work upstream because
>>> >  KWSys is shared with other projects.
>> Why isn't that time now?  The generator already works and passes all
>> tests on at least 2 platforms.
>
> Brad was talking about the changes to KWsys that have not yet been done  
> since Peter K, just asked about it:
>
>
> >Should I add kwsysSystem_Shell_Flag_Ninja in System.h?

Fair enough, but those changes are not required for the *nix port.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja generator on Windows

2012-01-31 Thread Peter Collingbourne
On Tue, Jan 31, 2012 at 05:06:28PM -0500, Brad King wrote:
> On 1/31/2012 4:44 PM, Peter Kümmel wrote:
>> OK, thanks. I already noticed the usage of OutputFormat::SHELL.
>> Then I have to extend cmLocalGenerator by a Ninja case. It will be
>> similar to NMake but additionally replaces colons by '$:'.
>>
>> Should I add kwsysSystem_Shell_Flag_Ninja in System.h?
>
> Yes.  Unfortunately KWSys was not really the right place to have all
> the platform-specific shell escaping code, but it ended up that way
> historically.  Please keep any commits that touch KWSys mutually
> exclusive from commits that touch files outside KWSys.

I disagree.  The escaping rules are already implemented correctly
and are only required in the specific case where a path name needs
to appear as a (lexicographical) identifier.  Also, it is impossible
to represent certain strings as identifiers without writing some
text beforehand.  For example, the generator will build things like
this for paths containing the '@' character:

ident0 = a@b

build foo: CUSTOM_COMMAND $ident0
  [...]

Therefore, the best place to keep this code is in the part of the Ninja
generator that is responsible for writing rules/build statements to
a file.

> That will
> be handy when it comes time to integrate this work upstream because
> KWSys is shared with other projects.

Why isn't that time now?  The generator already works and passes all
tests on at least 2 platforms.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja generator on Windows

2012-01-31 Thread Peter Collingbourne
On Tue, Jan 31, 2012 at 10:32:23PM +0100, Peter Kümmel wrote:
> On 31.01.2012 16:19, Peter Collingbourne wrote:
>> On Tue, Jan 31, 2012 at 12:15:59AM +0100, Peter Kümmel wrote:
>>> - Paths like 'c:\' - Ninja now supports colon escaping "c:" ->  "c$:"
>>>Is there a single place where the escaping could be done?
>>
>> ':' only needs to be escaped in identifiers.  So
>> cmGlobalNinjaGenerator::EncodeIdent is the correct place.
>
> Also the colons in drive names must be "escaped". For instance,
> when you wanna configure ninja to build the tests with gtest
> at C:\gtest-1.6.0, you have to call
>
>  python configure.py --with-gtest=c$:\gtest-1.6.0
>
> this generates the rule:
>
> build $builddir\gtest_main.o: cxx $
>  c$:\gtest-1.6.0\src/gtest_main.cc
>
> Without the $: the build script does not work.

Sorry if I wasn't clear here.  When I wrote "identifiers" I was
referring to lexicographical identifiers in the Ninja grammar.
Path names in build statements are lexed as identifiers, and so must
be escaped.  But variable values (including those containing paths)
are not lexed as identifiers so there is no need to escape anything
other than '$'.  All path names destined to be output as identifiers
should already be going through cmGlobalNinjaGenerator::EncodeIdent.

Probably the lack of correct identifier escaping for Ninja's configure
script is a bug in ninja_syntax.py.

>>> - Finding the msvc compiler fails because TARGET_IMPLIB is empty
>>>and to the linker /implib: is passed without an argument.
>>>Why is TARGET_IMPLIB empty?
>>
>> Probably because cmLocalGenerator::TargetImplib is not being
>> set.  Look at how the makefile generator does it.
>>
>
> Do you think much win32 stuff is missing? I ask, because you've based
> the ninja generator on a Makefile generator, or I'm wrong?

No idea.  I've never tried the generator on Windows.  We're probably
at least 3/4 of the way there though.

>>> - Some targets have the .exe extension, but the rule misses the .exe.
>>
>> Can you show an example of what you mean?  The generator should be
>> using cmTarget::GetFullPath to build paths to targets which seems
>> to append CMAKE_EXECUTABLE_SUFFIX (i.e. ".exe" on Windows) where
>> necessary.
>
> Is was a bug in my CMakeLists.txt because MSVC_IDE was set, even when I use
> the Ninja generator.
>
> Currently I use "CMAKE_GENERATOR STREQUAL Ninja" the check for the ninja 
> generator.
> Is there a simpler way? Couldn't we set NINJA or CMAKE_NINJA?

I don't see any reason to be inconsistent with the other generators.

>>> - When&&  is used for calling multiple command a 'cmd.exe /c' call is 
>>> necessary.
>>
>> Correct.  That will need to be prepended in
>> cmLocalNinjaGenerator::BuildCommandLine if the vector contains more
>> than one command.
>
> When there are no PRE and POST steps the cmd and && could be dropped 
> completely
> (also the "cd ." nop, which I've used instead of ":")

Yes.

I've reminded myself of something.  Remember that BuildCommandLine
is used to build both "full" command lines and sub-command lines
(such as lists of PRE and POST commands) and consider the case where
the sub command line contains more than one command.

COMMAND = cmd /c $PRE && foo && $POST
[...]
PRE = cmd /c foo && bar
POST = cmd /c foo && bar

Note the multiple "cmd /c"s.  I don't know what Windows will think
of that but it makes me nervous and in any case will probably slow
the build down.  We might need a pair of functions, one for building
full command lines and one for sub command lines.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] Ninja generator on Windows

2012-01-31 Thread Peter Collingbourne
On Tue, Jan 31, 2012 at 12:15:59AM +0100, Peter Kümmel wrote:
> I played a bit with the Ninja generator on Windows,
> and added some hacks to make it a bit running:
>
> https://github.com/syntheticpp/CMake/commit/0a55b61271106eb7c3319340f2c54f6bab3c0f8b
>
> Here are the problems I found so far:
>
> - Paths like 'c:\' - Ninja now supports colon escaping "c:" -> "c$:"
>   Is there a single place where the escaping could be done?

':' only needs to be escaped in identifiers.  So
cmGlobalNinjaGenerator::EncodeIdent is the correct place.

I don't think your changes to that function are correct though.
The old code would have correctly "escaped" colons by placing them in a
variable -- perhaps it could be made smarter by doing the replacement
if the only non-identifier character in the string is ':' (or ' '
or some other ident-escapable characters).

> - Finding the msvc compiler fails because TARGET_IMPLIB is empty
>   and to the linker /implib: is passed without an argument.
>   Why is TARGET_IMPLIB empty?

Probably because cmLocalGenerator::TargetImplib is not being
set.  Look at how the makefile generator does it.

> - Some targets have the .exe extension, but the rule misses the .exe.

Can you show an example of what you mean?  The generator should be
using cmTarget::GetFullPath to build paths to targets which seems
to append CMAKE_EXECUTABLE_SUFFIX (i.e. ".exe" on Windows) where
necessary.

> - When && is used for calling multiple command a 'cmd.exe /c' call is 
> necessary.

Correct.  That will need to be prepended in
cmLocalNinjaGenerator::BuildCommandLine if the vector contains more
than one command.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 0/3] The CMake Ninja generator.

2012-01-25 Thread Peter Collingbourne
On Tue, Jan 03, 2012 at 01:15:17AM +, Peter Collingbourne wrote:
> On Sat, Nov 12, 2011 at 02:36:04AM +0000, Peter Collingbourne wrote:
> > Hi,
> > 
> > These patches add the Ninja generator to CMake, which I am now
> > proposing for inclusion in mainline.
> [...]
> > This patch series is also available from my git repository:
> > git://github.com/pcc/CMake.git ninja-generator-pr
> 
> I've now updated the above repository with the changes suggested in
> this thread.
> 
> Is there anything else required for this to be merged to next?

Ping.

Branch has been updated with miscellaneous fixes for Mac OS X bringing
the total number of test failures on a Mac down to 4.  I don't have
time to fix the rest (which mainly relate to the construction of
apps/bundles/frameworks) so I will leave this to anyone who cares
enough about OS X to fix.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 0/3] The CMake Ninja generator.

2012-01-02 Thread Peter Collingbourne
On Sat, Nov 12, 2011 at 02:36:04AM +, Peter Collingbourne wrote:
> Hi,
> 
> These patches add the Ninja generator to CMake, which I am now
> proposing for inclusion in mainline.
[...]
> This patch series is also available from my git repository:
> git://github.com/pcc/CMake.git ninja-generator-pr

I've now updated the above repository with the changes suggested in
this thread.

Is there anything else required for this to be merged to next?

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 0/3] The CMake Ninja generator.

2011-11-22 Thread Peter Collingbourne
On Tue, Nov 22, 2011 at 09:21:59PM +0100, Alexander Neundorf wrote:
> On Tuesday 22 November 2011, Peter Collingbourne wrote:
> > On Tue, Nov 15, 2011 at 06:54:01PM +0100, Nicolas Desprès wrote:
> > > On Tue, Nov 15, 2011 at 5:29 PM, Bill Hoffman 
> wrote:
> > > > On 11/11/2011 9:36 PM, Peter Collingbourne wrote:
> > > >  Note that this generator is *nix only (it relies on POSIX shell
> > > >  
> > > >> functionality), and will only be built on *nix platforms.  I am not
> > > >> interested in Windows support, but I understand that others have
> > > >> expressed an interest in adding support.
> > > >> 
> > > >>  How much of the POSIX shell stuff can be done with cmake -E stuff? 
> > > >>  It
> > > > 
> > > > would be good to use as little POSIX stuff as possible as someday when
> > > > someone does the inevitable windows port it is easier...
> > > 
> > > Too much in my opinion. Specially the && operator of the shell. Using
> > > cmake -E is the first step. I am planning to suggest an evolution of
> > > Ninja to fix this issue. Actually, it is the main issue which stop me
> > > while writing the beginning of this generator.
> > 
> > Other than && we also have:
> > 
> > rm -f $file -- used to delete static library archives before rebuilding
> 
> cmake -E remove -f $file

I can't see a cmd.exe builtin with the same semantics as rm -f.
But it looks like the -f flag causes cmake to exit with a status
of 0 regardless of whether it was able to remove the file, which is
probably not what we want.  "cmake -E remove $file" should work.

> > :   -- used as a no-op command
> 
> maybe
> cmake -E echo

I don't think that we should spawn another process just for a no-op,
especially on Windows where process creation is expensive.  We should
use a shell builtin for this.  I can't see a cmd.exe builtin which
is explicitly a no-op, but perhaps a builtin with no effect such as
"echo off" would work.

Anyway, that change should be done by whoever does the Windows port.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 1/3] Add SystemTools::TrimWhitespace function

2011-11-22 Thread Peter Collingbourne
On Sun, Nov 13, 2011 at 01:04:24PM -0500, Brad King wrote:
> On 11/11/2011 9:36 PM, Peter Collingbourne wrote:
>> ---
>>   Source/kwsys/SystemTools.cxx|   15 +++
>>   Source/kwsys/SystemTools.hxx.in |5 +
>>   2 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/Source/kwsys/SystemTools.cxx b/Source/kwsys/SystemTools.cxx
>> index 1bf19c0..c14eb38 100644
>> --- a/Source/kwsys/SystemTools.cxx
>> +++ b/Source/kwsys/SystemTools.cxx
>> @@ -1195,6 +1195,21 @@ kwsys_stl::string SystemTools::UpperCase(const 
>> kwsys_stl::string&  s)
>> return n;
>>   }
>>
>> +// Returns a string that has whitespace removed from the start and the end.
>> +kwsys_stl::string SystemTools::TrimWhitespace(const kwsys_stl::string&  s)
>
> Please add this to Source/cmSystemTools instead.

Will do.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 3/3] Add the Ninja generator

2011-11-22 Thread Peter Collingbourne
On Mon, Nov 14, 2011 at 08:18:16AM +0100, Rolf Eike Beer wrote:
> > diff --git a/Modules/Compiler/GNU.cmake b/Modules/Compiler/GNU.cmake
> > index 8d6f5df..bdcaf9d 100644
> > --- a/Modules/Compiler/GNU.cmake
> > +++ b/Modules/Compiler/GNU.cmake
> > @@ -24,6 +24,15 @@ macro(__compiler_gnu lang)
> >set(CMAKE_SHARED_LIBRARY_${lang}_FLAGS "-fPIC")
> >set(CMAKE_SHARED_LIBRARY_CREATE_${lang}_FLAGS "-shared")
> > 
> > +  # Older versions of gcc (< 4.5) contain a bug causing them to report a
> > missing +  # header file as a warning if depfiles are enabled, causing
> > check_header_file +  # tests to always succeed.  Work around this by
> > disabling dependency tracking +  # in try_compile mode.
> > +  GET_PROPERTY(_IN_TC GLOBAL PROPERTY IN_TRY_COMPILE)
> > +  if(NOT _IN_TC OR CMAKE_FORCE_DEPFILES)
> > +set(CMAKE_DEPFILE_FLAGS_${lang} "-MMD -MF ")
> > +  endif()
> > +
> ># Initial configuration flags.
> >set(CMAKE_${lang}_FLAGS_INIT "")
> >set(CMAKE_${lang}_FLAGS_DEBUG_INIT "-g")
> 
> This may only affect Ninja, but it looks like a general bug. Shouldn't this 
> be 
> it's own commit then?

The Ninja generator is the only user of CMAKE_DEPFILE_FLAGS_${lang}
variables.  I will add this in its own commit to make it clear that
these variables can also be used by other generators.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 0/3] The CMake Ninja generator.

2011-11-22 Thread Peter Collingbourne
On Tue, Nov 15, 2011 at 06:54:01PM +0100, Nicolas Desprès wrote:
> On Tue, Nov 15, 2011 at 5:29 PM, Bill Hoffman wrote:
> 
> > On 11/11/2011 9:36 PM, Peter Collingbourne wrote:
> >
> >  Note that this generator is *nix only (it relies on POSIX shell
> >> functionality), and will only be built on *nix platforms.  I am not
> >> interested in Windows support, but I understand that others have
> >> expressed an interest in adding support.
> >>
> >>  How much of the POSIX shell stuff can be done with cmake -E stuff?  It
> > would be good to use as little POSIX stuff as possible as someday when
> > someone does the inevitable windows port it is easier...
> >
> >
> Too much in my opinion. Specially the && operator of the shell. Using cmake
> -E is the first step. I am planning to suggest an evolution of Ninja to fix
> this issue. Actually, it is the main issue which stop me while writing the
> beginning of this generator.

Other than && we also have:

rm -f $file -- used to delete static library archives before rebuilding
:   -- used as a no-op command

I think that for the Windows port we can simply select the appropriate
command (e.g. rm vs del) since we know which platform we are building
for.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 0/3] The CMake Ninja generator.

2011-11-22 Thread Peter Collingbourne
On Fri, Nov 18, 2011 at 10:02:54AM +0100, Nicolas Desprès wrote:
> On Wed, Nov 16, 2011 at 3:05 PM, OKUMURA Yuki  wrote:
> 
> > (Sorry Bill, i repost here..)
> >
> > 2011/11/16 Bill Hoffman :
> > - snip -
> > > What type of evolution will Ninja need?   I suppose it could use cmake
> > > scripts and exectue_process which can do && type things pretty easy.
> >
> > Why not insert "cmd /c" everywhere? :)
> > Anyway, i had tried this in early this month,
> >
> > https://github.com/okuoku/CMake/commit/db19df03606684f313c9e86b313218feee9cce3f
> > but it didn't work well.

How so?  Was there an escaping issue?  The cmd.exe documentation seems
to indicate that this will work.

http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs/en-us/ntcmds_shelloverview.mspx#EBG

> >
> > I think almost all use of "&&" is just "chdir /d $workdir &&
> > compile/link-command." So, adding $workdir
> > specifier to target syntax of Ninja might work for many cases.
> >
> 
> That's right. I just had a look at the build.ninja file generated by the
> Ninja Generator and it actually uses && only for changing the working
> directory.  I will suggest a patch to Ninja to support working directory
> for each build statement.

That's not the only thing that the Ninja generator uses && for.
It also uses && to prepend/append PRE_BUILD, PRE_LINK and POST_BUILD
commands to the link command, and to concatenate multiple custom
command lines.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 2/3] Add support for executable with exports flags to cmLocalGenerator::GetTargetFlags

2011-11-12 Thread Peter Collingbourne
On Sat, Nov 12, 2011 at 10:45:04AM -0500, David Cole wrote:
> Looks like PATCH 3/3 didn't come through... (too large for the mailing list?)

It apparently requires moderator approval due to its size:

> Your mail to 'cmake-developers' with the subject
> 
> [PATCH 3/3] Add the Ninja generator
> 
> Is being held until the list moderator can review it for approval.
> 
> The reason it is being held:
> 
> Message body is too big: 108985 bytes with a limit of 80 KB

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


[cmake-developers] [PATCH 1/3] Add SystemTools::TrimWhitespace function

2011-11-11 Thread Peter Collingbourne
---
 Source/kwsys/SystemTools.cxx|   15 +++
 Source/kwsys/SystemTools.hxx.in |5 +
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/Source/kwsys/SystemTools.cxx b/Source/kwsys/SystemTools.cxx
index 1bf19c0..c14eb38 100644
--- a/Source/kwsys/SystemTools.cxx
+++ b/Source/kwsys/SystemTools.cxx
@@ -1195,6 +1195,21 @@ kwsys_stl::string SystemTools::UpperCase(const 
kwsys_stl::string& s)
   return n;
 }
 
+// Returns a string that has whitespace removed from the start and the end.
+kwsys_stl::string SystemTools::TrimWhitespace(const kwsys_stl::string& s)
+{
+  kwsys_stl::string::const_iterator start = s.begin();
+  while(start != s.end() && *start == ' ')
+++start;
+  if (start == s.end())
+return "";
+
+  kwsys_stl::string::const_iterator stop = s.end()-1;
+  while(*stop == ' ')
+--stop;
+  return kwsys_stl::string(start, stop+1);
+}
+
 // Count char in string
 size_t SystemTools::CountChar(const char* str, char c)
 {
diff --git a/Source/kwsys/SystemTools.hxx.in b/Source/kwsys/SystemTools.hxx.in
index 04f1978..c03b8bc 100644
--- a/Source/kwsys/SystemTools.hxx.in
+++ b/Source/kwsys/SystemTools.hxx.in
@@ -129,6 +129,11 @@ public:
   static kwsys_stl::string UpperCase(const kwsys_stl::string&);
   
   /**
+   * Returns a string that has whitespace removed from the start and the end.
+   */
+  static kwsys_stl::string TrimWhitespace(const kwsys_stl::string& s);
+
+  /**
* Count char in string
*/
   static size_t CountChar(const char* str, char c);
-- 
1.7.5.3

--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


[cmake-developers] [PATCH 2/3] Add support for executable with exports flags to cmLocalGenerator::GetTargetFlags

2011-11-11 Thread Peter Collingbourne
---
 Source/cmLocalGenerator.cxx |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index 65d6fa6..ebdfdf5 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -1583,6 +1583,16 @@ void cmLocalGenerator::GetTargetFlags(std::string& 
linkLibs,
   this->Makefile->GetSafeDefinition("CMAKE_CREATE_CONSOLE_EXE");
 linkFlags += " ";
 }
+  if (target.IsExecutableWithExports())
+{
+std::string exportFlagVar = "CMAKE_EXE_EXPORTS_";
+exportFlagVar += linkLanguage;
+exportFlagVar += "_FLAG";
+
+linkFlags +=
+  this->Makefile->GetSafeDefinition(exportFlagVar.c_str());
+linkFlags += " ";
+}
   const char* targetLinkFlags = target.GetProperty("LINK_FLAGS");
   if(targetLinkFlags)
 {
-- 
1.7.5.3

--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


[cmake-developers] [PATCH 0/3] The CMake Ninja generator.

2011-11-11 Thread Peter Collingbourne
Hi,

These patches add the Ninja generator to CMake, which I am now
proposing for inclusion in mainline.  The generator should work
with the current 'master' branch of Ninja from:

https://github.com/martine/ninja

The generator passes the CMake test suite on Linux (Debian testing)
and FreeBSD 9.0-RC1, and I have been regularly using it myself with
LLVM and Clang for about 2 months.  I have also successfully tested it
with the build systems for OpenCV, Bullet Physics, BRL-CAD, kdelibs
and Path64.

Some performance numbers for no-op builds (2.0GHz dual core laptop,
Debian testing):

CMake:
Unix Makefiles: 0.6s
Ninja:  0.042s

kdelibs:
Unix Makefiles: 9.8s
Ninja:  2.5s [1]

LLVM/Clang/Polly:
Unix Makefiles: 2.9s
Ninja:  0.18s

BRL-CAD:
Unix Makefiles: 19s
Ninja:  0.40s

Path64:
Unix Makefiles: 3.9s
Ninja:  0.25s

Note that this generator is *nix only (it relies on POSIX shell
functionality), and will only be built on *nix platforms.  I am not
interested in Windows support, but I understand that others have
expressed an interest in adding support.

I made some minor changes to the ExternalProject module as well as
some of the tests to make them work with the generator, and added
explanations for those changes as comments.

I separated out the changes to kwsys, so that they can be applied
separately.

This patch series is also available from my git repository:
git://github.com/pcc/CMake.git ninja-generator-pr

Please review!

Thanks,
-- 
Peter

[1] The disproportionately large Ninja run time here seems to be
caused by a large number of required invocations of the 'automoc'
command.

--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 0/7] Preparation for Ninja generator: refactorings, bug fixes

2011-11-09 Thread Peter Collingbourne
On Tue, Oct 04, 2011 at 11:21:00PM +0200, Peter Kümmel wrote:
> On 04.10.2011 23:19, Peter Kümmel wrote:
> >On 03.10.2011 15:03, Brad King wrote:
> >>On 10/2/2011 1:41 PM, Peter Collingbourne wrote:
> >>>I have modified the commit message to include more details, and pushed
> >>>a modified branch to github.
> >>
> >>I've fetched the latest version of the branch.  Thanks for the updates.
> >>
> >>-Brad
> >
> >Could you also enable Ninja files for CodeBlock's generators?

This doesn't appear to work here.  I ran CMake with the
CodeBlocks/Ninja generator over the CMake project, and received the
following error in CodeBlocks when I tried to build:

Using makefile: Makefile
make: Makefile: No such file or directory
make: *** No rule to make target `Makefile'.  Stop.
Process terminated with status 2 (0 minutes, 0 seconds)
  0 errors, 0 warnings

This is with CodeBlocks version 10.05.  The fix may be trivial,
but I don't know how easy it would be to teach CodeBlocks to use an
alternative build tool.

> And maybe it is also trivial for 'Eclipse CDT4'.

It isn't.  I ran CMake with the Eclipse/Ninja generator over the
CMake project, and I received a segmentation fault.  It seems
that the Eclipse generator assumes that the underlying generator
is the Unix Makefile generator (see the static_cast on line 1038 of
cmExtraEclipseCDT4Generator.cxx).  Someone with more familiarity with
the Eclipse generator would need to look at this.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 7/7] Fix configuration-dependent flag lookup in cmLocalGenerator::GetTargetFlags

2011-10-02 Thread Peter Collingbourne
On Tue, Sep 27, 2011 at 05:33:44PM +0200, Nicolas Desprès wrote:
> On Tue, Sep 27, 2011 at 4:03 PM, Rolf Eike Beer  wrote:
> >> ---
> >>  Source/cmLocalGenerator.cxx |   23 ---
> >>  1 files changed, 20 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
> >> index aeda164..4044876 100644
> >> --- a/Source/cmLocalGenerator.cxx
> >> +++ b/Source/cmLocalGenerator.cxx
> >> @@ -1463,6 +1463,17 @@ void cmLocalGenerator::GetTargetFlags(std::string&
> >> linkLibs,
> >>          linkFlags += targetLinkFlags;
> >>          linkFlags += " ";
> >>          }
> >> +      if(buildType.size())
> >
> > This could also be "if(!buildType.empty())", but it's hard to decide which
> > one to use. This file alone uses a great mixture of both variants.
> 
> Your version is more readable.

I agree, and I decided to modify this patch to use
"if(!buildType.empty())" consistently within this function.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 1/7] Refactor TargetTypeNames.

2011-10-02 Thread Peter Collingbourne
On Tue, Sep 27, 2011 at 05:32:19PM +0200, Nicolas Desprès wrote:
> On Tue, Sep 27, 2011 at 3:55 PM, Rolf Eike Beer  wrote:
> >> From: Nicolas Despres 
> >>
> >> Make it a static method instead of an array. It is safer for the
> >> type checking and if we add a new target type we will be warned to add
> >> a case to the switch.
> >
> >> +      case cmTarget::UNKNOWN_LIBRARY:
> >> +        return "UNKNOWN_LIBRARY";
> >> +        // break; /* unreachable */
> >> +    }
> >> +  return 0;
> >> +}
> >
> > Shouldn't this spit out a warning or error message?
> 
> Frankly I don't know. I just refactor the code and kept the same
> original behavior.

This function should not expect to be passed an invalid target type as
a parameter, especially since we are covering all of the enumerators.
I have modified this patch to add an assertion check that the end of
the function is unreachable.

While at it, I also removed the commented out break statements, which
are just noise.

The modified patch is now in my github branch.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] [PATCH 0/7] Preparation for Ninja generator: refactorings, bug fixes

2011-10-02 Thread Peter Collingbourne
On Tue, Sep 27, 2011 at 08:19:39AM -0400, Brad King wrote:
> On 9/26/2011 11:48 PM, Peter Collingbourne wrote:
>> Now that the Ninja generator is passing all tests on Linux, I have
>> decided to start preparing patches, and I am starting with some
>> refactorings and bug fixes developed in the course of developing the
>> Ninja generator.  Three of the patches were originally by Nicolas
>> Despres, the rest are by myself.
>
> Fantastic!  Thanks for the well-formatted patch submission.  It was easy
> to review the inline patch series.  I also fetched your prep branch from
> github.  I think we can merge it as soon as the 2.8.6 release is finalized.

Great.  I will plan to submit the main Ninja generator patch on top
of the branch after it is merged.

> Patches 1-6 look good.  Please expand the commit message in patch 7 to
> elaborate on the specific problems it fixes.  That looks like a real bug
> fix rather than just refactoring.  I guess no one has been using per-config
> link flags.

I have modified the commit message to include more details, and pushed
a modified branch to github.

Per-config link flags should certainly work for most generators; the
test suite includes a test for them (which is how I found this bug).

It looks like the only other generators to use the ComputeLinkFlags
function are the Visual Studio 6 and 7 generators (via
cmTarget::CreateCustomTargetsAndCommands), which use it to build custom
link commands for languages not falling within a set of predefined
languages, including C and C++.  That might explain why the issue
with this function had not been noticed before.

Thanks,
-- 
Peter
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


[cmake-developers] [PATCH 7/7] Fix configuration-dependent flag lookup in cmLocalGenerator::GetTargetFlags

2011-09-26 Thread Peter Collingbourne
---
 Source/cmLocalGenerator.cxx |   23 ---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index aeda164..4044876 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -1463,6 +1463,17 @@ void cmLocalGenerator::GetTargetFlags(std::string& 
linkLibs,
 linkFlags += targetLinkFlags;
 linkFlags += " ";
 }
+  if(buildType.size())
+{
+std::string build = "STATIC_LIBRARY_FLAGS_";
+build += buildType;
+targetLinkFlags = target.GetProperty(build.c_str());
+if(targetLinkFlags)
+  {
+  linkFlags += targetLinkFlags;
+  linkFlags += " ";
+  }
+}
   }
   break; 
 case cmTarget::MODULE_LIBRARY:
@@ -1502,7 +1513,10 @@ void cmLocalGenerator::GetTargetFlags(std::string& 
linkLibs,
 {
 linkFlags += targetLinkFlags;
 linkFlags += " ";
-std::string configLinkFlags = targetLinkFlags;
+}
+  if(buildType.size())
+{
+std::string configLinkFlags = "LINK_FLAGS_";
 configLinkFlags += buildType;
 targetLinkFlags = target.GetProperty(configLinkFlags.c_str());
 if(targetLinkFlags)
@@ -1573,8 +1587,11 @@ void cmLocalGenerator::GetTargetFlags(std::string& 
linkLibs,
   if(targetLinkFlags)
 {
 linkFlags += targetLinkFlags;
-linkFlags += " ";  
-std::string configLinkFlags = targetLinkFlags;
+linkFlags += " ";
+}
+  if(buildType.size())
+{
+std::string configLinkFlags = "LINK_FLAGS_";
 configLinkFlags += buildType;
 targetLinkFlags = target.GetProperty(configLinkFlags.c_str());
 if(targetLinkFlags)
-- 
1.7.5.3

--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


[cmake-developers] [PATCH 6/7] Introduce a cmGlobalGenerator::ResolveLanguageCompiler function

2011-09-26 Thread Peter Collingbourne
It is factored out of cmGlobalUnixMakefileGenerator3::EnableLanguage,
and may be used by other generators to resolve CMAKE_*_COMPILER
settings.
---
 Source/cmGlobalGenerator.cxx  |   73 +
 Source/cmGlobalGenerator.h|7 +++
 Source/cmGlobalUnixMakefileGenerator3.cxx |   70 +---
 3 files changed, 81 insertions(+), 69 deletions(-)

diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index d62fb44..124519a 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -77,6 +77,79 @@ cmGlobalGenerator::~cmGlobalGenerator()
   this->ClearExportSets();
 }
 
+void cmGlobalGenerator::ResolveLanguageCompiler(const std::string &lang,
+cmMakefile *mf,
+bool optional) {
+  std::string langComp = "CMAKE_";
+  langComp += lang;
+  langComp += "_COMPILER";
+
+  if(!mf->GetDefinition(langComp.c_str()))
+{
+if(!optional)
+  {
+  cmSystemTools::Error(langComp.c_str(),
+   " not set, after EnableLanguage");
+  }
+return;
+}
+  const char* name = mf->GetRequiredDefinition(langComp.c_str());
+  std::string path;
+  if(!cmSystemTools::FileIsFullPath(name))
+{
+path = cmSystemTools::FindProgram(name);
+}
+  else
+{
+path = name;
+}
+  if((path.size() == 0 || !cmSystemTools::FileExists(path.c_str()))
+  && (optional==false))
+{
+std::string message = "your ";
+message += lang;
+message += " compiler: \"";
+message +=  name;
+message += "\" was not found.   Please set ";
+message += langComp;
+message += " to a valid compiler path or name.";
+cmSystemTools::Error(message.c_str());
+path = name;
+}
+  std::string doc = lang;
+  doc += " compiler.";
+  const char* cname = this->GetCMakeInstance()->
+GetCacheManager()->GetCacheValue(langComp.c_str());
+  std::string changeVars;
+  if(cname && (path != cname) && (optional==false))
+{
+std::string cnameString = cname;
+std::string pathString = path;
+// get rid of potentially multiple slashes:
+cmSystemTools::ConvertToUnixSlashes(cnameString);
+cmSystemTools::ConvertToUnixSlashes(pathString);
+if (cnameString != pathString)
+  {
+  const char* cvars =
+this->GetCMakeInstance()->GetProperty(
+  "__CMAKE_DELETE_CACHE_CHANGE_VARS_");
+  if(cvars)
+{
+changeVars += cvars;
+changeVars += ";";
+}
+  changeVars += langComp;
+  changeVars += ";";
+  changeVars += cname;
+  this->GetCMakeInstance()->SetProperty(
+"__CMAKE_DELETE_CACHE_CHANGE_VARS_",
+changeVars.c_str());
+  }
+}
+  mf->AddCacheDefinition(langComp.c_str(), path.c_str(),
+ doc.c_str(), cmCacheManager::FILEPATH);
+}
+
 // Find the make program for the generator, required for try compiles
 void cmGlobalGenerator::FindMakeProgram(cmMakefile* mf)
 {
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index adf7b77..96a326c 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -77,6 +77,13 @@ public:
   cmMakefile *, bool optional);
 
   /**
+   * Resolve the CMAKE__COMPILER setting for the given language.
+   * Intended to be called from EnableLanguage.
+   */
+  void ResolveLanguageCompiler(const std::string &lang, cmMakefile *mf,
+   bool optional);
+
+  /**
* Try to determine system infomation, get it from another generator
*/
   virtual void EnableLanguagesFromGenerator(cmGlobalGenerator *gen,
diff --git a/Source/cmGlobalUnixMakefileGenerator3.cxx 
b/Source/cmGlobalUnixMakefileGenerator3.cxx
index 169d77d..a23c0d8 100644
--- a/Source/cmGlobalUnixMakefileGenerator3.cxx
+++ b/Source/cmGlobalUnixMakefileGenerator3.cxx
@@ -39,7 +39,6 @@ void cmGlobalUnixMakefileGenerator3
  bool optional)
 {
   this->cmGlobalGenerator::EnableLanguage(languages, mf, optional);
-  std::string path;
   for(std::vector::const_iterator l = languages.begin();
   l != languages.end(); ++l)
 {
@@ -47,74 +46,7 @@ void cmGlobalUnixMakefileGenerator3
   {
   continue;
   }
-const char* lang = l->c_str();
-std::string langComp = "CMAKE_";
-langComp += lang;
-langComp += "_COMPILER";
-
-if(!mf->GetDefinition(langComp.c_str()))
-  {
-  if(!optional)
-{
-cmSystemTools::Error(langComp.c_str(),
- " not set, after EnableLanguage");
-}
-  continue;
-  }
-const char* name = mf->GetRequiredDefinition(langComp.c_str());
-if(!cmSystemTools::FileIsFullPath(name))
-  {
-  path = cmSystemTools::FindProgram(name);
-  }
-else
-  {
-  path = name;
-  }
-if((path.size() == 0 || !cmSystemTools::FileExists(path.c_str()))
-&&

[cmake-developers] [PATCH 4/7] Make cmLocalGenerator::ConvertToLinkReference virtual

2011-09-26 Thread Peter Collingbourne
This provides a mechanism for the local generator to override how
library search paths are generated.
---
 Source/cmLocalGenerator.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Source/cmLocalGenerator.h b/Source/cmLocalGenerator.h
index 1f5a26e..69e452b 100644
--- a/Source/cmLocalGenerator.h
+++ b/Source/cmLocalGenerator.h
@@ -369,7 +369,7 @@ protected:
   std::string FindRelativePathTopBinary();
   void SetupPathConversions();
 
-  std::string ConvertToLinkReference(std::string const& lib);
+  virtual std::string ConvertToLinkReference(std::string const& lib);
 
   /** Check whether the native build system supports the given
   definition.  Issues a warning.  */
-- 
1.7.5.3

--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


[cmake-developers] [PATCH 3/7] Constify many getters of cmGlobalGenerator.

2011-09-26 Thread Peter Collingbourne
From: Nicolas Despres 

---
 Source/cmGlobalGenerator.h  |   32 +++---
 Source/cmGlobalUnixMakefileGenerator3.h |   22 ++--
 2 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index 97cacc5..adf7b77 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -163,8 +163,8 @@ public:
 
   int TryCompileTimeout;
 
-  bool GetForceUnixPaths() {return this->ForceUnixPaths;}
-  bool GetToolSupportsColor() { return this->ToolSupportsColor; }
+  bool GetForceUnixPaths() const { return this->ForceUnixPaths; }
+  bool GetToolSupportsColor() const { return this->ToolSupportsColor; }
 
   ///! return the language for the given extension
   const char* GetLanguageFromExtension(const char* ext);
@@ -179,11 +179,11 @@ public:
   virtual const char* GetCMakeCFGInitDirectory()  { return "."; }
 
   /** Get whether the generator should use a script for link commands.  */
-  bool GetUseLinkScript() { return this->UseLinkScript; }
+  bool GetUseLinkScript() const { return this->UseLinkScript; }
 
   /** Get whether the generator should produce special marks on rules
   producing symbolic (non-file) outputs.  */
-  bool GetNeedSymbolicMark() { return this->NeedSymbolicMark; }
+  bool GetNeedSymbolicMark() const { return this->NeedSymbolicMark; }
 
   /*
* Determine what program to use for building the project.
@@ -213,7 +213,7 @@ public:
 
   /** Get the manifest of all targets that will be built for each
   configuration.  This is valid during generation only.  */
-  cmTargetManifest const& GetTargetManifest() { return this->TargetManifest; }
+  cmTargetManifest const& GetTargetManifest() const { return 
this->TargetManifest; }
 
   /** Get the content of a directory.  Directory listings are loaded
   from disk at most once and cached.  During the generation step
@@ -224,17 +224,17 @@ public:
 
   void AddTarget(cmTargets::value_type &v);
 
-  virtual const char* GetAllTargetName()  { return "ALL_BUILD"; }
-  virtual const char* GetInstallTargetName()  { return "INSTALL"; }
-  virtual const char* GetInstallLocalTargetName() { return 0; }
-  virtual const char* GetInstallStripTargetName() { return 0; }
-  virtual const char* GetPreinstallTargetName()   { return 0; }
-  virtual const char* GetTestTargetName() { return "RUN_TESTS"; }
-  virtual const char* GetPackageTargetName()  { return "PACKAGE"; }
-  virtual const char* GetPackageSourceTargetName(){ return 0; }
-  virtual const char* GetEditCacheTargetName(){ return 0; }
-  virtual const char* GetRebuildCacheTargetName() { return 0; }
-  virtual const char* GetCleanTargetName(){ return 0; }
+  virtual const char* GetAllTargetName()   const { return "ALL_BUILD"; 
}
+  virtual const char* GetInstallTargetName()   const { return "INSTALL"; }
+  virtual const char* GetInstallLocalTargetName()  const { return 0; }
+  virtual const char* GetInstallStripTargetName()  const { return 0; }
+  virtual const char* GetPreinstallTargetName()const { return 0; }
+  virtual const char* GetTestTargetName()  const { return "RUN_TESTS"; 
}
+  virtual const char* GetPackageTargetName()   const { return "PACKAGE"; }
+  virtual const char* GetPackageSourceTargetName() const { return 0; }
+  virtual const char* GetEditCacheTargetName() const { return 0; }
+  virtual const char* GetRebuildCacheTargetName()  const { return 0; }
+  virtual const char* GetCleanTargetName() const { return 0; }
 
   // Class to track a set of dependencies.
   typedef cmTargetDependSet TargetDependSet;
diff --git a/Source/cmGlobalUnixMakefileGenerator3.h 
b/Source/cmGlobalUnixMakefileGenerator3.h
index d21d5b9..7c6bbc2 100644
--- a/Source/cmGlobalUnixMakefileGenerator3.h
+++ b/Source/cmGlobalUnixMakefileGenerator3.h
@@ -137,17 +137,17 @@ protected:
   bool NeedRequiresStep(cmTarget const&);
 
   // Setup target names
-  virtual const char* GetAllTargetName()  { return "all"; }
-  virtual const char* GetInstallTargetName()  { return "install"; }
-  virtual const char* GetInstallLocalTargetName() { return "install/local"; }
-  virtual const char* GetInstallStripTargetName() { return "install/strip"; }
-  virtual const char* GetPreinstallTargetName()   { return "preinstall"; }
-  virtual const char* GetTestTargetName() { return "test"; }
-  virtual const char* GetPackageTargetName()  { return "package"; }
-  virtual const char* GetPackageSourceTargetName(){ return "package_source"; }
-  virtual const char* GetEditCacheTargetName(){ return "edit_cache"; }
-  virtual const char* GetRebuildCacheTargetName() { return "rebuild_cache"; }
-  virtual const char* GetCleanTargetName(){ return "clean"; }
+  virtual const char* GetAllTargetName()   const { return "all"; }
+  virtual const char* GetInstallTargetName()   const { return "install"; }
+  virtual const

[cmake-developers] [PATCH 2/7] Add const versions of some getters.

2011-09-26 Thread Peter Collingbourne
From: Nicolas Despres 

---
 Source/cmGlobalGenerator.h |1 +
 Source/cmLocalGenerator.h  |2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index 88eb8b6..97cacc5 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -120,6 +120,7 @@ public:
 
   ///! Get the CMake instance
   cmake *GetCMakeInstance() { return this->CMakeInstance; };
+  const cmake *GetCMakeInstance() const { return this->CMakeInstance; };
 
   void SetConfiguredFilesPath(cmGlobalGenerator* gen);
   const std::vector& GetLocalGenerators() const {
diff --git a/Source/cmLocalGenerator.h b/Source/cmLocalGenerator.h
index cfc09dc..1f5a26e 100644
--- a/Source/cmLocalGenerator.h
+++ b/Source/cmLocalGenerator.h
@@ -83,6 +83,8 @@ public:
   ///! Get the GlobalGenerator this is associated with
   cmGlobalGenerator *GetGlobalGenerator() {
 return this->GlobalGenerator; };
+  const cmGlobalGenerator *GetGlobalGenerator() const {
+return this->GlobalGenerator; };
 
   ///! Set the Global Generator, done on creation by the GlobalGenerator
   void SetGlobalGenerator(cmGlobalGenerator *gg);
-- 
1.7.5.3

--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


[cmake-developers] [PATCH 1/7] Refactor TargetTypeNames.

2011-09-26 Thread Peter Collingbourne
From: Nicolas Despres 

Make it a static method instead of an array. It is safer for the
type checking and if we add a new target type we will be warned to add
a case to the switch.
---
 Source/cmComputeTargetDepends.cxx |2 +-
 Source/cmLocalGenerator.cxx   |2 +-
 Source/cmMakefile.cxx |2 +-
 Source/cmTarget.cxx   |   81 ++---
 Source/cmTarget.h |2 +-
 5 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/Source/cmComputeTargetDepends.cxx 
b/Source/cmComputeTargetDepends.cxx
index 3a0ed06..8e701c4 100644
--- a/Source/cmComputeTargetDepends.cxx
+++ b/Source/cmComputeTargetDepends.cxx
@@ -404,7 +404,7 @@ cmComputeTargetDepends
 
 // Describe the depender.
 e << "  \"" << depender->GetName() << "\" of type "
-  << cmTarget::TargetTypeNames[depender->GetType()] << "\n";
+  << cmTarget::GetTargetTypeName(depender->GetType()) << "\n";
 
 // List its dependencies that are inside the component.
 EdgeList const& nl = this->InitialGraph[i];
diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index 1d1e8da..6af7fd5 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -996,7 +996,7 @@ cmLocalGenerator::ExpandRuleVariable(std::string const& 
variable,
   }
 if(variable == "TARGET_TYPE")
   {
-  return cmTarget::TargetTypeNames[replaceValues.CMTarget->GetType()];
+  return cmTarget::GetTargetTypeName(replaceValues.CMTarget->GetType());
   }
 }
   if(replaceValues.Output)
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index e5b5443..573c430 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -1379,7 +1379,7 @@ void cmMakefile::AddLinkLibraryForTarget(const char 
*target,
 {
 cmOStringStream e;
 e << "Target \"" << lib << "\" of type "
-  << cmTarget::TargetTypeNames[static_cast(tgt->GetType())]
+  << cmTarget::GetTargetTypeName(tgt->GetType())
   << " may not be linked into another target.  "
   << "One may link only to STATIC or SHARED libraries, or "
   << "to executables with the ENABLE_EXPORTS property set.";
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index fb92016..c0dc80d 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -25,12 +25,44 @@
 #include 
 #include  // required for atof
 #include 
-const char* cmTarget::TargetTypeNames[] = {
-  "EXECUTABLE", "STATIC_LIBRARY",
-  "SHARED_LIBRARY", "MODULE_LIBRARY", "UTILITY", "GLOBAL_TARGET",
-  "INSTALL_FILES", "INSTALL_PROGRAMS", "INSTALL_DIRECTORY",
-  "UNKNOWN_LIBRARY"
-};
+
+const char* cmTarget::GetTargetTypeName(TargetType targetType)
+{
+  switch( targetType )
+{
+  case cmTarget::STATIC_LIBRARY:
+return "STATIC_LIBRARY";
+// break; /* unreachable */
+  case cmTarget::MODULE_LIBRARY:
+return "MODULE_LIBRARY";
+// break; /* unreachable */
+  case cmTarget::SHARED_LIBRARY:
+return "SHARED_LIBRARY";
+// break; /* unreachable */
+  case cmTarget::EXECUTABLE:
+return "EXECUTABLE";
+// break; /* unreachable */
+  case cmTarget::UTILITY:
+return "UTILITY";
+// break; /* unreachable */
+  case cmTarget::GLOBAL_TARGET:
+return "GLOBAL_TARGET";
+// break; /* unreachable */
+  case cmTarget::INSTALL_FILES:
+return "INSTALL_FILES";
+// break; /* unreachable */
+  case cmTarget::INSTALL_PROGRAMS:
+return "INSTALL_PROGRAMS";
+// break; /* unreachable */
+  case cmTarget::INSTALL_DIRECTORY:
+return "INSTALL_DIRECTORY";
+// break; /* unreachable */
+  case cmTarget::UNKNOWN_LIBRARY:
+return "UNKNOWN_LIBRARY";
+// break; /* unreachable */
+}
+  return 0;
+}
 
 //
 struct cmTarget::OutputInfo
@@ -2346,7 +2378,7 @@ cmTarget::OutputInfo const* cmTarget::GetOutputInfo(const 
char* config)
 std::string msg = "cmTarget::GetOutputInfo called for ";
 msg += this->GetName();
 msg += " which has type ";
-msg += cmTarget::TargetTypeNames[this->GetType()];
+msg += cmTarget::GetTargetTypeName(this->GetType());
 this->GetMakefile()->IssueMessage(cmake::INTERNAL_ERROR, msg);
 abort();
 return 0;
@@ -2645,40 +2677,7 @@ const char *cmTarget::GetProperty(const char* prop,
   // the type property returns what type the target is
   if (!strcmp(prop,"TYPE"))
 {
-switch( this->GetType() )
-  {
-  case cmTarget::STATIC_LIBRARY:
-return "STATIC_LIBRARY";
-// break; /* unreachable */
-  case cmTarget::MODULE_LIBRARY:
-return "MODULE_LIBRARY";
-// break; /* unreachable */
-  case cmTarget::SHARED_LIBRARY:
-return "SHARED_LIBRARY";
-// break; /* unreachable */
-  case cmTarget::EXECUTABLE:
-return "EXECUTABLE

[cmake-developers] [PATCH 0/7] Preparation for Ninja generator: refactorings, bug fixes

2011-09-26 Thread Peter Collingbourne
Hi,

Now that the Ninja generator is passing all tests on Linux, I have
decided to start preparing patches, and I am starting with some
refactorings and bug fixes developed in the course of developing the
Ninja generator.  Three of the patches were originally by Nicolas
Despres, the rest are by myself.

These patches are also available as a git branch on top of master:
git://github.com/pcc/CMake.git ninja-generator-prep

Please review!

Thanks,
-- 
Peter

--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


[cmake-developers] [PATCH 5/7] Introduce a cmLocalGenerator::ConvertToIncludeReference function

2011-09-26 Thread Peter Collingbourne
This provides a mechanism for the local generator to override how
header search paths are generated.
---
 Source/cmLocalGenerator.cxx |9 -
 Source/cmLocalGenerator.h   |4 +++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index 6af7fd5..aeda164 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -1185,6 +1185,13 @@ 
cmLocalGenerator::ConvertToOutputForExisting(RelativeRoot remote,
 }
 
 //
+std::string
+cmLocalGenerator::ConvertToIncludeReference(std::string const& path)
+{
+  return this->ConvertToOutputForExisting(path.c_str());
+}
+
+//
 const char* cmLocalGenerator::GetIncludeFlags(const char* lang,
   bool forResponseFile)
 {
@@ -1285,7 +1292,7 @@ const char* cmLocalGenerator::GetIncludeFlags(const char* 
lang,
   }
 else
   {
-  includePath = this->ConvertToOutputForExisting(i->c_str());
+  includePath = this->ConvertToIncludeReference(*i);
   }
 if(quotePaths && includePath.size() && includePath[0] != '\"')
   {
diff --git a/Source/cmLocalGenerator.h b/Source/cmLocalGenerator.h
index 69e452b..c0fe886 100644
--- a/Source/cmLocalGenerator.h
+++ b/Source/cmLocalGenerator.h
@@ -179,7 +179,9 @@ public:
   path and short path if spaces.  */
   std::string ConvertToOutputForExisting(RelativeRoot remote,
  const char* local = 0);
-  
+
+  virtual std::string ConvertToIncludeReference(std::string const& path);
+
   /** Called from command-line hook to clear dependencies.  */
   virtual void ClearDependencies(cmMakefile* /* mf */, 
  bool /* verbose */) {}
-- 
1.7.5.3

--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers