Skip to content

Add missing assemblies to ref pack #19161

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
wtgodbe merged 12 commits into from
Feb 20, 2020
Merged

Add missing assemblies to ref pack #19161

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b61c478
Add missing assemblies to ref pack
Feb 19, 2020
d6654d1
amend
Feb 19, 2020
0e4635c
Update comments too
JunTaoLuo Feb 19, 2020
903ddbd
Use patchVersion=0 for ref pack dependencies
wtgodbe Feb 19, 2020
cac1ed2
Fixup versions
wtgodbe Feb 19, 2020
abe41c2
Enable targeting pack tests
Feb 20, 2020
44dab04
Fix targeting pack test
Feb 20, 2020
4c15267
Feedback
Feb 20, 2020
370f79e
Merge remote-tracking branch 'origin/release/3.1' into johluo/always-…
wtgodbe Feb 20, 2020
168dff4
Update Microsoft.AspNetCore.App.Ref.csproj
wtgodbe Feb 20, 2020
5df69da
Downgrade System.* versions for all ref/ projects
dougbu Feb 20, 2020
0572a25
Update condition
wtgodbe Feb 20, 2020
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.1.2. -->
<!-- We can remove the 3.1.2 line from any branch other than release/3.1 and from here after 3.1.2 is released. -->
<!-- Produce targeting pack installers/packages once per major.minor except in extraordinary cases i.e. 3.1.3. -->
<!-- We can remove the 3.1.3 line from any branch other than release/3.1 and from here after 3.1.3 is released. -->
<IsTargetingPackBuilding Condition=" '$(DotNetBuildFromSource)' == 'true' ">false</IsTargetingPackBuilding>
<IsTargetingPackBuilding
Condition=" '$(IsTargetingPackBuilding)' == '' AND '$(VersionPrefix)' == '3.1.2' ">true</IsTargetingPackBuilding>
Condition=" '$(IsTargetingPackBuilding)' == '' AND '$(VersionPrefix)' == '3.1.3' ">true</IsTargetingPackBuilding>
<IsTargetingPackBuilding
Condition=" '$(IsTargetingPackBuilding)' == '' AND '$(AspNetCorePatchVersion)' != '0' ">false</IsTargetingPackBuilding>
<IsTargetingPackBuilding Condition=" '$(IsTargetingPackBuilding)' == '' ">true</IsTargetingPackBuilding>
Expand Down
13 changes: 13 additions & 0 deletions eng/SharedFramework.External.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@
-->
<Project>

<!-- For the targeting pack, always use packages with PatchVersion=0 -->
<PropertyGroup Condition="'$(MSBuildProjectName)' == 'Microsoft.AspNetCore.App.Ref'">
<SystemIOPipelinesPackageVersion>$(SystemIOPipelinesPackageVersion.Split('.')[0]).$(SystemIOPipelinesPackageVersion.Split('.')[1]).0</SystemIOPipelinesPackageVersion>
<SystemSecurityCryptographyXmlPackageVersion>$(SystemSecurityCryptographyXmlPackageVersion.Split('.')[0]).$(SystemSecurityCryptographyXmlPackageVersion.Split('.')[1]).0</SystemSecurityCryptographyXmlPackageVersion>
<MicrosoftWin32SystemEventsPackageVersion>$(MicrosoftWin32SystemEventsPackageVersion.Split('.')[0]).$(MicrosoftWin32SystemEventsPackageVersion.Split('.')[1]).0</MicrosoftWin32SystemEventsPackageVersion>
<SystemDiagnosticsEventLogPackageVersion>$(SystemDiagnosticsEventLogPackageVersion.Split('.')[0]).$(SystemDiagnosticsEventLogPackageVersion.Split('.')[1]).0</SystemDiagnosticsEventLogPackageVersion>
<SystemDrawingCommonPackageVersion>$(SystemDrawingCommonPackageVersion.Split('.')[0]).$(SystemDrawingCommonPackageVersion.Split('.')[1]).0</SystemDrawingCommonPackageVersion>
<SystemSecurityCryptographyPkcsPackageVersion>$(SystemSecurityCryptographyPkcsPackageVersion.Split('.')[0]).$(SystemSecurityCryptographyPkcsPackageVersion.Split('.')[1]).0</SystemSecurityCryptographyPkcsPackageVersion>
<SystemSecurityPermissionsPackageVersion>$(SystemSecurityPermissionsPackageVersion.Split('.')[0]).$(SystemSecurityPermissionsPackageVersion.Split('.')[1]).0</SystemSecurityPermissionsPackageVersion>
<SystemWindowsExtensionsPackageVersion>$(SystemWindowsExtensionsPackageVersion.Split('.')[0]).$(SystemWindowsExtensionsPackageVersion.Split('.')[1]).0</SystemWindowsExtensionsPackageVersion>
</PropertyGroup>


<ItemGroup>
<!-- Dependencies from aspnet/Extensions -->
<ExternalAspNetCoreAppReference Include="Microsoft.Extensions.Caching.Abstractions" Version="$(MicrosoftExtensionsCachingAbstractionsPackageVersion)" />
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 @@ -33,7 +33,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="'$(IsTargetingPackBuilding)' != 'true'">$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).2</TargetingPackVersionPrefix>
<TargetingPackVersionPrefix Condition="'$(IsTargetingPackBuilding)' != 'true'">$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).3</TargetingPackVersionPrefix>
<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
2 changes: 1 addition & 1 deletion 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 @@ -130,7 +130,7 @@ This package is an internal implementation of the .NET Core SDK and is not meant
<!-- Exclude transitive external dependencies that are not directly referenced by projects in AspNetCore or Extensions -->
<AspNetCoreReferenceAssemblyPath
Include="@(ReferencePathWithRefAssemblies)"
Condition="'%(ReferencePathWithRefAssemblies.IsReferenceAssembly)' == 'true'"
Condition="'%(ReferencePathWithRefAssemblies.IsReferenceAssembly)' != 'false'"
Copy link
Contributor

@dougbu dougbu Feb 19, 2020

Choose a reason for hiding this comment

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

Have trouble believing the previous Exclude item was wrong. Instead the ref/ project shouldn't provide metadata in @(ReferencePathWithRefAssemblies).

Copy link
Contributor Author

@JunTaoLuo JunTaoLuo Feb 19, 2020

Choose a reason for hiding this comment

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

Ahh, I think I now understand what you mean. We should probably investigate what's happening here.

Copy link
Contributor

@dougbu dougbu Feb 19, 2020

Choose a reason for hiding this comment

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

I believe we need to exclude %(IsReferenceAssemblyProject) items from @(AspNetCoreAppReference) in the main @(Reference) item group at

aspnetcore/src/Framework/ref/Microsoft.AspNetCore.App.Ref.csproj

Line 62 in 0e4635c

<Reference Include="@(AspNetCoreAppReference);@(AspNetCoreAppReferenceAndPackage);@(ExternalAspNetCoreAppReference)" />
then add another with <ReferenceOutputAssembly>false</ReferenceOutputAssembly> for the @(AspNetCoreAppReference) items that are from ref/ projects. Might be possible to shorten the new code to

<ItemGroup>
  <Reference Include="@(AspNetCoreAppReference);@(AspNetCoreAppReferenceAndPackage);@(ExternalAspNetCoreAppReference)">
    <ReferenceOutputAssembly Condition=" '%(IsReferenceAssemblyProject)' == 'true' ">false</ReferenceOutputAssembly>
  </Reference>

Copy link
Contributor

@dougbu dougbu Feb 19, 2020

Choose a reason for hiding this comment

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

Might be fine to remove references to the ref/ projects entirely i.e. exclude @(AspNetCoreAppReference) and @(AspNetCoreAppReferenceAndPackage) items with '%(IsReferenceAssemblyProject)' == 'true' metadata. Of course, need to confirm that metadata is present. (Root cause here is those two item groups map names to projects and the names are ambiguous.)

Exclude="
@(_ReferencedExtensionsRefAssemblies);
@(ReferencePathWithRefAssemblies->WithMetadataValue('NuGetPackageId', 'Microsoft.NETCore.App.Ref'));
Expand Down
5 changes: 5 additions & 0 deletions src/Framework/test/Microsoft.AspNetCore.App.UnitTests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<RootNamespace>Microsoft.AspNetCore</RootNamespace>
<!-- https://github.com/aspnet/AspNetCore/issues/7939: This unit test requires the shared framework be available in Helix. -->
<BuildHelixPayload>false</BuildHelixPayload>
<VerifyAncmBinary Condition="'$(TargetOsName)' == 'win' AND '$(TargetArchitecture)' != 'arm'">true</VerifyAncmBinary>
</PropertyGroup>

<ItemGroup>
Expand Down Expand Up @@ -35,6 +36,10 @@
<_Parameter1>TargetingPackLayoutRoot</_Parameter1>
<_Parameter2>$(TargetingPackLayoutRoot)</_Parameter2>
</AssemblyAttribute>
<AssemblyAttribute Include="Microsoft.AspNetCore.TestData">
<_Parameter1>VerifyAncmBinary</_Parameter1>
<_Parameter2>$(VerifyAncmBinary)</_Parameter2>
</AssemblyAttribute>
</ItemGroup>

<ItemGroup>
Expand Down
10 changes: 8 additions & 2 deletions src/Framework/test/TargetingPackTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public TargetingPackTests(ITestOutputHelper output)
_targetingPackRoot = Path.Combine(TestData.GetTestDataValue("TargetingPackLayoutRoot"), "packs", "Microsoft.AspNetCore.App.Ref", TestData.GetTestDataValue("TargetingPackVersion"));
}

[Fact(Skip="https://github.com/aspnet/AspNetCore/issues/14832")]
[Fact]
public void AssembliesAreReferenceAssemblies()
{
IEnumerable<string> dlls = Directory.GetFiles(_targetingPackRoot, "*.dll", SearchOption.AllDirectories);
Expand All @@ -55,7 +55,7 @@ public void AssembliesAreReferenceAssemblies()
});
}

[Fact(Skip="https://github.com/aspnet/AspNetCore/issues/14832")]
[Fact]
public void PlatformManifestListsAllFiles()
{
var platformManifestPath = Path.Combine(_targetingPackRoot, "data", "PlatformManifest.txt");
Expand Down Expand Up @@ -90,6 +90,12 @@ public void PlatformManifestListsAllFiles()
})
.ToHashSet();

if (!TestData.VerifyAncmBinary())
{
actualAssemblies.Remove("aspnetcorev2_inprocess");
expectedAssemblies.Remove("aspnetcorev2_inprocess");
}

var missing = expectedAssemblies.Except(actualAssemblies);
var unexpected = actualAssemblies.Except(expectedAssemblies);

Expand Down
3 changes: 3 additions & 0 deletions src/Framework/test/TestData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq;
using System.Reflection;

Expand All @@ -20,6 +21,8 @@ public class TestData

public static string GetTargetingPackDependencies() => GetTestDataValue("TargetingPackDependencies");

public static bool VerifyAncmBinary() => string.Equals(GetTestDataValue("VerifyAncmBinary"), "true", StringComparison.OrdinalIgnoreCase);

public static string GetTestDataValue(string key)
=> typeof(TestData).Assembly.GetCustomAttributes<TestDataAttribute>().Single(d => d.Key == key).Value;
}
Expand Down