Skip to content
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

Creating a runtime pack artifacts directory in the libraries build #34662

Merged
merged 27 commits into from
Apr 16, 2020
Merged

Creating a runtime pack artifacts directory in the libraries build #34662

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8321dea
[libraries] Clean up SetupTestingHost
mdh1418 Apr 8, 2020
be068f2
[libraries] Setup runtime pack artifacts directory
mdh1418 Apr 8, 2020
d22cee8
WIP havent addressed feedback or finished cleaning up
mdh1418 Apr 8, 2020
e432bbc
Fixed up some xml
Apr 8, 2020
002b96b
[libraries] Set and use BinPlaceTestRuntimePack property
mdh1418 Apr 8, 2020
c3ba797
Updates to structure. Build for ios now generates the right things
Apr 10, 2020
4d91c78
Feedback items
Apr 13, 2020
94a03ab
Removed targetOS check
Apr 13, 2020
3cafc0f
[libraries] Address feedback
mdh1418 Apr 13, 2020
d71ddd1
[libraries] Add DotNetBuildTasksSharedFrameworkTaskFile property to l…
mdh1418 Apr 13, 2020
84241b1
[libraries] Modify sdk import and rename __RuntimeId to PackageRID
mdh1418 Apr 13, 2020
b5568e4
Brought back BinPlaceTestSharedFramework to generate testhost. Will …
Apr 13, 2020
07965e5
Syntax error :-(
Apr 14, 2020
2b3b736
Extra > character lingering
Apr 14, 2020
484d0ff
[libraries] Normalize NetCoreAppRuntimePack directories
mdh1418 Apr 14, 2020
05d4759
[libraries] Cleanup paths after normalizing directories
mdh1418 Apr 14, 2020
73b6800
[libraries] Add comment to explain the lib and native folders under l…
mdh1418 Apr 14, 2020
0dad68b
[libraries] Simplify GetBinPlaceItems
mdh1418 Apr 14, 2020
d465796
[libraries] Remove superfluous condition
mdh1418 Apr 14, 2020
60b9009
[libraries] Condition BinPlaceNETFXRuntime on not targeting mobile an…
mdh1418 Apr 14, 2020
23da306
[libraries] Remove hardcoded NETCoreAppFrameworkIdentifier in pretest…
mdh1418 Apr 14, 2020
211eece
[libraries] Import Sdk.targets into pretest.proj as well
mdh1418 Apr 14, 2020
2d4d7f4
[libraries] Reduce Copy usage in runtime.depproj
mdh1418 Apr 15, 2020
757c871
[libraries] Exclude unwanted files from runtime packs
mdh1418 Apr 15, 2020
bc3991a
[libraries] Change to singular NativeBinPlaceItem
mdh1418 Apr 15, 2020
ba6d021
[libraries] Consolidate itemgroups in runtime.depproj and move runtim…
mdh1418 Apr 15, 2020
0d46895
[libraries] Move pretest.proj imports to end of the file
mdh1418 Apr 15, 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
11 changes: 10 additions & 1 deletion src/libraries/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,11 @@

<TestHostRootPath>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'testhost', '$(BuildSettings)'))</TestHostRootPath>

<NETCoreAppRuntimePackPath>$(ArtifactsBinDir)\lib-runtime-packs</NETCoreAppRuntimePackPath>
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved
<NETCoreAppRuntimePackLibPath>$(NETCoreAppRuntimePackPath)\runtimes\$(__RuntimeId)\lib\$(NETCoreAppFramework)</NETCoreAppRuntimePackLibPath>
<NETCoreAppRuntimePackNativePath>$(NETCoreAppRuntimePackPath)\runtimes\$(__RuntimeId)\native</NETCoreAppRuntimePackNativePath>
<!--<RuntimePackTargetFrameworkPath>runtimes/$(__RuntimeId)</RuntimePackTargetFrameworkPath>-->
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved
<RuntimePackTargetFrameworkPath>runtimes/$(PackageRID)</RuntimePackTargetFrameworkPath>

<VersionFileForPackages Condition="'$(VersionFileForPackages)' == ''">$(ArtifactsObjDir)version.txt</VersionFileForPackages>

Expand Down Expand Up @@ -304,9 +309,13 @@

