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

Unpin C# compiler version for repo #65317

Closed
stephentoub opened this issue Feb 14, 2022 · 12 comments · Fixed by #72145
Closed

Unpin C# compiler version for repo #65317

stephentoub opened this issue Feb 14, 2022 · 12 comments · Fixed by #72145

Comments

@stephentoub
Copy link
Member

We're currently manually pinning the C# compiler version:

<!-- TODO: Remove pinned compiler version once arcade supplies runtime with a compiler capable of handling !! -->
<MicrosoftNetCompilersToolsetVersion>4.2.0-2.22105.4</MicrosoftNetCompilersToolsetVersion>

as the version currently specified from dotnet/arcade is too old:

4.1.0-2.21609.7 (81831342)

We need to ensure the compiler bits are flowing appropriately and unpin.

@stephentoub stephentoub added this to the 7.0.0 milestone Feb 14, 2022
@ghost
Copy link

ghost commented Feb 14, 2022

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

We're currently manually pinning the C# compiler version:

<!-- TODO: Remove pinned compiler version once arcade supplies runtime with a compiler capable of handling !! -->
<MicrosoftNetCompilersToolsetVersion>4.2.0-2.22105.4</MicrosoftNetCompilersToolsetVersion>

as the version currently specified from dotnet/arcade is too old:

4.1.0-2.21609.7 (81831342)

We need to ensure the compiler bits are flowing appropriately and unpin.

Author: stephentoub
Assignees: -
Labels:

area-Infrastructure

Milestone: 7.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 14, 2022
@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Mar 28, 2022
@hoyosjs
Copy link
Member

hoyosjs commented Aug 10, 2022

@stephentoub @AaronRobinsonMSFT I see runtime has this pinned at:

<MicrosoftNetCompilersToolsetVersion>4.4.0-1.22358.14</MicrosoftNetCompilersToolsetVersion>

While arcade is at 4.3.0-3.22401.3. My guess is this is not recent enough for all the ref work, file scoped classes, and other goodness the runtime has taken a dependency on? If not, then should this issue be moved to .NET 8, or should we close it and accept that we move faster than arcade with respect to language feature usage so inhouse management is better?

@jkoritzinsky
Copy link
Member

When we get the P7 SDK (which I'm upgrading Arcade to as we speak), we can unpin the compiler as it has the final version of 4.4.0-1.xxxxx.

@hoyosjs
Copy link
Member

hoyosjs commented Aug 11, 2022

@jkoritzinsky since you are picking up the SDK upgrade after the arcade update, I'll assign this to you (and I'll link it in the PR).

@hoyosjs hoyosjs linked a pull request Aug 11, 2022 that will close this issue
@jkoritzinsky
Copy link
Member

Based on #72145 (comment), we won't be able to unpin the C# compiler upgrade just yet. There's a bug in the P7 Roslyn that wasn't fixed until after P7 was built.

@jkoritzinsky
Copy link
Member

This wasn't able to be fixed in #72145 because of a regression in Roslyn. We'll fix this before we release (likely after RC1).

@jkoritzinsky
Copy link
Member

I don't think we'll be able to do this for the GA release. We recently accepted a change to update the Roslyn compiler for some ref-fields-related work (#75156) that only exists in the RC2 SDK. As a result, we'd need to either update to a nightly SDK for the release/7.0 branch or wait until after RC2 is finalized (so basically right before GA) to fix this. Additionally, there's another fix incoming from Roslyn that might be inserted into release/7.0, which would make it nearly impossible to fix this before GA without updating to a nightly SDK.

@stephentoub are we okay with fixing this in servicing when we update the release/7.0 branch to build against the released 7.0.100 sdk?

@stephentoub
Copy link
Member Author

That's fine from my perspective. I just want to make sure we get onto the official train in both servicing and main as soon as we can so that we stay up-to-date with fixes/improvements that come into the compiler.

I think we should also consider flowing compiler updates into dotnet/runtime separate from the rest of the SDK. Updating the compiler once every three months hasn't been a successful strategy.

@jkoritzinsky
Copy link
Member

I fully agree with moving to flowing compiler updates to dotnet/runtime instead of using the LKG version from the SDK. I think that will provide us with a much better experience and it will keep us up to date. It will also help us avoid some of the source-build issues we’ve had this release.

I’ll mark this as blocking servicing instead of blocking release.

@stephentoub
Copy link
Member Author

@jkoritzinsky, this can be closed now, right?

@jkoritzinsky
Copy link
Member

We still need to fix this in 7.0.x. I'll add that to my to-do list for tomorrow.

jkoritzinsky added a commit to jkoritzinsky/runtime that referenced this issue Jan 26, 2023
Unpin the C# compiler and build our shipping Roslyn components against the released Roslyn components for 7.0.100 instead of against pre-release builds.

Also rename the MicrosoftCodeAnalysisVersion_4_X property now that it's pinned to Roslyn 4.4 forever.

Fixes dotnet#65317
carlossanlop added a commit that referenced this issue Feb 11, 2023
* Unpin the C# compiler version

Unpin the C# compiler and build our shipping Roslyn components against the released Roslyn components for 7.0.100 instead of against pre-release builds.

Also rename the MicrosoftCodeAnalysisVersion_4_X property now that it's pinned to Roslyn 4.4 forever.

Fixes #65317

* Enable packaging servicing for updated OOB packages

* Fixed the servicing versions

* Remove packaging changes

---------

Co-authored-by: Carlos Sanchez <[email protected]>
@jkoritzinsky
Copy link
Member

Fixed by #81247

@ghost ghost locked as resolved and limited conversation to collaborators May 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants