Skip to content

Platform manifest in TargetingPack should refer to contents of runtime pack #17957

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

Closed
wants to merge 9 commits into from
Closed

Platform manifest in TargetingPack should refer to contents of runtime pack #17957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
57ba392
Platform manifest in TargetingPack should refer to contents of runtim…
wtgodbe Dec 19, 2019
3d04612
add back return to target
wtgodbe Dec 19, 2019
0d3497e
Build targetingPack in 3.0.2
wtgodbe Dec 19, 2019
a6226ac
Fix targeting pack version
wtgodbe Dec 19, 2019
386e473
Better condition for TargetingPackVersion
wtgodbe Dec 19, 2019
9a34dca
Remove custom versioning for targetingPack
wtgodbe Dec 19, 2019
da4bc6f
Add comment
wtgodbe Dec 19, 2019
a98edd4
Fix test
wtgodbe Dec 19, 2019
6b499f5
Add TargetingPackVersion back to versions.props
wtgodbe Dec 19, 2019
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
6 changes: 3 additions & 3 deletions Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@
<RuntimeInstallerBaseName>aspnetcore-runtime</RuntimeInstallerBaseName>
<TargetingPackInstallerBaseName>aspnetcore-targeting-pack</TargetingPackInstallerBaseName>

<!-- Produce targeting pack installers/packages once per major.minor except in extraordinary cases i.e. 3.0.1. -->
<!-- We can remove the 3.0.1 line from any branch other than release/3.0 and from here after 3.0.1 is released. -->
<!-- Produce targeting pack installers/packages once per major.minor except in extraordinary cases i.e. 3.0.2. -->
<!-- We can remove the 3.0.2 line from any branch other than release/3.0 and from here after 3.0.2 is released. -->
<IsTargetingPackBuilding Condition=" '$(DotNetBuildFromSource)' == 'true' ">false</IsTargetingPackBuilding>
<IsTargetingPackBuilding
Condition=" '$(IsTargetingPackBuilding)' == '' AND '$(VersionPrefix)' == '3.0.1' ">true</IsTargetingPackBuilding>
Condition=" '$(IsTargetingPackBuilding)' == '' AND '$(VersionPrefix)' == '3.0.2' ">true</IsTargetingPackBuilding>
Copy link
Member Author

Choose a reason for hiding this comment

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

Bump this to 3.0.3 if we ship this with that release

<IsTargetingPackBuilding
Condition=" '$(IsTargetingPackBuilding)' == '' AND '$(AspNetCorePatchVersion)' != '0' ">false</IsTargetingPackBuilding>
<IsTargetingPackBuilding Condition=" '$(IsTargetingPackBuilding)' == '' ">true</IsTargetingPackBuilding>
Expand Down
2 changes: 1 addition & 1 deletion eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<!-- TargetingPackVersionPrefix is used by projects, like .deb and .rpm, which use slightly different version formats. -->
<TargetingPackVersionPrefix>$(VersionPrefix)</TargetingPackVersionPrefix>
<!-- Targeting packs do not produce patch versions in servicing builds. No API changes are allowed in patches. -->
<TargetingPackVersionPrefix Condition="'$(IsServicingBuild)' == 'true'">$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).1</TargetingPackVersionPrefix>
Copy link
Member

Choose a reason for hiding this comment

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

Is this something that gets added back in on the next patch? If so, should we just comment this out or conditionalize it properly (e.g. by VersionPrefix)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I don't fully understand the ramifications of removing this, so I'll put it back

<TargetingPackVersionPrefix Condition="'$(IsTargetingPackBuilding)' != 'true'">$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).2</TargetingPackVersionPrefix>
Copy link
Member Author

Choose a reason for hiding this comment

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

Bump this to 3.0.3 if we ship this with that release

<ExperimentalVersionPrefix>0.3.$(AspNetCorePatchVersion)</ExperimentalVersionPrefix>
<!-- ANCM versioning is intentionally 10 + AspNetCoreMajorVersion because earlier versions of ANCM shipped as 8.x. -->
<AspNetCoreModuleVersionMajor>$([MSBuild]::Add(10, $(AspNetCoreMajorVersion)))</AspNetCoreModuleVersionMajor>
Expand Down
16 changes: 2 additions & 14 deletions src/Framework/ref/Microsoft.AspNetCore.App.Ref.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ This package is an internal implementation of the .NET Core SDK and is not meant
<!-- Platform manifest and package override metatdata -->
<ReferencePackSharedFxVersion>$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).0</ReferencePackSharedFxVersion>
<ReferencePackSharedFxVersion Condition="'$(VersionSuffix)' != ''">$(ReferencePackSharedFxVersion)-$(VersionSuffix)</ReferencePackSharedFxVersion>
<ReferencePlatformManifestOutputPath>$(ArtifactsObjDir)ref\PlatformManifest.txt</ReferencePlatformManifestOutputPath>

