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

[release/6.0] source-build: support building aspnetcore using non-portable runtime packages (backport #43937) #44752

Merged

Conversation

ayakael
Copy link

@ayakael ayakael commented Oct 26, 2022

source-build: support building aspnetcore using non-portable runtime packages (backport #43937)

Description

Backport of #43937

Per @tmds

Currently source-build performs 'runtime-portable' build that produces 'linux-{arch}' packages that are used when building ASP.NET Core.

With this change, we can use the non-portable packages that are produced by the source-build 'runtime' build, and eliminate the 'runtime-portable' build.

Customer impact

Reduces build time with source-build significantly due to not needing the building of portable runtime.

Regression?

  • Yes
  • No

[If yes, specify the version the behavior has regressed from]

Risk

  • High
  • Medium
  • Low

Only activates when PortableBuild=false. As it defaults to true, this change should not affect any other customers than distro maintainers.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@ayakael ayakael requested review from a team as code owners October 26, 2022 19:51
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 26, 2022
@ghost
Copy link

ghost commented Oct 26, 2022

Thanks for your PR, @ayakael. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@ghost ghost added this to the 6.0.x milestone Oct 26, 2022
@ghost ghost added this to In Progress in Servicing Oct 26, 2022
@ghost
Copy link

ghost commented Oct 26, 2022

Hi @ayakael. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@dougbu
Copy link
Member

dougbu commented Oct 26, 2022

Backport of #43937

Please describe any changes between that PR and this. Everything feels familiar but I may be missing something.

Separately, have you tested this branch with /p:TargetRuntimeIdentifier={something} /p:PortableBuild=false

@mkArtakMSFT mkArtakMSFT added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Oct 27, 2022
@ghost
Copy link

ghost commented Oct 27, 2022

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

@ayakael
Copy link
Author

ayakael commented Oct 29, 2022

Backport of #43937

Please describe any changes between that PR and this. Everything feels familiar but I may be missing something.

Separately, have you tested this branch with /p:TargetRuntimeIdentifier={something} /p:PortableBuild=falsegrey_question

I essentially applied the patch as it was in the backport, manually cleaning up whatever didn't apply cleanly due to code not existing yet. In this case, the following was cut due to that code not existing yet:

diff --git a/src/aspnetcore/eng/tools/GenerateFiles/Directory.Build.targets.in b/src/aspnetcore/eng/tools/GenerateFiles/Directory.Build.targets.in
index 0100971de43d..e97a6485d5b9 100644
--- a/src/aspnetcore/eng/tools/GenerateFiles/Directory.Build.targets.in
+++ b/src/aspnetcore/eng/tools/GenerateFiles/Directory.Build.targets.in
@@ -56,22 +56,22 @@ 
     <KnownAppHostPack Update="Microsoft.NETCore.App">
       <AppHostPackVersion
         Condition=" '%(TargetFramework)' == '${DefaultNetCoreTargetFramework}' ">${MicrosoftNETCoreAppRuntimeVersion}</AppHostPackVersion>
-      <AppHostRuntimeIdentifiers Condition=" '$(PortableBuild)' != 'true' ">$(TargetRuntimeIdentifier)</AppHostRuntimeIdentifiers>
+      <AppHostRuntimeIdentifiers Condition=" '$(PortableBuild)' == 'false' ">$(TargetRuntimeIdentifier)</AppHostRuntimeIdentifiers>
     </KnownAppHostPack>
 
     <KnownRuntimePack Update="Microsoft.NETCore.App">
       <LatestRuntimeFrameworkVersion
         Condition=" '%(TargetFramework)' == '${DefaultNetCoreTargetFramework}' ">${MicrosoftNETCoreAppRuntimeVersion}</LatestRuntimeFrameworkVersion>
-      <AppHostRuntimeIdentifiers Condition=" '$(PortableBuild)' != 'true' ">$(TargetRuntimeIdentifier)</AppHostRuntimeIdentifiers>
+      <AppHostRuntimeIdentifiers Condition=" '$(PortableBuild)' == 'false' ">$(TargetRuntimeIdentifier)</AppHostRuntimeIdentifiers>
     </KnownRuntimePack>

@ayakael
Copy link
Author

ayakael commented Oct 29, 2022

/p:TargetRuntimeIdentifier={something}

As far as testing is concerned, all checks passes here with these parameters set: dotnet/installer#14816.

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.

Sounds good. We'll merge when the branches open next.

@dougbu dougbu added the tell-mode Indicates a PR which is being merged during tell-mode label Nov 2, 2022
@dougbu dougbu modified the milestones: 6.0.x, 6.0.12 Nov 2, 2022
@dougbu dougbu merged commit 1e3dacc into dotnet:release/6.0 Nov 2, 2022
Servicing automation moved this from In Progress to Done Nov 2, 2022
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 community-contribution Indicates that the PR has been added by a community member tell-mode Indicates a PR which is being merged during tell-mode
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants