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

Remove Windows arm32 support #85947

Merged
merged 2 commits into from
May 10, 2023

Conversation

BruceForstall
Copy link
Member

Mostly, remove the CI runs. Windows arm32 product support was removed before .NET 6.

Mostly, remove the CI runs. Windows arm32 product support was removed
before .NET 6.
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 8, 2023
@ghost ghost assigned BruceForstall May 8, 2023
@BruceForstall BruceForstall added area-Infrastructure and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 8, 2023
@ghost
Copy link

ghost commented May 8, 2023

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

Issue Details

Mostly, remove the CI runs. Windows arm32 product support was removed before .NET 6.

Author: BruceForstall
Assignees: BruceForstall
Labels:

area-Infrastructure

Milestone: -

@BruceForstall
Copy link
Member Author

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BruceForstall
Copy link
Member Author

cc @dotnet/jit-contrib

@BruceForstall
Copy link
Member Author

Comments, anyone?

There are probably many more things that could be removed after this is merged. The primary goal here was to stop running win-arm builds and tests in the CI system.

Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

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

LGTM. Do we want to remove support for building the product build for Windows ARM32, or just the runtime test build/run?

Comment on lines -205 to -208
- ${{ if and(eq(parameters.osGroup, 'windows'), eq(parameters.archType, 'arm')) }}:
- script: $(Build.SourcesDirectory)/src/coreclr/build-runtime$(scriptExt) $(buildConfig) $(archType) -hostarch x86 $(osArg) -ci $(compilerArg) -component crosscomponents -cmakeargs "-DCLR_CROSS_COMPONENTS_BUILD=1" $(officialBuildIdArg) $(clrRuntimePortableBuildArg)
displayName: Build CoreCLR Cross-Arch Tools (Tools that run on x86 targeting arm)

Copy link
Member

Choose a reason for hiding this comment

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

There's a condition in eng/Subsets.props that corresponds to this condition. Do we want to remove that as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. But that will be a follow-up.

@jkotas
Copy link
Member

jkotas commented May 8, 2023

There is more that can be cleaned up in the sources (e.g. delete src\coreclr\vm\arm*.asm). It can be done as a separate change.

@@ -200,7 +199,7 @@ echo.
echo where:
echo.
echo./? -? /h -h /help -help - View this message.
echo ^<build_architecture^> - Specifies build architecture: x64, x86, arm, or arm64 ^(default: x64^).
echo ^<build_architecture^> - Specifies build architecture: x64, x86, or arm64 ^(default: x64^).
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I'm still seeing linux_arm / linux_musl_arm jobs in the above pipelines, are we removing arm32 support even on these?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, linux-arm is still a supported platform. (But run.cmd is only used on Windows)

<Issue>https://github.com/dotnet/runtime/issues/81241</Issue>
</ExcludeList>
</ItemGroup>

<!-- Windows arm64 specific excludes -->
Copy link
Member

Choose a reason for hiding this comment

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

Not as part of this PR but I guess at some point we should audit this issue list and close those that are no longer relevant i.e. that only occur on the no longer supported Windows arm platform.

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

LGTM overall modulo one nitpick. I guess we'll likely want to make an equivalent change in the .NET 6 / 7 servicing branches, correct?

@BruceForstall
Copy link
Member Author

I guess we'll likely want to make an equivalent change in the .NET 6 / 7 servicing branches, correct?

Since we don't support win-arm in .NET 6/7, that would make sense.

Only change YML files to remove windows_arm runs.

Don't build Windows arm cross components.

Don't touch mono files.

Changing scripts to remove builds, and remove code, can follow.
@BruceForstall
Copy link
Member Author

I decided to simplify this to only remove win-arm CI runs. Removing the ability to build via build scripting can happen later.

@BruceForstall
Copy link
Member Author

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BruceForstall BruceForstall merged commit 7f4b072 into dotnet:main May 10, 2023
@BruceForstall BruceForstall deleted the DisableWinArm32Testing branch May 10, 2023 16:59
BruceForstall added a commit to BruceForstall/runtime that referenced this pull request May 11, 2023
BruceForstall added a commit that referenced this pull request May 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants