Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Meta issue] windows_registry and the Visual Studio environment #722

Open
ChrisDenton opened this issue Sep 15, 2022 · 7 comments
Open

[Meta issue] windows_registry and the Visual Studio environment #722

ChrisDenton opened this issue Sep 15, 2022 · 7 comments
Labels
O-windows Windows targets and toolchains

Comments

@ChrisDenton
Copy link
Member

There's been a bit of discussion recently spread across a number of PRs1 so I'd like to centralize discussion and explain my position here. What follows is my view of the situation. My hope is that a consensus can be found or, if not, then at least we can gain a better understanding of where everyone is coming from.

windows_registry2 can be used to query Visual Studio for the path of tools and to get associated libraries, binaries and header files. For example, rustc uses it to find link.exe and also to link required libraries such as the crt and import libraries such as kernel32.lib.

cc-rs finds the tool by first attempting to use current environment variables. These are set up when using a Visual Studio shell or by something else setting the build environment.

However, if the environment is not set, is incomplete, or it is incompatible with the build target (e.g. x86 vs. x64) then cc-rs will use the Visual Studio Installer API to query for the requested tool and other paths.

In this case it mostly ignores the environment variables and prioritises its own, hard coded, set of defaults. This means that any preferences are lost and must be manually reapplied by users of cc-rs. This can also make it harder to integrate with other build systems. My view is that any discrepancies this causes should be considered a bug in cc-rs.

Note that there are currently two case where we do use environment defaults: when the target doesn't match the environment the Visual Studio directory is taken from the environment and when looking for the Windows API SDK, it will first check environment variables instead of always using the latest.


So my question is on a point of policy. What is the role of cc-rs here? Should should it defer to the environment when setting initial defaults or should it continue to (mostly) override the environment with its own hard-coded defaults?

cc @dot-asm @thomcc

Footnotes

  1. Notably in #673 and #687 (though there's a lot of back and forth as people try to understand each other's position).

  2. The name "windows_registry" is a bit of a misnomer these days.

@dot-asm
Copy link
Contributor

dot-asm commented Sep 17, 2022

It's important to recognize and keep in mind that cc-rs is effectively a dual-purpose crate. On one hand it compiles C/C++ modules and collects them into a library, and on another hand it facilitates linking of all Rust binaries [irregardless of whether or not a C/C++ module was compiled]. We're talking about the latter here, and it affects all *-pc-windows-msvc users, even those who have never heard of cc-rs.

To spare time, my position on the issue is that if one trusts %PATH% to locate [suitable] link.exe, there is no reason to not fully trust %LIB% environment variable. I recognize that it can trigger some undesired side effects and make it error-prone. It's just that I customarily advocate for more power/control to the users. Of course at the cost of greater responsibility, in the sense that any errors resulting from prioritizing %LIB% would be on user to resolve. After all, they can always re-run without it set ;-)

@dot-asm
Copy link
Contributor

dot-asm commented Sep 17, 2022

This means that any preferences are lost and must be manually reapplied by users of cc-rs.

With "dual-purpose"-ness in mind I want to emphasize that this would be rather about cc-rs's first purpose. And the thing is that developers who use it to interoperate with C can't make assumptions about somebody else's environment, and hence have to assume the worst in either case and make their build.rs act accordingly and enforce the preferences. Well, one can make a case for improving their experience...

@ChrisDenton
Copy link
Member Author

If we reach the point of auto-discovery then we are already overriding %LIB% (as well as %INCLUDE% and %PATH% for that matter), just with potentially the wrong libraries.

@dot-asm
Copy link
Contributor

dot-asm commented Sep 18, 2022

If we reach the point of auto-discovery then we are already overriding %LIB% (as well as %INCLUDE% and %PATH% for that matter),

This sounds "orthogonal" to what I'm suggesting. I mean I'm suggesting "trust %PATH-%LIB% combo", but it says nothing about my position on the auto-discovery scenario. Because %PATH% takes precedence over auto-discovery [right?].

just with potentially the wrong libraries.

Well, I can imagine one can legitimately argue that in most cases it would be more sustainable to disregard %LIB% in the auto-discovery case. Because it affects all Rust users, not only those who interoperate with C/C++. Or in other words, an average Rust user has no reason to trust %LIB% that is not accompanied with %PATH% pointing to link.exe.

@ChrisDenton
Copy link
Member Author

Because %PATH% takes precedence over auto-discovery [right?]

Not really. There are at least a couple of issues with blindly using PATH and hoping for the best. For example, say link.exe is in the path:

  • What if link.exe is not msvc link? This is a common problem, especially for users of the msys2 environment
  • What if compiling for more than one target (e.g. Rust build scripts are always built for the host regardless of the target)? In that case PATH and LIB cannot satisfy the conflicting requirements.
  • What if LIB is set but does not contain the required libraries?

Also note that preventing auto discovery in this case may be considered a breaking change for rustc. It could create a fork but making rustc use different rules then cc-rs is likely to lead issues if a different tooling is used for rustc and C/C++ dependencies.

@dot-asm
Copy link
Contributor

dot-asm commented Oct 5, 2022

Because %PATH% takes precedence over auto-discovery [right?]

Just in case, it was a genuine question. What I meant was rather "[as far as I understand] there is a mechanism to override auto-discovery, and it's %PATH%, right?"

Not really.

Ah! [Thanks!] It's not %PATH% alone. It uses the combination of %VSINSTALLDIR% and %VSCMD_ARG_TGT_ARCH% as a "gatekeeper" to %PATH%. Fair enough. I suppose my question can be refined as follows. Once %PATH% is vetted, is there a reason to distrust %LIB%?

... may be considered a breaking change for rustc.

Right. (As effectively acknowledged in my initial remark;-).

issues if a different tooling is used for rustc and C/C++ dependencies.

I want to remind you about the "dual-purpose" nature of the cc-rs. I would argue that it would be more appropriate to reach some consensus about the linking part first, and then consider what it means to interop with C/C++ :-)

@thomcc thomcc added the O-windows Windows targets and toolchains label Oct 29, 2022
@dpaoliello
Copy link
Contributor

I don't think we'll ever be able to move cc-rs into either a pure "discover only" or "use ambient env" mode, but what would be helpful (as someone who is trying to integrate Rust into a few different external build systems) is documentation on when cc-rs does each. Essentially a flow chart that says "if cc-rs sees [thing] then it will use that, otherwise it will check for [thing]".

This logic all already exists within cc-rs, but documenting it and making it part of the contract to the user is vital - then we can have design discussions for each of those decision points (e.g., does having the correct linker on the PATH mean that LIB is used as-is or discovered?) and changes like #673 can be added to the documentation to show how cc-rs now has new decision points.

I'm also volunteering myself to write said documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Windows targets and toolchains
Projects
None yet
Development

No branches or pull requests

4 participants