ninja-build / ninja

a small build system with a focus on speed
https://ninja-build.org/
Apache License 2.0
11.05k stars 1.59k forks source link

Can msvc_deps_prefix be set on a per-rule basis? #1043

Closed NiklasRosenstein closed 8 years ago

NiklasRosenstein commented 8 years ago

I'm working on a meta build system. Although very unlikely, it might be possible that a build script makes use of two different versions of the MSVC compiler which could theoretically be in different languages. Is it possible to set the msvc_deps_prefix on a per-rule basis?

evmar commented 8 years ago

It looks like it's looked up in the normal manner, which obeys all the scope rules:

https://github.com/martine/ninja/blob/a48434493d69085aa6fc0c002180ace555cad592/src/build.cc#L747

Does it not work if you set it on a per-rule basis?

NiklasRosenstein commented 8 years ago

Thanks @martine, I used an old Ninja version for which it didn't work. Works in the latest 1.6.0 :)

NiklasRosenstein commented 8 years ago

I have to re-open this issue as it does not seem to be possible to set variables for a rule, but only for a build instruction. I get

ninja: error: build.ninja:6: unexpected variable 'msvc_deps_prefix'
   msvc_deps_prefix = Note: including file:

for

rule test
  command = cl $in /Fe$out /showIncludes
  msvc_deps_prefix = Note: including file:

with Ninja 1.6.0 on OSX. Wondering why I thought it worked before, and I'm search for the test I used. Maybe I specified the variable on the build part there.

Note: I'm just testing the syntax here, I don't actually want to build stuff right now. It doesn't make sense to use cl on OSX. :) Btw. Ninja says ninja: fatal: unknown deps type 'msvc' when I set deps = msvc. Does Ninja only supports this value on a Windows build?

Edit: Updated some spelling, corrected example and added info.

NiklasRosenstein commented 8 years ago

Looking at the documentation, msvc_deps_prefix is supported as a rule variable since 1.5.0. Is the error I find because I'm testing it with a Mac build?

NiklasRosenstein commented 8 years ago

Just tested this on Windows.

# this file was automatically generated by Craftr-2.0.0-dev
# https://github.com/craftr-build/craftr

rule nr.test.objects
  command = cl /nologo /c $in /Fo$out /Ic:\users\niklas\desktop\craftr-2.0.0\nr.test\include /EHsc /W4 /Ox /showIncludes
  deps = msvc
  msvc_deps_prefix = Note: including file:
build nr.test\obj\src\main.obj: nr.test.objects c$:\users\niklas\desktop\craftr-2.0.0\nr.test\src\main.cpp
build nr.test.objects: phony nr.test\obj\src\main.obj

rule nr.test.program
  command = link /nologo $in /OUT:$out
build nr.test\main.exe: nr.test.program nr.test\obj\src\main.obj
build nr.test.program: phony nr.test\main.exe

Gives me

λ ninja
ninja: error: build.ninja:6: unexpected variable 'msvc_deps_prefix'
  msvc_deps_prefix = Note: including file:
                                          ^ near here

with Ninja 1.6.0

NiklasRosenstein commented 8 years ago

https://github.com/ninja-build/ninja/blob/a48434493d69085aa6fc0c002180ace555cad592/src/eval_env.cc#L65-L75

msvc_deps_prefix is not accepted as a reserved binding. I'll submit a patch.