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

Don't separately build arm64 installers #31158

Merged
merged 4 commits into from
Mar 24, 2021

Conversation

JunTaoLuo
Copy link
Contributor

@JunTaoLuo JunTaoLuo commented Mar 23, 2021

This should fix BAR push.

I think we should backport this to 3.1 as well. Although the error will only surface when we need to rebuild targeting pack in servicing which is very rare.

I'm going to launch an internal build to verify the artifacts.

@JunTaoLuo JunTaoLuo requested review from BrennanConroy and a team March 23, 2021 19:07
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Mar 23, 2021
Copy link
Member

@wtgodbe wtgodbe left a comment

Choose a reason for hiding this comment

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

Do the arm64 installers get built somewhere else?

@BrennanConroy
Copy link
Member

This will need to go into the 6.0-preview3 branch

@JunTaoLuo
Copy link
Contributor Author

Yes all msi installers are built in the x64/x86 step. In 3.1 we needed to build it as part of arm64 build since no other installers wouldbe building. I forgot to remove that as it merged cleanly 3.1->main.

@JunTaoLuo JunTaoLuo changed the base branch from main to release/6.0-preview3 March 23, 2021 20:34
Copy link
Member

@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.

This won't fix the actual problem because we're currently building the targeting packs in every build job. It also sets us up for major problems in preview 3 and later because the ARM64 installers won't be included. Core issue is at https://github.com/dotnet/aspnetcore/blob/main/Directory.Build.targets#L82-L83 General problem after reverting recent problems there will be building the ARM64 installer inputs without shipping them i.e. excluding them from the asset manifest.

@JunTaoLuo
Copy link
Contributor Author

Ah I see, the change removed the duplications of aspnetcore-targeting-pack-6.0.0-preview.3.21172.9.tar.gz but the nupkgs are still being built as part of building on each individual platform.

Copy link
Member

@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.

Looks better😃

You had a reason to mark Microsoft.AspNetCore.App.Ref and …Internal projects as packable in release/3.1, likely something about providing all inputs the installers needed. Was that reason invalid❔

@JunTaoLuo
Copy link
Contributor Author

I had to specifically mark Ref and Ref.Internal as packable in release/3.1 since we needed those zips built for arm64 but not any other platform. Usually all targeting pack is built together (zips and installers) in the x86/x64 leg. Hence the oddity in 3.1. However, in the 6.0 releases, there's no need for this special case.

@JunTaoLuo
Copy link
Contributor Author

The internal build artifacts look correct and the duplication has been resolved.

@JunTaoLuo JunTaoLuo merged commit 4ce1f2c into release/6.0-preview3 Mar 24, 2021
@JunTaoLuo JunTaoLuo deleted the johluo/fix-arm64-assets branch March 24, 2021 00:24
@BrennanConroy
Copy link
Member

@JunTaoLuo I believe you need to manually forward port this to main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants