Use checked-in platform manifest in 3.1.2 #18250
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JunTaoLuo
reviewed
Jan 10, 2020
| <IsTargetingPackBuilding Condition=" '$(DotNetBuildFromSource)' == 'true' ">false</IsTargetingPackBuilding> | ||
| <IsTargetingPackBuilding | ||
| Condition=" '$(IsTargetingPackBuilding)' == '' AND '$(VersionPrefix)' == '3.0.1' ">true</IsTargetingPackBuilding> | ||
| Condition=" '$(IsTargetingPackBuilding)' == '' AND '$(VersionPrefix)' == '3.1.2' ">true</IsTargetingPackBuilding> |
There was a problem hiding this comment.
FYI I already made these changes in https://github.com/dotnet/aspnetcore/pull/18151/files so there may be some conflict when merging.
JunTaoLuo
reviewed
Jan 10, 2020
| @@ -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="'$(IsServicingBuild)' == 'true'">$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).0</TargetingPackVersionPrefix> | |||
| <TargetingPackVersionPrefix Condition="'$(IsTargetingPackBuilding)' != 'true'">$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).2</TargetingPackVersionPrefix> | |||
Pilchie
approved these changes
Jan 10, 2020
There was a problem hiding this comment.
This looks good to me based on the conversations I've had so far with @ericstj and @nguerrera
Pilchie
added
the
feature-platform
dougbu
requested changes
Jan 10, 2020
|
This was approved for 3.1.2 in #17957 |
wtgodbe
commented
Jan 10, 2020
dougbu
approved these changes
Jan 10, 2020
There was a problem hiding this comment.
Looks good but please do a fake non-servicing build after cleaning your repo e.g. port to 'master'. That'll confirm build ordering doesn't mess up the changes to the App.Ref. (App.Runtime depends on App.Ref, not the other way 'round. But, I'm not sure about when the specific packaging targets run.)
| @@ -53,7 +53,8 @@ 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> | |||
| <ReferencePlatformManifestPath Condition="'$(IsServicingBuild)' != 'true'">$(PlatformManifestOutputPath)</ReferencePlatformManifestPath> | |||
There was a problem hiding this comment.
nit:
Suggested change
| <ReferencePlatformManifestPath Condition="'$(IsServicingBuild)' != 'true'">$(PlatformManifestOutputPath)</ReferencePlatformManifestPath> | |
| <ReferencePlatformManifestPath>$(PlatformManifestOutputPath)</ReferencePlatformManifestPath> |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I generated this platform manifest based off of the 3.1.0 runtime pack. Checking it into the repo is sufficient for 3.1.2, since there is no "known good" platform manifest for 3.1.x that we can harvest and insert into the package. Replaces #17957. Copied from there:
I'll open an issue to track coming up with a better solution in case we need to service the targeting pack in the future - ideally a solution would be general across all repos that generate a targeting pack
CC @nguerrera @dagood @ericstj @Pilchie @JunTaoLuo @jkotalik @dougbu