Skip to content

Verify MSBuild version in Developer CMD prompt#24544

Merged
jaredpar merged 4 commits intodotnet:dev15.7.xfrom
jaredpar:fix-it
Feb 2, 2018
Merged

Verify MSBuild version in Developer CMD prompt#24544
jaredpar merged 4 commits intodotnet:dev15.7.xfrom
jaredpar:fix-it

Conversation

@jaredpar
Copy link
Member

Roslyn is designed to have the simplest possible contribution story:
clone then build. Every pre-req needed is either located on the machine
or bootstrapped via NuGet. All the way down to using an xcopy MSBuild if
needed.

The one case which causes a problem is the VS command prompt. In this
case MSBuild is pre-installed on the machine and may or may not be
suitable for building Roslyn.

Previously when building from a VS command prompt we just used whatever
MSBuild was provided. The assumption being a developer command prompt
was an explicit statement of whath MSBuild you wanted to use. Based on
all of our customer reports though this does not seem to be the
assumption that consumers of our repo have. The build gave them no
explicit errors about the provided toolset and hence when the build
failed they assigned flakiness to our repo.

Going forward we are applying the same version validation to MSBuild
when provided via a developer command prompt. If it doesn't match we
will refuse to build asking the user to upgrade VS or build from a
normal command prompt.

Ask Mode template not completed

Customer scenario

What does the customer do to get into this situation, and why do we think this
is common enough to address for this release. (Granted, sometimes this will be
obvious "Open project, VS crashes" but in general, I need to understand how
common a scenario is)

Bugs this fixes

(either VSO or GitHub links)

Workarounds, if any

Also, why we think they are insufficient for RC vs. RC2, RC3, or RTW

Risk

This is generally a measure our how central the affected code is to adjacent
scenarios and thus how likely your fix is to destabilize a broader area of code

Performance impact

(with a brief justification for that assessment (e.g. "Low perf impact because no extra allocations/no complexity changes" vs. "Low")

Is this a regression from a previous update?

Root cause analysis

How did we miss it? What tests are we adding to guard against it in the future?

How was the bug found?

(E.g. customer reported it vs. ad hoc testing)

Test documentation updated?

If this is a new non-compiler feature or a significant improvement to an existing feature, update https://github.com/dotnet/roslyn/wiki/Manual-Testing once you know which release it is targeting.

Roslyn is designed to have the simplest possible contribution story:
clone then build. Every pre-req needed is either located on the machine
or bootstrapped via NuGet. All the way down to using an xcopy MSBuild if
needed.

The one case which causes a problem is the VS command prompt. In this
case MSBuild is pre-installed on the machine and may or may not be
suitable for building Roslyn.

Previously when building from a VS command prompt we just used whatever
MSBuild was provided. The assumption being a developer command prompt
was an explicit statement of whath MSBuild you wanted to use. Based on
all of our customer reports though this does not seem to be the
assumption that consumers of our repo have. The build gave them no
explicit errors about the provided toolset and hence when the build
failed they assigned flakiness to our repo.

Going forward we are applying the same version validation to MSBuild
when provided via a developer command prompt. If it doesn't match we
will refuse to build asking the user to upgrade VS or build from a
normal command prompt.
# Dose this version of Visual Studio meet our minimum requirements for building.
function Test-SupportedVisualStudioVersion([string]$version) {
if (-not ($version -match "^([\d.]+)(\+|-)?.*$")) {
Write-Host "here"
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this is leftover debugging detritus?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. Removed.


# Dose this version of Visual Studio meet our minimum requirements for building.
function Test-SupportedVisualStudioVersion([string]$version) {
if (-not ($version -match "^([\d.]+)(\+|-)?.*$")) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not just call Version.TryParse?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I need to strip off the prelease modifiers here.

Copy link
Member

Choose a reason for hiding this comment

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

@jaredpar Oh, that's what that regex does? I then find your lack of comments disturbing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment added

return
}
else {
throw "Developer Command Prompt for VS $(${env:VSCMD_VER}) is not recent enough. Please upgrade or build from a normal CMD window"
Copy link
Member

Choose a reason for hiding this comment

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

Should this say what the minimum version should be?

Copy link
Member Author

Choose a reason for hiding this comment

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

Considered that but seemed like it could be more confusing. You can't really upgrade to a specific version of VS, you just upgrade to latest. Hence why tell them a version they can't control?

Copy link
Member

Choose a reason for hiding this comment

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

If we ever make a change that demands a prerelease version, you'd want to know that?

Copy link
Member Author

Choose a reason for hiding this comment

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

"Please install 15.6.0-preview4." Not sure users are going to find that actionable over "Please install the latest preview of VS"

Copy link
Member

Choose a reason for hiding this comment

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

It will when a user is trying to use preview 4 and we haven't even shipped that yet.

@jaredpar
Copy link
Member Author

jaredpar commented Feb 1, 2018

ping

@jaredpar
Copy link
Member Author

jaredpar commented Feb 1, 2018

ping @dotnet/roslyn-infrastructure

@jaredpar jaredpar merged commit e2472c4 into dotnet:dev15.7.x Feb 2, 2018
@jaredpar jaredpar deleted the fix-it branch February 2, 2018 19:07
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.

2 participants