<PropertyGroup Condition="'$(BuildAllConfigurations)' != 'true'">
<!-- We add extra binplacing for the test shared framework until we can get hardlinking with the runtime directory working on all platforms -->
<BinPlaceTestSharedFramework Condition="'$(BuildTargetFramework)' == '$(NetCoreAppCurrent)'">true</BinPlaceTestSharedFramework>
<BinPlaceTestSharedFramework Condition="'$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'tvOS' or '$(TargetOS)' == 'Android'">false</BinPlaceTestSharedFramework>
steveisok marked this conversation as resolved.
Show resolved Hide resolved
<BinPlaceTestSharedFramework Condition="'$(BinPlaceTestSharedFramework)' == '' and '$(BuildTargetFramework)' == '$(NetCoreAppCurrent)'">true</BinPlaceTestSharedFramework>

<BinPlaceNETFXRuntime Condition="'$(BuildTargetFramework)' == '$(NetFrameworkCurrent)'">true</BinPlaceNETFXRuntime>

<BinPlaceTestRuntimePack Condition="'$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'tvOS' or '$(TargetOS)' == 'Android'">true</BinPlaceTestRuntimePack>
Copy link
Member

Choose a reason for hiding this comment

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

It seems like we're starting to use these long condition in a lot of places: '$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'tvOS' or '$(TargetOS)' == 'Android' -- should we introduce a property to make it easier? Something like TargetsMobile?

Copy link
Member

Choose a reason for hiding this comment

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

I was wondering the same thing.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would make sense to do it now before we start adding it to more places. Looks like we should be able to define it here https://github.com/dotnet/runtime/blob/07965e57e294493757bb14f274137d7047628d3b/src/libraries/targetframework.props in the according when statements.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want to replace all instances of '$(TargetOS)' == 'iOS' or ... with a TargetsMobile property? Defining it in targetframework.props and replacing all instances of that condition everywhere seems to break things. Replacing only in src/installer and src/libraries builds w/o error, but we then lose a couple folders lib-runtime-packs/data lib-runtime-packs/runtimes/$(PackageRID)/native.

Copy link
Member

Choose a reason for hiding this comment

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

We can do it in a separate PR to not complicate things here.


<NETCoreAppTestSharedFrameworkPath>$([MSBuild]::NormalizeDirectory('$(TestHostRootPath)', 'shared', 'Microsoft.NETCore.App', '$(ProductVersion)'))</NETCoreAppTestSharedFrameworkPath>

<TestHostRuntimePath Condition="'$(BinPlaceTestSharedFramework)' == 'true'">$(NETCoreAppTestSharedFrameworkPath)</TestHostRuntimePath>
Expand Down
9 changes: 9 additions & 0 deletions src/libraries/Directory.Build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@
<RuntimePath>$(TestHostRootPath)</RuntimePath>
</BinPlaceTargetFrameworks>

<!-- Setup the runtime pack structure for mobile platforms (for testing and installer) -->
<BinPlaceTargetFrameworks Condition="'$(BinPlaceTestRuntimePack)' == 'true'" Include="$(NetCoreAppCurrent)-$(TargetOS)">
<RuntimePath>$(NETCoreAppRuntimePackLibPath)</RuntimePath>
</BinPlaceTargetFrameworks>
<BinPlaceTargetFrameworks Condition="'$(BinPlaceTestRuntimePack)' == 'true'" Include="$(NetCoreAppCurrent)-$(TargetOS)">
<RuntimePath>$(NETCoreAppRuntimePackNativePath)</RuntimePath>
<ItemName>NativeRuntimePackBinPlaceItem</ItemName>
</BinPlaceTargetFrameworks>
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved

<!-- binplace targeting packs which may be different from Build TargetFramework -->
<BinPlaceTargetFrameworks Include="netstandard2.0">
<RefPath>$(NetStandard20RefPath)</RefPath>
Expand Down
14 changes: 13 additions & 1 deletion src/libraries/Native/native-binplace.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

<!-- Ordering matters! Overriding GetBinPlaceItems and Build targets after the Sdk import. -->
<Target Name="GetBinPlaceItems">
<ItemGroup>
<ItemGroup Condition="'$(BinPlaceTestRuntimePack)' == ''">
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved
<BinPlaceItem Include="$(NativeBinDir)*.dll" />
<BinPlaceItem Include="$(NativeBinDir)*.pdb" />
<BinPlaceItem Include="$(NativeBinDir)*.lib" />
Expand All @@ -22,6 +22,18 @@
<BinPlaceItem Include="$(NativeBinDir)*.dwarf" />
<FileWrites Include="@(BinPlaceItem)" />
</ItemGroup>
<ItemGroup Condition="'$(BinPlaceTestRuntimePack)' == 'true'">
<NativeRuntimePackBinPlaceItem Include="$(NativeBinDir)/*.dll" />
<NativeRuntimePackBinPlaceItem Include="$(NativeBinDir)/*.pdb" />
<NativeRuntimePackBinPlaceItem Include="$(NativeBinDir)/*.lib" />
<NativeRuntimePackBinPlaceItem Include="$(NativeBinDir)/*.a" />
<NativeRuntimePackBinPlaceItem Include="$(NativeBinDir)/*.bc" />
<NativeRuntimePackBinPlaceItem Include="$(NativeBinDir)/*.so" />
<NativeRuntimePackBinPlaceItem Include="$(NativeBinDir)/*.dbg" />
<NativeRuntimePackBinPlaceItem Include="$(NativeBinDir)/*.dylib" />
<NativeRuntimePackBinPlaceItem Include="$(NativeBinDir)/*.dwarf" />
<FileWrites Include="@(NativeRuntimePackBinPlaceItem)" />
</ItemGroup>
</Target>

