Skip to content

Conversation

@shirhatti
Copy link
Contributor

Addresses #bugnumber (in this specific format)

@shirhatti shirhatti changed the title [DonAdd ARM64 targets for ANCM [Dont merge] Add ARM64 targets for ANCM Mar 5, 2021
@shirhatti shirhatti requested a review from a team as a code owner March 5, 2021 06:13
@shirhatti
Copy link
Contributor Author

A couple of issues so far:

  • The arm64 ancm msi has the wrong bitness ancm included
  • Schema file isn't included in the installer
  • There's no way to test ancmv2 IISexpress installer as currently there's arm64 iisexpress build

@shirhatti shirhatti closed this Mar 5, 2021
@shirhatti shirhatti reopened this Mar 5, 2021
@shirhatti shirhatti marked this pull request as draft March 7, 2021 09:48
@shirhatti
Copy link
Contributor Author

@dotnet/aspnet-build Any suggestions on what's going wrong here? I'm specifically looking at the Windows ARM64 build failure where pretty much every project fails with the same issue

src\Analyzers\Analyzers\test\Microsoft.AspNetCore.Analyzers.Test.csproj(0,0): error NU1503: (NETCORE_ENGINEERING_TELEMETRY=Restore) Skipping restore for project 'D:\workspace\_work\1\s\src\Analyzers\Analyzers\test\Microsoft.AspNetCore.Analyzers.Test.csproj'. The project file may be invalid or missing targets required for restore.

I'm able to build the projects (including all the native projects that I've modified) through their respective build scripts .\build.cmd /p:Platform=arm64.

I've also tried modifying the sln file to add arm64 config section to each project to no avail.

Details

Regex to modify .sln file

Search: (^\t\t\{[0-9,A-F,-]{36}\}\.(Debug|Release)\|)(x86)(\.)(Build)(\.0 = (Debug|Release)\|Any CPU)$
Replace: $1$3$4$5$6\n$1ARM64$4$5$6\n$1ARM64$4ActiveCfg$6

Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

Real problem is AspNetCore.sln should never be used in CI builds.

D:\workspace\_work\1\s\src\Analyzers\Analyzers\test\Microsoft.AspNetCore.Analyzers.Test.csproj : error NU1503: Skipping restore for project 'D:\workspace\_work\1\s\src\Analyzers\Analyzers\test\Microsoft.AspNetCore.Analyzers.Test.csproj'. The project file may be invalid or missing targets required for restore. [D:\workspace\_work\1\s\AspNetCore.sln]

Root cause is you haven't updated eng/Build.props to include any projects in that build step.

</ItemGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
</Project>
</Project>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please reset your preferences so that newlines at the ends of files are left alone. Then fix this and other changed files up. Right now at least, most developers seem to add newlines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack. Will fix.

<NativeProjectReference Include="$(MSBuildThisFileDirectory)..\..\AspNetCoreModuleV2\OutOfProcessRequestHandler\OutOfProcessRequestHandler.vcxproj" Platform="Win32" />

<NativeProjectReference Include="$(MSBuildThisFileDirectory)..\..\AspNetCoreModuleV2\AspNetCore\AspNetCore.vcxproj" Platform="ARM64" />
<NativeProjectReference Include="$(MSBuildThisFileDirectory)..\..\AspNetCoreModuleV2\OutOfProcessRequestHandler\OutOfProcessRequestHandler.vcxproj" Platform="ARM64" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Something further down the stack only conditionally builds AspNetCoreModuleShim. Missing that assembly leads to at least some of the build failures.

<IntDir>$(IntermediateOutputPath)</IntDir>
</PropertyGroup>

<!-- These test projects are skipped because you cannot test native ARM64 projects on a x86/x64 machine. -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkotalik You may want to specifically review this and the changes to the Cpp.Common.Targets as well

@shirhatti
Copy link
Contributor Author

@dougbu What do I do about the error where the ARM64 build fails because the ARM64 leg of the build doesn't produce a targeting pack.

I looked the Build.props and saw this comment which left me quite confused

<!-- This really shouldn't be here, but instead of harvesting from the intermediate/output directories, the targetting pack installer logic
harvests from a zip of the reference assemblies. Producing it in each leg ends up with multiple targeting packs
getting produced and the BAR will reject the build. Centralize building the targeting pack in the x86/x64 leg. -->
<ProjectToBuild Include="$(RepoRoot)src\Installers\Windows\TargetingPack\TargetingPack.wixproj" AdditionalProperties="Platform=arm64" />

@shirhatti
Copy link
Contributor Author

shirhatti commented Mar 8, 2021

On second thought, I think I got it. The requisite ARM64 ANCM bits already get built in the x64 build stage.

I assume I shouldn't build native as part of the ARM64 build? Is that correct?

@shirhatti
Copy link
Contributor Author

I realize that this approach fails as well. Since the the x64 and and x86 builds happen in the same pipeline, we cheat by only building native assets once for all platforms as part of just the x64 build.

Since we're building ARM64 in a separate pipeline that won't work. Building the Microsoft.AspNetCore.Server.IIS package has native deps which relies on the native build having already run.

I can only think of some crazy ideas to fix this. I assume the build team wouldn't be happy with any of my suggestions 🤣

  • Move the entire ARM64 build into same pipeline as x64/x86
  • RID-split the Microsoft.AspNetCore.Server.IIS package and build each rid in it's own pipeline

@jkotalik
Copy link
Contributor

jkotalik commented Mar 8, 2021

RID-split the Microsoft.AspNetCore.Server.IIS package and build each rid in it's own pipeline

I think we'd have to go with this solution. We didn't have to rid split before because this only was on windows.

@dougbu
Copy link
Contributor

dougbu commented Mar 8, 2021

Building the Microsoft.AspNetCore.Server.IIS package has native deps which relies on the native build having already run.

Correct. Could you describe why disabling native builds is necessary❔

Move the entire ARM64 build into same pipeline as x64/x86

Adding more to that job is a non-starter because that's one of the longest legs as it is.

Another possibility is to build the Windows native assets once in a separate job that all other Windows jobs depend on. That would take some care to upload all the relevant files to an AzDO artifact, download them, and place the files where managed projects and the installer builds expect them. Might get messy but would lengthen the build only slightly (for the job startup delay plus the download / file placement work).

@shirhatti
Copy link
Contributor Author

@dougbu
Copy link
Contributor

dougbu commented Mar 9, 2021

@shirhatti ah, I thought you meant all native projects. The targeting pack and shared Fx bundles should not build for ARM64. Problem here is instead at https://github.com/dotnet/aspnetcore/pull/30680/files#diff-366b25472963c6b33188d84139f6c8997298c5adc857a2624ab0d26a2d87baf0R66 That change is incorrect because we already enable ARM64 *.wixproj builds in a separate block from the x64 / x86 one. Basically, we're now building too many *.wixproj projects for too many architectures.

You need to extend the *.wixproj build on ARM64 to include the IIS-related installers but that should be about it.

@Tratcher Tratcher removed their request for review May 21, 2021 21:46
@shirhatti shirhatti added the community-contribution Indicates that the PR has been added by a community member label Jun 25, 2021
@mkArtakMSFT mkArtakMSFT removed the community-contribution Indicates that the PR has been added by a community member label Jul 12, 2021
@pranavkm
Copy link
Contributor

@HaoK could we close this PR now that you have a copy?

@HaoK HaoK closed this Jan 24, 2022
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
@wtgodbe wtgodbe deleted the shirhatti/addarm64buildtargets branch November 15, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants