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

Improve detection of msvc environment when compiling from msvc #1310

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

dlon
Copy link
Contributor

@dlon dlon commented Dec 2, 2024

Fix #1308. This makes path/env detection a bit pickier by not using PATH if the compiler has an unexpected target. Previously this caused issues for build scripts since rustc always needs to target the native arch.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 3, 2024

cc @ChrisDenton can you take a look at this please?

(I'm not that familiar with windowd/msvc)

@ChrisDenton
Copy link
Member

Hmm, I did have to consider this a bit more. On balance, I think I'd rather keep the non-msbuild case the same as it was to avoid unintended side-effects.

So ideally this PR would only affect MSBuild.

@dlon
Copy link
Contributor Author

dlon commented Dec 4, 2024

@ChrisDenton Is this not achieved by checking that VSTEL_MSBuildProjectFullPath is set?

Note that this problem does affect building from VS as well, not only msbuild.

Copy link
Member

@ChrisDenton ChrisDenton left a comment

Choose a reason for hiding this comment

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

Ok, I've been testing this and I'm happy that it solves the issue. I do have one small suggestion.

@dlon dlon force-pushed the improve-msvc-check branch from 5ff0342 to 4605d62 Compare December 4, 2024 18:02
Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thanks!

@NobodyXu NobodyXu merged commit ec73df9 into rust-lang:main Dec 5, 2024
27 checks passed
@github-actions github-actions bot mentioned this pull request Dec 6, 2024
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.

Compiling from msbuild incorrectly uses target build tools for build scripts
3 participants