-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Simplify ref/ assembly generation #24136
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,6 +116,8 @@ | |
<SystemReflectionMetadataPackageVersion>5.0.0-preview.8.20361.2</SystemReflectionMetadataPackageVersion> | ||
<SystemResourcesExtensionsPackageVersion>5.0.0-preview.8.20361.2</SystemResourcesExtensionsPackageVersion> | ||
<SystemRuntimeCompilerServicesUnsafePackageVersion>5.0.0-preview.8.20361.2</SystemRuntimeCompilerServicesUnsafePackageVersion> | ||
<!-- System.Security.AccessControl should only be referenced in Dependencies.props and RTMVersions.csproj. --> | ||
<SystemSecurityAccessControlPackageVersion>5.0.0-preview.8.20361.2</SystemSecurityAccessControlPackageVersion> | ||
<SystemSecurityCryptographyCngPackageVersion>5.0.0-preview.8.20361.2</SystemSecurityCryptographyCngPackageVersion> | ||
<SystemSecurityCryptographyPkcsPackageVersion>5.0.0-preview.8.20361.2</SystemSecurityCryptographyPkcsPackageVersion> | ||
<SystemSecurityCryptographyXmlPackageVersion>5.0.0-preview.8.20361.2</SystemSecurityCryptographyXmlPackageVersion> | ||
|
@@ -163,6 +165,14 @@ | |
Upshot is we need Major.Minor.0 runtime packages for compilation and the targeting pack and Major.Minor.Latest | ||
runtime packages for everything else. This is not an issue for assemblies available in Microsoft.NETCore.App.Ref or | ||
Microsoft.Extensions.Internal.Transport because it is next to impossible we would service those packages. | ||
|
||
System.Security.AccessControl should only be referenced in Dependencies.props and RTMVersions.csproj. Because | ||
it's a transitive reference, we reship the ref/ assembly in Microsoft.AspNetCore.App.Ref. dotnet/runtime ships | ||
the implementation assemblies in Microsoft.NETCore.App.Runtime.* packages. | ||
|
||
If testing this configuration prior to servicing, update the versions of dependencies too. E.g. change | ||
`$(SystemSecurityPrincipalWindowsV0PackageVersion)` if you change `$(SystemSecurityAccessControlV0PackageVersion)` | ||
because System.Security.AccessControl will otherwise be loadable. This should not be necessary in servicing. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ericstj when I updated only Will we hit something similar if dotnet/runtime ships System.Security.AccessControl 5.0.1 but leaves System.Security.Principal.Windows at 5.0.0❔ Put another way, will dotnet/runtime build but not ship a newer System.Security.Principal.Windows.dll and use that when compiling System.Security.AccessControl.dll❔ If yes, we'll need to be really careful here (perhaps adding further workarounds) and I shouldn't include the "not be necessary" words. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There’s nothing fancy here. It just chooses the newer version. You’ll only see things go app local if you use a newer package than what’s in the shared framework you target. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't generally use our own part of the shared Fx when building and the example assemblies are in Microsoft.AspNetcore.App.Ref, not Microsoft.NETCore.App.Ref. So, does "nothing fancy" help us with the corner case I describe❔ I'd like to know we can rev one without the other reliably before we get to 5.0.1. (And, these packages / assemblies have a few other inter-dependencies.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless someone objects, I'm going to assume the worst case here would be silently compiling against 5.0.x (newer) versions of these packages when servicing. Will file an issue about testing that |
||
--> | ||
<PropertyGroup Condition=" '$(IsServicingBuild)' == 'true' "> | ||
<MicrosoftWin32RegistryV0PackageVersion>$(MicrosoftWin32RegistryPackageVersion.Split('.')[0]).$(MicrosoftWin32RegistryPackageVersion.Split('.')[1]).0</MicrosoftWin32RegistryV0PackageVersion> | ||
|
@@ -252,9 +262,6 @@ | |
<MonoCecilPackageVersion>0.11.2</MonoCecilPackageVersion> | ||
<NewtonsoftJsonBsonPackageVersion>1.0.2</NewtonsoftJsonBsonPackageVersion> | ||
<NewtonsoftJsonPackageVersion>12.0.2</NewtonsoftJsonPackageVersion> | ||
<!-- Begin: STOP!!! Razor need to reference the version of JSON that our HOSTS support. --> | ||
<Razor_NewtonsoftJsonPackageVersion>9.0.1</Razor_NewtonsoftJsonPackageVersion> | ||
<!-- End: STOP!!! Razor need to reference the version of JSON that our HOSTS support. --> | ||
<NSwagApiDescriptionClientPackageVersion>13.0.4</NSwagApiDescriptionClientPackageVersion> | ||
<SeleniumSupportPackageVersion>3.12.1</SeleniumSupportPackageVersion> | ||
<SeleniumWebDriverMicrosoftDriverPackageVersion>17.17134.0</SeleniumWebDriverMicrosoftDriverPackageVersion> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,7 +127,7 @@ | |
</Target> | ||
|
||
<!-- | ||
This target resolves remaining Referene items to Packages, if possible. If not, they are left as Reference items fo the SDK to resolve. | ||
This target resolves remaining Reference items to Packages, if possible. If not, they are left as Reference items fo the SDK to resolve. | ||
This executes on NuGet restore and during DesignTimeBuild. It should not run in the outer, cross-targeting build. | ||
--> | ||
<Target Name="ResolveCustomReferences" | ||
|
@@ -205,21 +205,23 @@ | |
</Target> | ||
|
||
<!-- | ||
Muck with @(ResolvedCompileFileDefinitions) items between generation and use in order to compile against RTM lib/ | ||
Change @(ResolvedCompileFileDefinitions) items between generation and use in order to compile against RTM lib/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good change 😆 |
||
or ref/ assemblies. The approach works for all TFMs because it happens after a specific assembly is chosen for | ||
compilation; no need to restrict this to (say) the default TFM. | ||
|
||
Condition checks for RTMVersions.csproj.nuget.g.props file only to ensure restore operation is complete. | ||
|
||
This target could get confused if the layout changes for one of the dozen special-cased packages during servicing. | ||
E.g. ResolvePackageAssets picks a compatible assembly from the 5.0.1 package and _UseRTMReferenceAssemblies | ||
changes the path to one not present in the 5.0.0 package. Fortunately, this will break the build with complaints | ||
about the "invalid strong name". | ||
--> | ||
<Target Name="_UseRTMReferenceAssemblies" | ||
Condition=" '$(EnableCustomReferenceResolution)' == 'true' AND EXISTS('$(RepoRoot)eng\RTMVersions\obj\RTMVersions.csproj.nuget.g.props') " | ||
Condition=" '$(MSBuildProjectName)' != 'RepoTasks' " | ||
AfterTargets="ResolvePackageAssets" | ||
BeforeTargets="GenerateBuildDependencyFile;GeneratePublishDependencyFile;ILLink;ResolveLockFileReferences" | ||
DependsOnTargets="ResolvePackageAssets"> | ||
<Error Condition=" !EXISTS('$(RepoRoot)artifacts\obj\RepoTasks\RepoTasks.csproj.nuget.g.props') " | ||
Text="The eng/tools/RepoTasks project must be restored before building other projects." /> | ||
|
||
<JoinItems Left="@(ResolvedCompileFileDefinitions)" | ||
Right="@(LatestPackageReference->HasMetadata('RTMVersion'))" | ||
LeftKey="Filename" | ||
|
Uh oh!
There was an error while loading. Please reload this page.