Adding net461 target to Microsoft.Bcl.* packages#38104
Adding net461 target to Microsoft.Bcl.* packages#38104joperezr merged 2 commits intodotnet:masterfrom
Conversation
|
@dotnet/dncenghot Looks like CI is broken, error says I checked and eng/pipelines/linux.yml does still exist, so is this a known issue? |
|
@joperezr Yeah, this is a known issue between GitHub and Azure DevOps. For now retrying is the only recourse. See https://github.com/dotnet/core-eng/issues/6579 |
|
CI is pretty busted right now, but we managed to run the CI again with my latest changes, here is the link of the build: https://dev.azure.com/dnceng/public/_build/results?buildId=207266 Once it is green I'll go ahead and merge this. |
|
CI is green, so I'll go ahead and merge. |
| <Reference Include="System" /> | ||
| <Reference Include="System.Core" /> | ||
| <Reference Include="System.ValueTuple" /> | ||
| <Reference Include="System.Numerics.Vectors" /> |
There was a problem hiding this comment.
Why did we end up needing these additional references? Should they be in a separate condition block where only TargetsNetFx is true?
There was a problem hiding this comment.
We can separate them out if you prefer, but this itemgroup was already applying for netstandard build and netfx. When in netstandard, these extra references will not do any harm as they are only facades in netstandard and don't have any typedefs, so the resulting assembly will be exactly the same. I wanted to keep the netstandard and netfx configurations as joint as possible when adding the extra configuration, which is why I didn't bother to add the extra itemgroup. That said, we can change this if you prefer.
| </ProjectReference> | ||
| <ProjectReference Include="..\src\System.Text.Json.csproj" /> | ||
| <ProjectReference Include="..\src\System.Text.Json.csproj"> | ||
| <SupportedFramework>net461;netcoreapp2.0;uap10.0.16299;$(AllXamarinFrameworks)</SupportedFramework> |
There was a problem hiding this comment.
Can you explain why we need to add the src project?
There was a problem hiding this comment.
The source project was already there, all we did here was adding the Supported frameworks clause to the source project. I did it because this project will not have the ref asset in the package because of the following property:
corefx/src/System.Text.Json/pkg/System.Text.Json.pkgproj
Lines 19 to 23 in 2469a91
Since we have that, we do still want to make sure that we do validation even when only the src project is the one includded in the package, hence I added the supported clause to the source project.
* Adding net461 target to Microsoft.Bcl.* packages * Adding System.Text.Json net461 target as well Commit migrated from dotnet/corefx@fdab38c
4 participants
Fixes #37929
cc: @onovotny @stephentoub
@ahsonkhan I'll try to add Json package to this PR as well, but I do want to be able to get this in for the next preview, so I will add that separate in case I see that I am running out of time.