cc-rs icon indicating copy to clipboard operation
cc-rs copied to clipboard

windows_registry::find functions cannot find VS-installed LLVM tools

Open Qyriad opened this issue 1 year ago • 12 comments

cc-rs hardcodes "VC\Tools\MSVC" when searching for tools, but LLVM tools are in "VC\Tools\Llvm", which prevents finding things like clang-cl if it not actively being used by any Builds.

Qyriad avatar Jul 14 '22 13:07 Qyriad

I spy unjustified "but" :-) But on a serious note, the wording suggests that spotting Llvm is something that would be necessary, while it's by all means optional. Well, one can make a case for improving user experience, but one can't express it that categorically. Most notably, we don't know if the user has installed a newer/different clang version by herself. So one can say that cc-rs could detect if there is clang-cl on the program search path (and check that it matches the target platform?) and only then attempt to see if it's installed as part of VS... But, again, one can't really put it as "must find LLVM tools over there."

dot-asm avatar Jul 22 '22 21:07 dot-asm

To be clear, this isn't an issue about arbitrary installations of LLVM tools — LLVM tools are installed to VC\Tools\Llvm specifically when installed as a Visual Studio component, and the windows_registry module is specifically for finding Visual Studio metadata and components. Not being able to find one specific subcategory of Visual Studio components for a generalized API is an incompleteness that should be, if not fixed, at least documented.

Qyriad avatar Jul 22 '22 22:07 Qyriad

this isn't an issue about arbitrary installations of LLVM tools — LLVM tools are installed to VC\Tools\Llvm specifically when installed as a Visual Studio component

At the same time if user has installed "arbitrary" LLVM toolkit and made arrangements to use specifically that, it would be inappropriate to override the choice. Would you agree?

the windows_registry module is specifically for finding Visual Studio metadata and components.

It's "a helper module to probe the Windows Registry when looking for windows-specific tools," and explicitly refers to them as "tools within an MSVC installation." So that formally speaking it does exactly what is documented :-)

dot-asm avatar Jul 25 '22 15:07 dot-asm

At the same time if user has installed "arbitrary" LLVM toolkit and made arrangements to use specifically that, it would be inappropriate to override the choice. Would you agree?

You can make the identical argument for the MSVC compiler itself. I see no reason why the LLVM tooling is any different in this case.

It's "a helper module to probe the Windows Registry when looking for windows-specific tools," and explicitly refers to them as "tools within an MSVC installation." So that formally speaking it does exactly what is documented :-)

These LLVM tools, specifically clang-cl.exe are themselves Windows specific tools, which Microsoft has specifically blessed by including them as a mainline component of their toolchain installer. Referencing the documentation for clang-cl , we find:

When Clang compiles C++ code for Windows, it attempts to be compatible with MSVC. There are multiple dimensions to compatibility. ... Finally, there is clang-cl, a driver program for clang that attempts to be compatible with MSVC’s cl.exe.

Furthermore, this component is expected to be on the path when working with MSVC command-line tools environment when the component is installed as referenced here, so clearly it is something that is expected to be on the path as part of the environment and is very much considered part of what a reasonable person would consider a MSVC installation.

I find this to be something at least justifying additional, clearer, documentation; as if this is truly something considered out of scope for this project, then the existing documentation does not make this clear enough.

Lunaphied avatar Jul 26 '22 20:07 Lunaphied

At the same time if user has installed "arbitrary" LLVM toolkit and made arrangements to use specifically that, it would be inappropriate to override the choice. Would you agree?

I wouldn't. This isn't the behavior for MSVC, so why would it be the behavior for LLVM? If I set $CC to a non-standard cl.exe instance, I'd certainly agree that cc::Build should attempt to use that for building libraries, but that shouldn't (and doesn't currently) affect the behavior of cc::windows_registry::find() and friends. As @modwizcode said, I don't see why that would be any different for LLVM.

Qyriad avatar Jul 26 '22 21:07 Qyriad

At the same time if user has installed "arbitrary" LLVM toolkit and made arrangements to use specifically that, it would be inappropriate to override the choice. Would you agree?

You can make the identical argument for the MSVC compiler itself. I see no reason why the LLVM tooling is any different in this case.

Because that's how it worked so far. It was user's responsibility to make the arrangements. And the fact that Microsoft "blessed" a specific clang version doesn't have to mean that users should be denied the choice. I didn't say that there is no room improvement (see my initial remark), what I'm trying to say is that overriding the user preferences wouldn't be an improvement.

Anyway, the cards are on the table, it's up to maintainers to choose the path :-)

dot-asm avatar Jul 26 '22 21:07 dot-asm

I'm not sure where a notion of overriding user preferences or denying users a choice is coming from. I haven't proposed anything like that in this issue.

Qyriad avatar Jul 26 '22 21:07 Qyriad

I don't follow. I asked if you would agree that overriding user choice would be inappropriate. And you didn't agree, i.e. you would find overriding user preferences appropriate. And doesn't "This isn't the behavior for MSVC, so why would it be the behavior for LLVM?" mean that the suggestion is that Microsoft-"blessed" version should take precedence [over "arbitrary" clang installation]?

dot-asm avatar Jul 26 '22 22:07 dot-asm

Can you explain what you mean by "user choice" and "user preferences" here? What kind of user preferences would you expect to affect the functionality of windows_registry::find() and co, and how?

Qyriad avatar Jul 27 '22 05:07 Qyriad

Quoting my initial remark, "detect if there is clang-cl on the program search path (and check that it matches the target platform?) and only then attempt to see if it's installed as part of VS." Does it actually take an explicit spelling out that "having clang-cl on the program search path" is a choice made by the user? And that "a choice" is an expression of preference [which makes it possible to use them to a degree interchangeably]?

Either way, the question was about giving precedence to the "clang-cl on the program search path" over "but LLVM tools are in "VC\Tools\Llvm"". I advocate for prioritizing the former, while your answer suggested that you advocate for the latter. These are the "cards on the table," which clang-cl gets the priority.

dot-asm avatar Jul 27 '22 15:07 dot-asm

There is no precedent for grabbing arbitrary tools in the user's PATH with windows_registry::find(). That is not current functionality, and I don't propose adding that functionality in this issue. My proposal is no more overriding user preferences than the exact current functionality.

If you think that the find-family of functions in this module should respect such kinds of preferences, consider opening a separate issue 🙂

Qyriad avatar Jul 27 '22 17:07 Qyriad

Could you describe how do you envision it would work in a build script? Would I call windows_registry::find("clang-cl") in order to figure out if it's installed [as part of VS]? And then pass it on to .compiler() method. This is provided that I, as a crate maintainer, prefer my crate to be compiled specifically with clang for whatever reason. To the extent that I can choose to fail the compilation if clang-cl is not installed. Is this it? Not that it would be an inherently wrong approach, I just want to understand. It's just that I for one find it way more natural to let user decide which compiler to use. And preferably in a more platform-neutral way [or rather independently of the build script]. I mean expectation is that user would set CC environment variable to clang on Linux and clang-cl on Windows and off it goes...

dot-asm avatar Aug 01 '22 18:08 dot-asm