Use RuntimeFrameworkVersion instead for directory name #82
Conversation
| @@ -17,6 +17,6 @@ | |||
| </DepsFiles> | |||
| </ItemGroup> | |||
|
|
|||
| <Copy SourceFiles="%(DepsFiles.FullPath)" DestinationFiles="$(DepsOutputPath)\%(DepsFiles.PackageName)\Microsoft.NETCore.App\2.0.0\deps.json" /> | |||
| <Copy SourceFiles="%(DepsFiles.FullPath)" DestinationFiles="$(DepsOutputPath)\%(DepsFiles.PackageName)\Microsoft.NETCore.App\$(RuntimeFrameworkVersion)\deps.json" /> | |||
There was a problem hiding this comment.
@glennc also found that we need to add shared into this path. Also the file name needs to be .deps.json instead of just deps.json.
so DestinationFiles="$(DepsOutputPath)\%(DepsFiles.PackageName)shared\\Microsoft.NETCore.App\$(RuntimeFrameworkVersion)\%(DepsFiles.PackageName).deps.json"
There was a problem hiding this comment.
Looks good, but make sure to get a sign-off from @JunTaoLuo before checking in.
| @@ -7,16 +7,6 @@ | |||
| </PropertyGroup> | |||
|
|
|||
| <ItemGroup> | |||
| <PackageReference Include="@(RuntimeStorePackageReference)" /> | |||
| <PackageReference Include="@(RuntimeStorePackageReference)" Version="$(AspNetCoreVersion)" PrivateAssets="None" /> | |||
There was a problem hiding this comment.
why the version and private assets attributes?
| @@ -0,0 +1,4 @@ | |||
| public class Program | |||
There was a problem hiding this comment.
hate to put a copyright header on this but we probably should. Unless we start relocating these files out of src/ add copyright to every source code file.
| <MsBuild Projects="$(MetaPackageFile)" Targets="CollectDeps" Properties="DepsOutputPath=$(DepsOutputPath)"/> | ||
|
|
||
| <!--- MSBuild caches things if you run inproc so have to use Exec --> | ||
| <Exec Command="dotnet msbuild /t:Restore;Rebuild;CollectDeps $(HostingStartupTemplateFile) /p:DepsOutputPath=$(DepsOutputPath);HostingStartupPackageName=%(HostingStartupPackageReference.Identity);HostingStartupPackageVersion=%(Version)"/> |
src/TrimDeps/TrimDeps.csproj
Outdated
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Newtonsoft.Json" Version="10.0.1" /> |
There was a problem hiding this comment.
put the version in dependencies.props.
|
|
||
| <Target Name="CollectDeps"> | ||
| <PropertyGroup> | ||
| <DestinationDepsFile>$(DepsOutputPath)\$(HostingStartupPackageName)\shared\Microsoft.NETCore.App\$(RuntimeFrameworkVersion)\deps.json</DestinationDepsFile> |
There was a problem hiding this comment.
must have some kind of file name in front of deps.json. Could be as simple as app.deps.json
src/TrimDeps/Program.cs
Outdated
| { | ||
| public static void Main(string[] args) | ||
| { | ||
| ChangeEntryPointLibraryName(args[0], null); |
There was a problem hiding this comment.
I'm a little confused what is trying to be achieved here. From the code below it looks like you are only removing the target library which is the first entry under targets/tfm and libraries. In that case why bother with the second parameter? Also is it always guaranteed that the first entry is what we want to trim? Is it better to be logging for the specific package *HostingStartup.Template I presume?
There was a problem hiding this comment.
Sorry, should have added WIP to commit message, I copied this from the code I wrote to build shared runtime I'm going to trim it today.
| string version = null; | ||
| foreach (JProperty target in deps["targets"]) | ||
| { | ||
| var targetLibrary = target.Value.Children<JProperty>().FirstOrDefault(); |
There was a problem hiding this comment.
Is this logic too fragile? Are we guaranteed that the first entry is always the one we want to remove? Should we not check the name of the assembly or something?
There was a problem hiding this comment.
https://github.com/dotnet/core-setup/blob/6887ab556bc8302390782711dcdd75b75e769cf5/src/test/build/shared-build-targets-utils/Utils/SharedFrameworkPublisher.cs#L149 we use it to build shared runtime, I think it's good enough.
No description provided.