State of the `m̀svc` tool

[David Turner]

Is the Ninja `msvc` tool still needed?

This tool is only compiled in Ninja when the latter is buillt with the _MSVC, which seems surprising (other Windows-specific features use #ifdef _WIN32). In particular, it is not available when using Ninja with a cross-compiler like `clang-cl` that generates Windows binaries on Linux (which is how I encountered the issue).

Documentation is pretty terse, so looking at the implementation, I see it supports the following options:

`-e ENVFILE`, which seems to load a file that should contain an environment definition string (i.e. a list of VAR=VALUE statements separated with semi-columns) that will be used to set the environment before calling the `cl.exe` compiler. Note that the implementation also extract the PATH value from this file and pushes it into the current Ninja process' environment (without ever resetting it to the previous value). That last point really looks like a bug too.

`-o FILE` and `-p STRING` are essentially used to parse the output of cl.exe with the /showIncludes flag and generate a depfile for it. The -p option is only used to add a prefix to all paths encountered in the output).

In other words, it looks like this tool was used to generate depfiles in the gcc specific format a long time ago (it was introduced in 2012, last modified in 2013). There are 

It looks like this is no longer needed since Ninja now supports depfiles with the msvc format natively. If this is true, what about removing it? Any actual users of this feature anymore? Or should we fix the bugs in it?

[Nico Weber]

In general: We can't know if it's still needed. Ninja is backwards-compatible, so we won't remove stuff.

In particular: I'd expect that `-e` is used by most folks, since it's needed if you use MSVC's toolchain. clang-cl and lld-link are (almost) in a state where you can get away without `ninja -e`. Note that the way this is used is that your compile command runs as `ninja -e foo cl.exe …`, so the outer ninja that runs the build invokes `ninja -e`, so it shouldn't matter that the `ninja -e` process changes PATH. The `-e` process only exists for the duration of the cl.exe execution.

I'd expect that `-o` and `p` are not used heavily. But they also don't take up a ton of code, and we don't want to break people.

[Nico Weber]

Unless it's not working for some use case you have, I just wouldn't touch it. There's always the risk of breaking something, and if there's no upside of fixing a problem you have, why bother?

On Mon, Feb 14, 2022, 9:31 AM David Turner <di...@google.com> wrote:

Le lun. 14 févr. 2022 à 14:57, Nico Weber <tha...@chromium.org> a écrit :

In general: We can't know if it's still needed. Ninja is backwards-compatible, so we won't remove stuff.

In particular: I'd expect that `-e` is used by most folks, since it's needed if you use MSVC's toolchain. clang-cl and lld-link are (almost) in a state where you can get away without `ninja -e`. Note that the way this is used is that your compile command runs as `ninja -e foo cl.exe …`, so the outer ninja that runs the build invokes `ninja -e`, so it shouldn't matter that the `ninja -e` process changes PATH. The `-e` process only exists for the duration of the cl.exe execution.

I'd expect that `-o` and `p` are not used heavily. But they also don't take up a ton of code, and we don't want to break people.

I'm ok with not breaking the tool. it is still tagged as (EXPERIMENTAL) in the source tree so I had to ask. I'll try to fix the bugs I mentioned, but I don't have a Windows machine to test much so no promises :)
 

On Monday, February 14, 2022 at 7:59:36 AM UTC-5 David Turner wrote:

Is the Ninja `msvc` tool still needed?

This tool is only compiled in Ninja when the latter is buillt with the _MSVC, which seems surprising (other Windows-specific features use #ifdef _WIN32). In particular, it is not available when using Ninja with a cross-compiler like `clang-cl` that generates Windows binaries on Linux (which is how I encountered the issue).

Documentation is pretty terse, so looking at the implementation, I see it supports the following options:

`-e ENVFILE`, which seems to load a file that should contain an environment definition string (i.e. a list of VAR=VALUE statements separated with semi-columns) that will be used to set the environment before calling the `cl.exe` compiler. Note that the implementation also extract the PATH value from this file and pushes it into the current Ninja process' environment (without ever resetting it to the previous value). That last point really looks like a bug too.

`-o FILE` and `-p STRING` are essentially used to parse the output of cl.exe with the /showIncludes flag and generate a depfile for it. The -p option is only used to add a prefix to all paths encountered in the output).

