Move to .NET 10 P5 SDK and Arcade 10 SDK

[JoeRobich]

Restores the previous move to .NET 10 P5 changes and brings in Arcade 10 SDK.

Targets a specific build of Arcade 10 SDK because later builds introduce bootstrap failures.

[RikkiGibson]

It looks like there are some package version issues in the correctness leg

[RikkiGibson]

What's going on with the Build_Unix_Debug leg?

[RikkiGibson]

Correctness_Bootstrap_Build_Default is also deeply broken, there are almost 150k lines of "System.Object not defined" type of errors, despite the restore seemingly not reporting any complaints.

[333fred]

If we don't test these steps they will end up bitrotting. I'd prefer to keep them.

[JoeRobich]

Ran into a issue where a package required for publish wasn't being downloaded because we do not restore with the --publish option. Most straight forward fix is to simply not publish but we can also just add the publish option to the restore.

[JoeRobich]

Guess I'll push back a little and say that nothing ensures the publish and pack options do anything here. They could silently do nothing here and there would be no failure. If we want to avoid regressions, I think we would want to add correctness tests to verify these options do what they are told.

[333fred]

Sure, but I would prefer we have a min-bar of making sure the commands can run without failure.

[jaredpar]

Is this change locking us to preview 5 or is it moving us to Arcade 10 which will update the SDK when it chooses? Right now that is latest (pretty sure). That has a cadence of roughly every two weeks. If that is what we intend to sign up for that's okay (will need some messaging though). Just want to make sure we're on the same page with what this is doing.

[JoeRobich]

Since we cannot opt out of SDK updates from Arcade, what we have done for some servicing branches is to downgrade the SDK in the Arcade update PR. That would be my suggestion here. Add back the policy to automatically block Arcade PRs that modify the SDK, which we used to have with FabricBot.

roslyn/.github/fabricbot.json

Lines 199 to 244 in 26a3ec1

	{
	"taskType": "trigger",
	"capabilityId": "IssueResponder",
	"subCapability": "PullRequestResponder",
	"version": "1.0",
	"config": {
	"conditions": {
	"operator": "and",
	"operands": [
	{
	"name": "isActivitySender",
	"parameters": {
	"user": "dotnet-maestro[bot]"
	}
	},
	{
	"name": "isAction",
	"parameters": {
	"action": "opened"
	}
	},
	{
	"name": "bodyContains",
	"parameters": {
	"bodyPattern": "Updates sdk.version"
	}
	}
	]
	},
	"eventType": "pull_request",
	"eventNames": [
	"pull_request",
	"issues",
	"project_card"
	],
	"actions": [
	{
	"name": "requestChangesPullRequest",
	"parameters": {
	"comment": "This PR changes the .NET SDK version. Review required from @dotnet/roslyn-infrastructure before merging.\nTasks:\n- [ ] Getting Started Documentation has been updated\n- [ ] NuGet dependency version updated to match version shipping in SDK"
	}
	}
	],
	"taskName": "Require PR approval before merging SDK change"
	}
	},

[jaredpar]

Why did we remove an accessor here?

[CyrusNajmabadi]

because the rule says "no redundant accessibility modifiers in interfaces" and "public" is redundant.

[CyrusNajmabadi]

ide code changes LGTM. i have no idea about anything related to the "eng" directory :)

[RikkiGibson]

holding until InfraSwat is ready to take this.

[RikkiGibson]

We have some RPS counters in a recent insertion which require more data to decide if there is a real problem or not. I would like to wait to see if our upcoming build inserts cleanly, before taking this. Thanks! https://dev.azure.com/dnceng/internal/_build/results?buildId=2733703&view=results

[JoeRobich]

/azp run roslyn-integration-CI

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[CyrusNajmabadi]

@JoeRobich any luck with this?

[jaredpar]

@JoeRobich are you able to reprooduce this failure locally? There is not really any info to go off of in the test run that I could see.

[JoeRobich]

@JoeRobich are you able to reprooduce this failure locally? There is not really any info to go off of in the test run that I could see.

I have not been able to run integration tests locally. Trying to workout what is going on with my DevBox.

I have looked at the Screenshots artifacts from the failing run and see that the locale seems to be misconfigured during the CSharpInteractive WorkspaceClearedAfterReset test. It is expecting "Stack overflow." Since the test is checking error strings in this way it should be using the UseCultureAttribute.

In BasicRename we have a test failing due to the cursor not moving to the end of the identifier after a rename. In NewDocumentFormatting we have a test failing because the Error List is not clearing when a new project is opened.

I had moved the CI image to use the scouting queue meaning 17.14.7. I've reverted that change to see if these issues exist when running on 17.14.5.

[JoeRobich]

OK, so more failures on 17.14.5.

Looking at BasicRename and it seems like the cursor location is a known problem that we tried to work around. TrueException is a XunitException so not sure why this isn't working for us.

roslyn/src/VisualStudio/IntegrationTest/New.IntegrationTests/VisualBasic/BasicRename.cs

Lines 384 to 403 in 334d4c7

	try
	{
	// This is the expected behavior
	await TestServices.EditorVerifier.TextEqualsAsync(@"
	Imports System
	Public Class CustomAttribute$$
	Inherits Attribute
	End Class", HangMitigatingCancellationToken);
	}
	catch (XunitException)
	{
	// But sometimes we get this instead
	await TestServices.EditorVerifier.TextEqualsAsync(@"
	Imports System
	Public Class CustomA$$ttribute
	Inherits Attribute
	End Class", HangMitigatingCancellationToken);
	}

[JoeRobich]

Still seeing the Interactive test fail as in #78925 (comment) even with the UseCulture("en-US") attribute. It makes me think the interactive is running OOP and may not share the locale of the process that launched it. @tmat

[tmat]

@JoeRobich Any idea why would the behavior be different now?

[JoeRobich]

@JoeRobich Any idea why would the behavior be different now?

Perhaps these VM images have the system culture set differently. I know for both the LSP and BuildHost we pass along locale so that logging matches the process that launched them.

[tmat]

We do that as well here:

roslyn/src/Interactive/Host/Interactive/Core/InteractiveHost.LazyRemoteService.cs

Line 64 in aafd6eb

var remoteService = await TryStartProcessAsync(Options.HostPath, Options.Culture, Options.UICulture, cancellationToken).ConfigureAwait(false);

[JoeRobich]

We do that as well here

@tmat I was sure hoping we did. =) Unfortunately that makes what we are seeing even more bizarre. I don't know what explains this

no longer blocking this.

[RikkiGibson]

@dibarbet for visibility