Skip to content

Conversation

@MichalStrehovsky
Copy link
Member

@MichalStrehovsky MichalStrehovsky commented Jan 6, 2022

init-vs-env.cmd is going to set up the environment for x86 tools (the "VS Developer Command Prompt") and that messes with things if they run before we run vcvarsall to set the actual target architecture, so it's better not to have such window. We might not even run the vcvarsall line if native test build is skipped.

As an additional improvement, init-vs-env.cmd can also run vcvarsall and set up CMake, so I'm asking it to do it (done by passing an extra parameter to the script to specify which environment to activate).

I need this change for a subsequent change that is broken if we have x86 tools set up.

`init-vs-env.cmd` is going to set up the environment for x86 tools and that messes with things if they run before we run vcvarsall to set the actual target architecture, so it's better not to have such window.

As an additional improvement, init-vs-env.cmd can also run vcvarsall (that we do a couple lines below where I'm moving this), and find CMake (we do that a couple lines above). I could collapse those two, but we'll lose some console logging (init-vs-env doesn't log anything it does to the console), so not sure if that's an improvement.
@ghost
Copy link

ghost commented Jan 6, 2022

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

init-vs-env.cmd is going to set up the environment for x86 tools and that messes with things if they run before we run vcvarsall to set the actual target architecture, so it's better not to have such window.

As an additional improvement, init-vs-env.cmd can also run vcvarsall (that we do a couple lines below where I'm moving this), and find CMake (we do that a couple lines above). I could collapse those two, but we'll lose some console logging (init-vs-env doesn't log anything it does to the console), so not sure if that's an improvement.

Author: MichalStrehovsky
Assignees: MichalStrehovsky
Labels:

area-Infrastructure-coreclr

Milestone: -

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup, thank you!

@MichalStrehovsky MichalStrehovsky merged commit 03c5dbc into main Jan 7, 2022
@MichalStrehovsky MichalStrehovsky deleted the MichalStrehovsky-patch-1 branch January 7, 2022 22:44
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants