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

Setup MSVC environment in CI workflow #2942

Merged
merged 7 commits into from
Dec 7, 2024
Merged

Conversation

anmyachev
Copy link
Contributor

@anmyachev anmyachev commented Dec 5, 2024

Part of #2030
Part of #2824

For all other compilers the situation is about the same, it is expected that they are already in the paths. I don't think it should be any different for Windows.

Signed-off-by: Anatoly Myachev <[email protected]>
Signed-off-by: Anatoly Myachev <[email protected]>
Signed-off-by: Anatoly Myachev <[email protected]>
@anmyachev anmyachev changed the title Reuse CLFinder.py in setup.py Setup MSVC environment in CI workflow Dec 6, 2024
@anmyachev anmyachev requested review from gshimansky and pbchekin and removed request for gshimansky December 6, 2024 18:29
@anmyachev anmyachev marked this pull request as ready for review December 6, 2024 18:29
@anmyachev anmyachev linked an issue Dec 6, 2024 that may be closed by this pull request
Comment on lines +49 to +53
cmd /c '"C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\Build\vcvarsall.bat" x64 && set' | ForEach-Object {
if ($_ -match '^(.*?)=(.*)$') {
[Environment]::SetEnvironmentVariable($matches[1], $matches[2])
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also work (and simpler).

Suggested change
cmd /c '"C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\Build\vcvarsall.bat" x64 && set' | ForEach-Object {
if ($_ -match '^(.*?)=(.*)$') {
[Environment]::SetEnvironmentVariable($matches[1], $matches[2])
}
}
Invoke-BatchFile "C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\Build\vcvarsall.bat"

Copy link
Contributor

Choose a reason for hiding this comment

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

Invoke-BatchFile is a non-standard extension which is not installed by default. I actually like that Anatoly found a solution that doesn't require installing it, we may want to use it instead. Also save environment variables into GITHUB_ENV for further steps.

Copy link
Contributor

Choose a reason for hiding this comment

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

We install a lot of custom stuff to the runner image, including support for Invoke-BatchFile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've rolled back the last changes and am going to merge the version that was approved. Feel free to ask me to finish something, I can do it in a separate pull request.

@anmyachev anmyachev merged commit b57f1b5 into main Dec 7, 2024
5 checks passed
@anmyachev anmyachev deleted the amyachev/windows-debug branch December 7, 2024 16:10
anmyachev added a commit that referenced this pull request Dec 7, 2024
Part of #2030
Part of #2824

For all other compilers the situation is about the same, it is expected
that they are already in the paths. I don't think it should be any
different for Windows.

---------

Signed-off-by: Anatoly Myachev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get rid of using CLFinder.py
3 participants