Skip to content

Fix MSB4086 if LangVersion is a keyword #1420

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

Merged
merged 1 commit into from
Apr 9, 2020
Merged

Fix MSB4086 if LangVersion is a keyword #1420

merged 1 commit into from
Apr 9, 2020

Conversation

martincostello
Copy link
Member

@martincostello martincostello commented Apr 8, 2020

Fix compilation error MSB4086 with version 0.12.1 if the LangVersion MSBuild property is already set to one of the keywords such as preview, latest and latestMajor (docs).

Otherwise MSBuild complains that it cannot perform a numeric comparison and compilation fails.

Example compilation error from a project with <LangVersion>latest</LangVersion> specified in a Directory.Build.props file in the repository root:

\BenchmarkDotNet.Autogenerated.csproj(20,18): error MSB4086: A numeric comparison was attempted on "$(LangVersion)" that evaluates to "latest" instead of a number, in condition "'$(LangVersion)' == '' Or '$(LangVersion)' < '7.3'".

Open to alternative suggestions for the fix, but I've gone with just checking whether the first character of the existing value is a digit (implying it's a version number) before performing the comparison. Otherwise it assumes that the keyword resolves to a version greater than 7.3.

I'm also not sure on what the best way to add a regression test for this, again open to suggestions.

Issue appears to have been introduced by #1389 (d02c8c3).

Fix compilation error MSB4086 if LangVersion is already set to one of the keywords such as preview, latest and latestMajor. Otherwise MSBuild complains that it cannot perform a numeric comparison.
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

@martincostello big thanks for providing the fix!

@adamsitnik
Copy link
Member

@eerhardt could you please take a look? I want to make sure that it's the best fix possible (it looks good to me)

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

I think the fix looks good.

The only other possibility I could imagine would be using the new functionality in dotnet/msbuild#4911: $([MSBuild]::VersionLessThan(a, b)). But I don't think there is a IsVersion or TryParse kind of method there. So I think what we have here is fine.

@adamsitnik
Copy link
Member

@eerhardt thank you!

@adamsitnik adamsitnik merged commit 82b15ee into dotnet:master Apr 9, 2020
@adamsitnik
Copy link
Member

@AndreyAkinshin this bug fix and the Xamarin support make it a very good candidates to release 0.12.2 :D

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.

4 participants