In other words, it looks like this tool was used to generate depfiles in the gcc specific format a long time ago (it was introduced in 2012, last modified in 2013). There are 

It looks like this is no longer needed since Ninja now supports depfiles with the msvc format natively. If this is true, what about removing it? Any actual users of this feature anymore? Or should we fix the bugs in it?

--
You received this message because you are subscribed to a topic in the Google Groups "ninja-build" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/ninja-build/os1tUBfc4Pw/unsubscribe.
To unsubscribe from this group and all its topics, send an email to ninja-build...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ninja-build/8d7a4afc-54e2-4d01-8cb4-da094975a977n%40googlegroups.com.

[David Turner]

Le lun. 14 févr. 2022 à 14:57, Nico Weber <tha...@chromium.org> a écrit :

In general: We can't know if it's still needed. Ninja is backwards-compatible, so we won't remove stuff.

In particular: I'd expect that `-e` is used by most folks, since it's needed if you use MSVC's toolchain. clang-cl and lld-link are (almost) in a state where you can get away without `ninja -e`. Note that the way this is used is that your compile command runs as `ninja -e foo cl.exe …`, so the outer ninja that runs the build invokes `ninja -e`, so it shouldn't matter that the `ninja -e` process changes PATH. The `-e` process only exists for the duration of the cl.exe execution.

I'd expect that `-o` and `p` are not used heavily. But they also don't take up a ton of code, and we don't want to break people.

I'm ok with not breaking the tool. it is still tagged as (EXPERIMENTAL) in the source tree so I had to ask. I'll try to fix the bugs I mentioned, but I don't have a Windows machine to test much so no promises :)
 

On Monday, February 14, 2022 at 7:59:36 AM UTC-5 David Turner wrote:

Is the Ninja `msvc` tool still needed?

This tool is only compiled in Ninja when the latter is buillt with the _MSVC, which seems surprising (other Windows-specific features use #ifdef _WIN32). In particular, it is not available when using Ninja with a cross-compiler like `clang-cl` that generates Windows binaries on Linux (which is how I encountered the issue).

Documentation is pretty terse, so looking at the implementation, I see it supports the following options:

`-e ENVFILE`, which seems to load a file that should contain an environment definition string (i.e. a list of VAR=VALUE statements separated with semi-columns) that will be used to set the environment before calling the `cl.exe` compiler. Note that the implementation also extract the PATH value from this file and pushes it into the current Ninja process' environment (without ever resetting it to the previous value). That last point really looks like a bug too.

`-o FILE` and `-p STRING` are essentially used to parse the output of cl.exe with the /showIncludes flag and generate a depfile for it. The -p option is only used to add a prefix to all paths encountered in the output).

In other words, it looks like this tool was used to generate depfiles in the gcc specific format a long time ago (it was introduced in 2012, last modified in 2013). There are 

It looks like this is no longer needed since Ninja now supports depfiles with the msvc format natively. If this is true, what about removing it? Any actual users of this feature anymore? Or should we fix the bugs in it?

[David Turner]

Le lun. 14 févr. 2022 à 16:19, Nico Weber <tha...@chromium.org> a écrit :

Unless it's not working for some use case you have, I just wouldn't touch it. There's always the risk of breaking something, and if there's no upside of fixing a problem you have, why bother?

The usual reasons: because that code has obvious but minor bugs, has no regression tests at all, so we don't know if some parts have bit-rotted or not, and because maintaining it is a tax on future refactorings (e.g. some people want to turn Ninja into a library, where changing PATH in the middle of a Ninja run is a really bad idea). 
I found that it is still used by the Chromium build though, so at least one valid reason to keep it: https://source.chromium.org/chromium/chromium/src/+/main:build/toolchain/win/BUILD.gn;drc=364c9f71522ca2e7ce2fb1f155cd558bf480a8d5;l=141