Skip to content

Flow internal transport packages during servicing #75969

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.

Sign up for GitHub

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

Merged
ViktorHofer merged 1 commit into from
Sep 21, 2022
Merged

Flow internal transport packages during servicing #75969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions eng/packaging.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@
'$(IsRIDSpecificProject)' == 'true') and
'$(PreReleaseVersionLabel)' != 'servicing' and
'$(GitHubRepositoryName)' != 'runtimelab'">true</GeneratePackageOnBuild>
<GeneratePackageOnBuild Condition="'$(GeneratePackageOnBuild)' != 'true'">false</GeneratePackageOnBuild>
<!-- When in source-build we need to generate all packages when building for all configurations even in servicing. -->
<GeneratePackageOnBuild Condition="!$(GeneratePackageOnBuild) and
<GeneratePackageOnBuild Condition="'$(GeneratePackageOnBuild)' != 'true' and
Copy link
Contributor

@carlossanlop carlossanlop Sep 21, 2022

Choose a reason for hiding this comment

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

Comparing to the 6.0 commit, I see the conditions are very different now.

  • Added: GeneratePackageOnBuild != true
  • Added: IsRIDSpecificProject != true
  • Removed: PreReleaseVersionLabel == servicing
  • Removed: GitHubRepositoryName != runtimelab
  • Changed: BuildAllConfigurations != true to == true
  • Added: DotNetBuildFromSource == true

For my own education, would you mind explaining all these differences?

Copy link
Member Author

@ViktorHofer ViktorHofer Sep 21, 2022

Choose a reason for hiding this comment

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

Those conditions were "cleaned-up" in an unrelated change. This change just uses a different style for the condition: instead of !$(GeneratePackageOnBuild) it now uses '$(GeneratePackageOnBuild)' != 'true' which is the syntax that I prefer. Mainly because it also works when the property isn't set.

'$(BuildAllConfigurations)' == 'true' and
'$(DotNetBuildFromSource)' == 'true'">true</GeneratePackageOnBuild>
<!-- Search for the documentation file in the intellisense package and otherwise pick up the generated one. -->
Expand Down
6 changes: 6 additions & 0 deletions ...nal.Runtime.AspNetCore.Transport/src/Microsoft.Internal.Runtime.AspNetCore.Transport.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
<NoWarn>$(NoWarn);NU5131</NoWarn>
</PropertyGroup>

<!-- Always generate this package during servicing to flow the dependency to AspNetCore. -->
<PropertyGroup Condition="'$(PreReleaseVersionLabel)' == 'servicing'">
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<ServicingVersion>$(PatchVersion)</ServicingVersion>
</PropertyGroup>

<ItemGroup>
<!-- Requires Private=true to calculate ReferenceCopyLocalPaths items. -->
<ProjectReference Include="@(AspNetCoreAppLibrary->'$(LibrariesProjectRoot)%(Identity)\src\%(Identity).csproj')"
Expand Down
6 changes: 6 additions & 0 deletions ...ime.WindowsDesktop.Transport/src/Microsoft.Internal.Runtime.WindowsDesktop.Transport.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
<NoWarn>$(NoWarn);NU5131</NoWarn>
</PropertyGroup>

<!-- Always generate this package during servicing to flow the dependency to WindowsDesktop. -->
<PropertyGroup Condition="'$(PreReleaseVersionLabel)' == 'servicing'">
Copy link
Contributor

@carlossanlop carlossanlop Sep 21, 2022

Choose a reason for hiding this comment

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

Why does this PropertyGroup and the one from the file above contain the condition for 'servicing', but not the 6.0 change? Back then, this condition was part of the packaging.targets file.

Copy link
Member Author

@ViktorHofer ViktorHofer Sep 21, 2022

Choose a reason for hiding this comment

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

Because the 6.0 change was directly committed to release/6.0 when release/6.0 meant 6.0.1. That branch will never again mean anything else than servicing, that's why the condition wasn't needed. This change is more universal and works in both servicing, RTM and main.

<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<ServicingVersion>$(PatchVersion)</ServicingVersion>
</PropertyGroup>

<ItemGroup>
<!-- Requires Private=true to calculate ReferenceCopyLocalPaths items.
ReferringTargetFramework is set to $(NetCoreAppCurrent)-windows so that we pack the Windows specific implementation assemblies -->
Expand Down