Skip to content

use 4.2 version of the compiler to build#60065

Merged
jmarolf merged 1 commit intodotnet:mainfrom
jmarolf:infrastructure/update-compiler-version
Mar 16, 2022
Merged

use 4.2 version of the compiler to build#60065
jmarolf merged 1 commit intodotnet:mainfrom
jmarolf:infrastructure/update-compiler-version

Conversation

@jmarolf
Copy link
Copy Markdown
Contributor

@jmarolf jmarolf commented Mar 9, 2022

This brings us to the latest public preview of VS and allows the use of new features like raw string literals.

@jmarolf jmarolf requested a review from a team as a code owner March 9, 2022 21:33
@ghost ghost added the Area-Infrastructure label Mar 9, 2022
davidwengier
davidwengier approved these changes Mar 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

‼️

@jmarolf jmarolf requested a review from sharwell March 9, 2022 21:36
@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Mar 9, 2022

@sharwell I want to tag you on this in case there is something I am missing about why we can't just make this change.

@jmarolf jmarolf enabled auto-merge March 9, 2022 21:37
@RikkiGibson
Copy link
Copy Markdown
Member

RikkiGibson commented Mar 9, 2022

#59903 (comment)

@jmarolf jmarolf disabled auto-merge March 9, 2022 21:53
@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Mar 9, 2022

#59903 (comment)

gol darn it you are right. can you block this PR? I just checked the internal schedule and we need to wait jus' a bit.

RikkiGibson
RikkiGibson previously requested changes Mar 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

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

just wait a little longer. Also, we can do this by updating SDK, code style packages etc.

@davidwengier
Copy link
Copy Markdown
Member

davidwengier commented Mar 9, 2022

Can someone explain why we need to wait for preview 2, when it works in preview 1?

@runfoapp runfoapp bot mentioned this pull request Mar 9, 2022
Closed
@sharwell
Copy link
Copy Markdown
Contributor

sharwell commented Mar 9, 2022

just wait a little longer. Also, we can do this by updating SDK, code style packages etc.

MicrosoftNetCompilersToolsetVersion overrides the SDK version.

sharwell
sharwell approved these changes Mar 9, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

This is a fairly standard change for us.

@RikkiGibson RikkiGibson dismissed their stale review March 9, 2022 23:25

If this really does what we are expecting it to do (enable use of raw string literals) then I don't have an objection to taking it.

@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Mar 15, 2022

/azp run roslyn-integration-CI

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Mar 15, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Mar 16, 2022

/azp run roslyn-integration-corehost

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Mar 16, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Mar 16, 2022

Preview 2 is out merging...

@jmarolf jmarolf merged commit 53162ef into dotnet:main Mar 16, 2022
@jmarolf jmarolf deleted the infrastructure/update-compiler-version branch March 16, 2022 18:27
@333fred
Copy link
Copy Markdown
Member

333fred commented Mar 16, 2022

@jmarolf is there a version of the SDK that our non-VS users (me) can use here?

@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Mar 16, 2022

shouldn't need anything on the omnisharp side, what errors are you seeing?

@333fred
Copy link
Copy Markdown
Member

333fred commented Mar 16, 2022

shouldn't need anything on the omnisharp side, what errors are you seeing?

I'm assuming that I will see errors. Haven't actually tried yet though.

@333fred
Copy link
Copy Markdown
Member

333fred commented Mar 16, 2022

Ok, I gave it a try. Raw string literals are not recognized.

@ghost ghost added this to the Next milestone Mar 16, 2022
@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Mar 16, 2022

ok, might need to talk with @JoeRobich about this one. My expectation is that if we set DOTNET_ROOT like the SDK team does here: https://github.com/dotnet/sdk/blob/dd53a44b49cc8dbe85c5fdfaee1560bc3cc96d61/eng/dogfood.ps1#L47 then O# will pick that sdk up and use it if this option is set to true:

roslyn/.devcontainer/devcontainer.json

Line 40 in 159eea2

"omnisharp.useModernNet": true,

So we should just need to update restore.sh to install the right SDK and set the right variables. We already call this script when setting up a new environment here:

roslyn/.devcontainer/devcontainer.json

Line 56 in 159eea2

"postCreateCommand": "${containerWorkspaceFolder}/restore.sh"

@333fred can you test setting DOTNET_ROOT and launching O# on linux? If that works then we should just need to update the global.json file to install 6.0.300 and the set the right environment variables in restore.sh

@333fred
Copy link
Copy Markdown
Member

333fred commented Mar 16, 2022

@jmarolf and I are looking offline, but the short answer is no, this does not work. We need to avoid using raw string literals and other new C# preview features for the moment.

@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Mar 16, 2022

yeah, will need to block PRs for a bit until the new Omnisharp build comes out.

@RikkiGibson
Copy link
Copy Markdown
Member

RikkiGibson commented Mar 17, 2022

Does this mean that OmniSharp uses its own in-box compiler even when it's using a runtime given by DOTNET_ROOT or similar?

@333fred
Copy link
Copy Markdown
Member

333fred commented Mar 17, 2022

Yes.

@allisonchou allisonchou modified the milestones: Next, 17.2.P3 Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidwengier davidwengier davidwengier approved these changes

@RikkiGibson RikkiGibson RikkiGibson left review comments

+1 more reviewer

@sharwell sharwell sharwell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-Infrastructure

Projects

None yet

Milestone

17.2.P3

Development

Successfully merging this pull request may close these issues.

6 participants

@jmarolf @RikkiGibson @davidwengier @sharwell @333fred @allisonchou