[cmake-developers] [PATCH for bug 15253] Ninja: Pass only the flags relevant to the language.

[Brian Smith]

Brad King <brad.king at kitware.com> wrote:
> On 01/29/2015 03:12 AM, Brian Smith wrote:
>> I don't see how to make CMake execute yasm -M -o <x>.obj <x>.asm > $DEP_FILE
>> and then execute yasm -o <x>.obj <x>.asm to do the actual assembly.
>
> The CMAKE_<LANG>_COMPILE_OBJECT value can be set to a list of multiple
> strings.  Each will be invoked in order.  I haven't tested this though.
> Also it would have to be set up in a generator-specific manner for Ninja.

I thought about trying that. Ninja generator unconditionally appends
CMAKE_DEPFILE_FLAGS_<lang> to $FLAGS for the compile command.
Presumably, if there were to commands, it would append
CMAKE_DEPFILE_FLAGS_<lang> to the flags of both commands. But, in this
case, it should only be incorporated into one of the two. It seems
like CMAKE_<LANG>_COMPILE_OBJECT needs to have another parameter
<DEPFILE_FLAGS> distinct from <FLAGS> to handle this. But, that means
would require modifying all the templates that use
CMAKE_DEPFILE_FLAGS_<lang> and all the generators, which is a lot of
work.

>> But, when I execute ninja and it executes the rule, it never provides
>> a value for $DEP_FILE. Instead, the executed command looks like this:
>
> Please try out this commit:
>
>  Ninja: Use "deps = msvc" only for C and CXX
>  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=2596f95e

I tried out your change. With it, BoringSSL will build without
warnings using yasm, which was my original goal. Also, I verified that
even without adding my changes to use "-MD" or "-M", ninja does
rebuild the obj file from the asm file.

> Then rebase your changes on it.

It seems like the support for -MD that I've written isn't useful for
my needs, since I don't use .include. Also, even if I added -MD to
yasm, that solution wouldn't work for any older versions of yasm. It
seems like it would be better to have support for the two-step
compilation process (compile; then generate dep file), but that's too
much for me to take on right now.

Thanks a lot! I really appreciate you fixing this.

Also, note that this bug was originally reported by "karlhungus" in
the bug tracker, but your commit message says it was reported by me.
It would be great if you could give "karlhungus" the credit for
reporting the bug instead of me when you merge it into the trunk.

Thanks again,
Brian