Create common manifest file #188
Conversation
| @@ -175,8 +175,23 @@ | |||
| <Copy SourceFiles="$(RepositoryRoot)build\Build.RS.nuspec" DestinationFolder="$(ArtifactsDir)" Condition="'$(OSPlatform)'=='Linux'" /> | |||
| <WriteLinesToFile File="$(ArtifactsDir)version.txt" Lines="$(VersionPrefix)-$(VersionSuffix)" Overwrite="true" Condition="'$(OSPlatform)'=='Linux'" /> | |||
|
|
|||
| <!-- Preview 2 Workaround: remove Microsoft.AspNetCore.Mvc.Razor.ViewCompilation from publish manifest --> | |||
| <Exec Command="sed -i -e '/microsoft.aspnetcore.mvc.razor.viewcompilation/d' $(ArtifactsDir)%(PackageStoreManifestFiles.TimestampDestinationFile)" Condition="'$(OS)' != 'Windows_NT'" /> | |||
| <!-- Add a common manifest for package trimming --> | |||
There was a problem hiding this comment.
The previous workaround is no longer needed after aspnet/MvcPrecompilation#143. We only need to generate the common manifest once on the windows machines so I'm arbitrarily picking the win7-x64 builds.
build/repo.targets
Outdated
|
|
||
| <ItemGroup> | ||
| <PackageToTrim Include="runtime.win-arm64.runtime.native.system.data.sqlclient.sni" /> | ||
| <ProjectAssetsJson Include="$(RuntimeStoreReferenceDirectory)**\project.assets.json" /> |
There was a problem hiding this comment.
This step uses the restored lock file from tools/Build.RuntimeStore.References to determine the versions of the the packages that goes into the store. This project is guaranteed to be restored during the runtime store generation step.
|
|
||
| foreach (var package in Packages.Select(i => i.ItemSpec)) | ||
| { | ||
| var packageVersion = packages |
There was a problem hiding this comment.
There may be cleaner ways of doing this. along with parsing out the Package.Name/Version from the project.assets.json lines 40-42
|
|
||
| namespace RepoTasks | ||
| { | ||
| public class CreateCommonManifestTask : Task |
There was a problem hiding this comment.
Can you add a description of what this task does (for posterity and the next guy who has to edit this)?
build/repo.targets
Outdated
|
|
||
| <Error Text="@(ProjectAssetsJson->Count()) project.assets.json file(s) found." Condition="'@(ProjectAssetsJson->Count())' != 1" /> | ||
|
|
||
| <CreateCommonManifestTask DestinationFilePath="$(ArtifactsDir)$(CommonManifestFileName)" ProjectAssetsJsonFilePath="@(ProjectAssetsJson)" Packages="@(PackageToTrim)"/> |
There was a problem hiding this comment.
Nit: use the task namespace when using repo tasks. <RepoTasks.CreateCommonManifest ... Makes it more obvious where this task comes from.
|
|
||
| namespace RepoTasks | ||
| { | ||
| public class CreateCommonManifestTask : Task |
There was a problem hiding this comment.
Also, MSBuild convention is to leave off the Task name. public class CreateCommonManifest : Task
build/dependencies.props
Outdated
| <MoqVersion>4.7.49</MoqVersion> | ||
| <MsBuildPackageVersions>15.1.1012</MsBuildPackageVersions> |
There was a problem hiding this comment.
Latest MSBuild is 15.3.0.
There was a problem hiding this comment.
The current versions we use in our repos is 15.1.1012, see https://github.com/search?utf8=%E2%9C%93&q=org%3Aaspnet+filename%3Adependencies.props+MsBuildPackageVersions&type=Code. Is there a reason why we can use a higher version here or does this need to be updated everywhere?
There was a problem hiding this comment.
Doesn't really matter as these are reference assemblies. MSBuild resolves this assembly to whatever version it carried. But since you've pointed it out, we should upgrade the other tools in our org.
There was a problem hiding this comment.
Btw, seeing this makes me think it would be better if KoreBuild set common properties and package references so that each RepoTasks.csproj didn't need it....
There was a problem hiding this comment.
That would be nice.
build/tasks/RepoTasks.csproj
Outdated
| <PackageReference Include="Microsoft.Build.Utilities.Core" Version="$(MsBuildPackageVersions)" PrivateAssets="All" /> | ||
| <PackageReference Include="Newtonsoft.Json" Version="$(JsonNetInMSBuildVersion)" /> | ||
| </ItemGroup> | ||
| </Project> |
There was a problem hiding this comment.
Nit: add trailing newlines.
JunTaoLuo
force-pushed
the
johluo/trim-sni
branch
from
26c36a8 to
b71a81c
Compare
|
🆙📅 |
JunTaoLuo
force-pushed
the
johluo/trim-sni
branch
2 times, most recently
from
063caea to
6b97b52
Compare
build/repo.targets
Outdated
| @@ -1,6 +1,8 @@ | |||
| <Project> | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
There was a problem hiding this comment.
This is going to cause MSBuild warnings about double imports of Microsoft.Common.targets. We shouldn't be using Microsoft.NET.Sdk from repo.targets.
This will trim additional packages such as runtime.win-arm64.runtime.native.system.data.sqlclient.sni
JunTaoLuo
force-pushed
the
johluo/trim-sni
branch
from
48e203d to
bb1d80b
Compare
Addresses: #172
This will trim additional packages such as runtime.win-arm64.runtime.native.system.data.sqlclient.sni