</PropertyGroup>

<ItemGroup>
Expand Down Expand Up @@ -141,23 +141,11 @@ This package is an internal implementation of the .NET Core SDK and is not meant

<AspNetCoreReferenceDocXml Include="@(_ResolvedProjectReferencePaths->WithMetadataValue('IsReferenceAssembly', 'false')->'%(RootDir)%(Directory)%(FileName).xml')" />
<AspNetCoreReferenceDocXml Include="@(_SelectedExtensionsRefAssemblies->'$(MicrosoftInternalExtensionsRefsPath)%(FileName).xml')" />
</ItemGroup>

<RepoTasks.GenerateSharedFrameworkDepsFile
DepsFilePath="$(ProjectDepsFilePath)"
TargetFramework="$(TargetFramework)"
FrameworkName="$(TargetingPackName)"
FrameworkVersion="$(ReferencePackSharedFxVersion)"
References="@(AspNetCoreReferenceAssemblyPath)"
RuntimeIdentifier="$(TargetRuntimeIdentifier)"
RuntimePackageName="$(PackageId)"
PlatformManifestOutputPath="$(ReferencePlatformManifestOutputPath)" />
Copy link
Member

Choose a reason for hiding this comment

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

Why does this get removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks to be called from Microsoft.AspNetCore.App.Runtime.csproj: https://github.com/aspnet/AspNetCore/blob/20fc1adf2ada01a4a1fc7934706d2b641acbb9f4/src/Framework/src/Microsoft.AspNetCore.App.Runtime.csproj#L230. As I don't think the result of GenerateSharedFrameworkDepsFile is being used for the Runtime project, should we move it all to the Ref project?

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks to be called from Microsoft.AspNetCore.App.Runtime.csproj

That's correct, we use the platform manifest generated by the Runtime pack since that's what the file should refer to. The runtime pack uses ReferenceCopyLocalPaths to calculate what should be included, so I guess it should stay in that file. I'll add a comment explaining why it's there and what it's for.


<ItemGroup>
<RefPackContent Include="@(AspNetCoreReferenceAssemblyPath)" PackagePath="$(RefAssemblyPackagePath)" />
<RefPackContent Include="@(AspNetCoreReferenceDocXml)" PackagePath="$(RefAssemblyPackagePath)" />
<RefPackContent Include="$(TargetDir)$(PackageConflictManifestFileName)" PackagePath="$(ManifestsPackagePath)" />
<RefPackContent Include="$(ReferencePlatformManifestOutputPath)" PackagePath="$(ManifestsPackagePath)" />
<RefPackContent Include="$(PlatformManifestOutputPath)" PackagePath="$(ManifestsPackagePath)" />
</ItemGroup>
</Target>

Expand Down
2 changes: 2 additions & 0 deletions src/Framework/src/Microsoft.AspNetCore.App.Runtime.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ This package is an internal implementation of the .NET Core SDK and is not meant
<_RuntimeReference Include="@(ReferenceCopyLocalPaths)" Condition="'%(ReferenceCopyLocalPaths.Extension)' != '.pdb' AND '%(ReferenceCopyLocalPaths.Extension)' != '.map'" />
</ItemGroup>

<!-- This is the platform manifest file that will be included in the Targeting Pack (Microsoft.AspNetCore.App.Ref).
It is a reference to what is included in the shared framework, so that apps know at compile time what will be available at runtime. -->
<RepoTasks.GenerateSharedFrameworkDepsFile
DepsFilePath="$(ProjectDepsFilePath)"
TargetFramework="$(TargetFrameworkMoniker)"
Expand Down
2 changes: 1 addition & 1 deletion src/Framework/test/TargetingPackTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void AssembliesAreReferenceAssemblies()
public void PlatformManifestListsAllFiles()
{
var platformManifestPath = Path.Combine(_targetingPackRoot, "data", "PlatformManifest.txt");
var expectedAssemblies = TestData.GetTargetingPackDependencies()
var expectedAssemblies = TestData.GetSharedFxDependencies()
.Split(';', StringSplitOptions.RemoveEmptyEntries)
.Select(i =>
{
Expand Down