-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Fix BuildAllConfigurations #15754
Fix BuildAllConfigurations #15754
Changes from 2 commits
36c0d5a
1793360
33e27ef
8791a78
fb30574
843cc24
d549c08
d676bca
e12a443
eef6073
e3fd245
48680c2
3010d7d
ac5b1e7
814253c
42844e6
a2f3d67
758a0e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,4 +42,7 @@ | |
<Target Name="GetTargetPath" | ||
DependsOnTargets="Compile;GetBinplaceItems" | ||
Returns="@(BinplaceItem)" /> | ||
|
||
<!-- Don't do any filtering of Targeting pack packages --> | ||
<Target Name="FilterTargetingPackResolvedNugetPackages" /> | ||
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. Should we put this in the depproj.targets file? 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 can in a follow up. |
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" DefaultTargets="Build"> | ||
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" /> | ||
<PropertyGroup> | ||
<IsReferenceAssembly>true</IsReferenceAssembly> | ||
<BinPlaceRef>true</BinPlaceRef> | ||
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. Note this change will conflict with my PR #15747. We need to make sure one of us gets this in correctly. 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. This doesn't actually conflict. IsReferenceAssembly still works, but it triggers the "OmitTransitiveCompileReferences" behavior which was trimming out all of NETStandard.Library. For the TargetingPack packages they are flat so they wouldn't hit this. We should try to be consistent about how we specify this, but that can be cleanup, not conflict. 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. Yep I'm cleaning that up with my current PR. |
||
<NuGetDeploySourceItem>Reference</NuGetDeploySourceItem> | ||
<NugetRuntimeIdentifier>None</NugetRuntimeIdentifier> | ||
</PropertyGroup> | ||
|
@@ -21,27 +21,9 @@ | |
<RefPath Condition="'$(ForShims)' != 'true'">$(RefPath)</RefPath> | ||
<RefPath Condition="'$(ForShims)' == 'true'">$(NetFxRefPath)</RefPath> | ||
</BinplaceConfiguration> | ||
</ItemGroup> | ||
|
||
<!-- Filter the targeting pack to just these assemblies which we need netcoreapp shims for --> | ||
<ItemGroup> | ||
<TargetingPackReference Include="mscorlib" /> | ||
<TargetingPackReference Include="System" /> | ||
<TargetingPackReference Include="System.Core" /> | ||
<TargetingPackReference Include="System.Drawing" /> | ||
<TargetingPackReference Include="System.Numerics" /> | ||
<TargetingPackReference Include="System.Runtime.Serialization" /> | ||
<TargetingPackReference Include="System.Web" /> | ||
<TargetingPackReference Include="System.Xml" /> | ||
<TargetingPackReference Include="System.Xml.Linq" /> | ||
</ItemGroup> | ||
<ItemGroup Condition="'$(ForShims)' != 'true'"> | ||
<TargetingPackReference Include="System.ComponentModel.DataAnnotations" /> | ||
<TargetingPackReference Include="System.Data" /> | ||
<TargetingPackReference Include="System.Security" /> | ||
<TargetingPackReference Include="System.ServiceProcess" /> | ||
<TargetingPackReference Include="System.Transactions" /> | ||
<TargetingPackReference Include="WindowsBase" /> | ||
|
||
<FileToExclude Include="System.EnterpriseServices.Thunk" /> | ||
<FileToExclude Include="System.EnterpriseServices.Wrapper" /> | ||
</ItemGroup> | ||
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" /> | ||
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. I'm interested in understanding why the filtered approach didn't work still. My expectation is that our builds didn't need the full targeting pack and would likely be able to get away with just a smaller subset. 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 can filter what goes into netfx. Previously I had a separate filter for netfx/net4* configured projects and shims. This broke because they needed to use the same folder. The shims will always be a smaller subset than the libs we build against so we need filtering in the shims. We can easily add back filtering to the depprojs at will in the future. |
||
</Project> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does BinPlaceRef != true equate to not a runtime assembly? I thought we do have some cases where it would be a runtime assembly that we bin place into ref.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See below. We're only here if a project only declared BinPlaceRef (and not BinPlaceRuntime). This doesn't break anything, it allows for projects to opt in to only ref without getting side effects of claiming to be IsReferenceAssembly.