<Target Name="Build" DependsOnTargets="BinPlace" />
Expand Down
36 changes: 35 additions & 1 deletion src/libraries/pretest.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

<PropertyGroup>
<TraversalGlobalProperties>BuildAllProjects=true</TraversalGlobalProperties>
<NETCoreAppFrameworkIdentifier>.NETCoreApp</NETCoreAppFrameworkIdentifier>
<SharedFrameworkName>Microsoft.NETCore.App</SharedFrameworkName>
Copy link
Member

Choose a reason for hiding this comment

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

Share values with installer project tree? (Move to common ancestor?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Where would be a good place to place these properties in <runtimeroot>/src/Directory.Build.props?
Is it a significant impact to performance/organization to move it up? I'm not familiar enough to know now if those properties have a chance of changing or differing between the directories in <runtimeroot>/src. If they were to differ later would it be better to have each specify their own properties?

Copy link
Member

Choose a reason for hiding this comment

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

It might not be worth it. My knee-jerk reaction is that duplicating code is what needs justification rather than reusing it, so just a quick suggestion.

Copy link
Member

Choose a reason for hiding this comment

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

Let's open an issue for this. Consolidate all places in props and targets where we use Microsoft.NETCore.App to a single shared property.

</PropertyGroup>

<!-- Explicitly build the runtime.depproj project first to correctly set up the testhost. -->
Expand Down Expand Up @@ -35,7 +37,7 @@
<UsingTask TaskName="GenerateFileVersionProps" AssemblyFile="$(InstallerTasksAssemblyPath)"/>
<Target Name="GenerateFileVersionProps"
DependsOnTargets="CollectSharedFrameworkRuntimeFiles"
Condition="'$(PlatformManifestFile)' != '' and '$(BuildTargetFramework)' == '$(NetCoreAppCurrent)'"
Condition="'$(PlatformManifestFile)' != '' and '$(BuildTargetFramework)' == '$(NetCoreAppCurrent)' and '$(BinplaceTestSharedFramework)' == 'true'"
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved
AfterTargets="RestoreTestHost">
<GenerateFileVersionProps Files="@(SharedFrameworkRuntimeFiles)"
PackageId="Microsoft.NETCore.App"
Expand All @@ -58,4 +60,36 @@
RuntimeGraphFiles="$(RuntimeIdGraphDefinitionFile)"
TargetRuntimeIdentifier="$(PackageRID)" />
</Target>

<UsingTask TaskName="CreateFrameworkListFile" AssemblyFile="$(InstallerTasksAssemblyPath)"/>
steveisok marked this conversation as resolved.
Show resolved Hide resolved
<Target Name="GenerateFrameworkListFile"
Condition="'$(BinPlaceTestRuntimePack)' == 'true'"
AfterTargets="RestoreTestHost">
<PropertyGroup>
<FrameworkListFilename>RuntimeList.xml</FrameworkListFilename>
<FrameworkListFile>$(NETCoreAppRuntimePackPath)/data/$(FrameworkListFilename)</FrameworkListFile>
</PropertyGroup>

<ItemGroup>
<_rpLibFiles Include="$(NETCoreAppRuntimePackLibPath)\*.*">
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved
<TargetPath>$(RuntimePackTargetFrameworkPath)/lib/$(NETCoreAppFramework)</TargetPath>
<IsSymbolFile Condition="$([System.String]::Copy('%(Identity)').EndsWith('pdb'))">true</IsSymbolFile>
steveisok marked this conversation as resolved.
Show resolved Hide resolved
</_rpLibFiles>
<_rpNativeFiles Include="$(NETCoreAppRuntimePackNativePath)\*.*">
<TargetPath>$(RuntimePackTargetFrameworkPath)/native</TargetPath>
<IsNative>true</IsNative>
</_rpNativeFiles>

<FrameworkListRootAttributes Include="Name" Value=".NET $(NETCoreAppFrameworkVersion)" />
<FrameworkListRootAttributes Include="TargetFrameworkIdentifier" Value="$(NETCoreAppFrameworkIdentifier)" />
<FrameworkListRootAttributes Include="TargetFrameworkVersion" Value="$(NETCoreAppFrameworkVersion)" />
<FrameworkListRootAttributes Include="FrameworkName" Value="$(SharedFrameworkName)" />
</ItemGroup>

<CreateFrameworkListFile
Files="@(_rpLibFiles);@(_rpNativeFiles)"
TargetFile="$(FrameworkListFile)"
TargetFilePrefixes="ref/;runtimes/"
RootAttributes="@(FrameworkListRootAttributes)" />
</Target>
</Project>
28 changes: 24 additions & 4 deletions src/libraries/restore/runtime/runtime.depproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@
<FileToExclude Include="apphost" />
</ItemGroup>

<!-- In-tree runtime packs do not require the host, so exclude -->
<ItemGroup Condition="'$(BinPlaceTestRuntimePack)' == 'true'">
<FileToExclude Include="dotnet" />
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved
<FileToExclude Include="libnethost" />
<FileToExclude Include="libhostfxr" />
<FileToExclude Include="libhostpolicy" />
<FileToExclude Include="nethost" />
</ItemGroup>

<ItemGroup>
<!-- System.Data.SqlClient is not live-built anymore -->
<PackageReference Include="System.Data.SqlClient" Version="$(SystemDataSqlClientVersion)" />
Expand All @@ -44,9 +53,6 @@
Condition="'$(BinPlaceTestSharedFramework)' == 'true'"
AfterTargets="AfterResolveReferences">
<PropertyGroup>
<GlobalJsonContent>$([System.IO.File]::ReadAllText('$(RepoRoot)global.json'))</GlobalJsonContent>
<DotNetVersion>$([System.Text.RegularExpressions.Regex]::Match($(GlobalJsonContent), '(%3F&lt;="dotnet": ").*(%3F=")'))</DotNetVersion>

<HostFxrFileName Condition="'$(TargetsWindows)' == 'true'">hostfxr</HostFxrFileName>
<HostFxrFileName Condition="'$(TargetsWindows)' != 'true'">libhostfxr</HostFxrFileName>

Expand All @@ -73,6 +79,20 @@
<Exec Command="chmod +x $(TestHostRootPath)%(DotnetExe.Filename)%(DotnetExe.Extension)" Condition="'$(TargetOS)' != 'Windows_NT' and '$(OS)' != 'Windows_NT'"/>
</Target>

<Target Name="SetupRuntimePackNative"
Copy link
Member

Choose a reason for hiding this comment

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

Given this target is specific to Mono should we include Mono in the target name?

Copy link
Member

Choose a reason for hiding this comment

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

there are going to be some differences with desktop mono, so I'd like to address them in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

Do you have an issue tracking this work we can reference here?

Copy link
Member

Choose a reason for hiding this comment

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

#34203

Condition="'$(BinPlaceTestRuntimePack)' == 'true' and '$(RuntimeFlavor)' == 'Mono'"
AfterTargets="AfterResolveReferences">
<Copy SourceFiles="@(RuntimeFiles)"
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved
DestinationFolder="$(NETCoreAppRuntimePackNativePath)"
SkipUnchangedFiles="true" />
<Copy SourceFiles="@(MonoCrossFiles)"
DestinationFolder="$(NETCoreAppRuntimePackNativePath)/cross"
SkipUnchangedFiles="true" />
<Copy SourceFiles="@(MonoIncludeFiles)"
DestinationFolder="$(NETCoreAppRuntimePackNativePath)/include/%(RecursiveDir)"
SkipUnchangedFiles="true" />
</Target>

<Target Name="OverrideRuntimeCoreCLR"
DependsOnTargets="ResolveRuntimeFilesFromLocalBuild"
AfterTargets="AfterResolveReferences;FilterNugetPackages"
Expand All @@ -89,7 +109,7 @@
DependsOnTargets="ResolveRuntimeFilesFromLocalBuild"
AfterTargets="AfterResolveReferences;FilterNugetPackages"
Condition="'$(RuntimeFlavor)' == 'Mono'">
<ItemGroup>
<ItemGroup Condition="'$(BinPlaceTestRuntimePack)' == ''">
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved
<ReferenceCopyLocalPaths Include="@(RuntimeFiles)" />
</ItemGroup>
</Target>
Expand Down