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.

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

2.1 Reference resolution project fails to add patched CoreFx assembly to Microsoft.Aspnetcore.App #30279

Closed
wtgodbe opened this issue Feb 18, 2021 · 2 comments
Assignees
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Done This issue has been fixed
Milestone

Comments

@wtgodbe
Copy link
Member

wtgodbe commented Feb 18, 2021

In 2.1.26, we found that, despite CoreFx shipping a new version of System.Text.Encodings.Web, we failed to include the new version of the binary in Microsoft.Aspnetcore.App``. The reason for this is that the we didn't patch any of the 3 projects that depend on S.T.E.W, so all references to it were resolved as BaselinePackageReferences, which still point to the unpatched version. I found this by examining the binlog and noticing that, despite `SystemTextEncodingsWebPackageVersion` being updated to 4.5.0, all of the resolved `PackageReferences` to it had version 4.5.0, and were coming from our `ResolveCustomReferences` target. We should fix our logic so that the SharedFx always gets the latest version of any assembly coming from repos below us.

Strangely, this behavior was not present for at least one recent ingestion of a patched CoreFx library. System.IO.Pipelines was patched in the 2.1.23 build, and we did not ship any patched packages, but Microsoft.AspNetCore.App did include the latest version of System.IO.Pipelines. I haven't figured out why it worked then but not now.

Original text from email:

My working theory is that we’re actually resolving this dependency from our package baseline, where its version is [4.5.0, ]. The only projects we have that reference S.T.E.W all have it listed as BaselinePackageReferences in our baseline, so they all resolve it from there, and get the 4.5.0 version – nothing is restoring it as an external reference with the 4.5.1 version, So it’s the 4.5.0 version that actually gets restored. See these 3 projects which are the only ones that reference S.T.E.W:

<Reference Include="System.Text.Encodings.Web" />

<Reference Include="System.Text.Encodings.Web" />

<Reference Include="System.Text.Encodings.Web" />

And their baseline entries, which list a BaselinePackageReference to S.T.E.W at 4.5.0 or higher:

<ItemGroup Condition=" '$(PackageId)' == 'Microsoft.AspNetCore.Html.Abstractions' AND '$(TargetFramework)' == 'netstandard2.0' ">
<BaselinePackageReference Include="System.Text.Encodings.Web" Version="[4.5.0, )" />

<ItemGroup Condition=" '$(PackageId)' == 'Microsoft.AspNetCore.Http.Abstractions' AND '$(TargetFramework)' == 'netstandard2.0' ">
<BaselinePackageReference Include="Microsoft.AspNetCore.Http.Features" Version="[2.1.1, )" />
<BaselinePackageReference Include="System.Text.Encodings.Web" Version="[4.5.0, )" />

<ItemGroup Condition=" '$(PackageId)' == 'Microsoft.AspNetCore.WebUtilities' AND '$(TargetFramework)' == 'netstandard2.0' ">
<BaselinePackageReference Include="Microsoft.Net.Http.Headers" Version="[2.1.1, )" />
<BaselinePackageReference Include="System.Text.Encodings.Web" Version="[4.5.0, )" />

@wtgodbe
Copy link
Member Author

wtgodbe commented Feb 18, 2021

CC @dotnet/aspnet-build

@wtgodbe wtgodbe added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Feb 18, 2021
@wtgodbe wtgodbe added this to the 2.1.x milestone Feb 18, 2021
dougbu added a commit that referenced this issue Mar 4, 2021
- #30279
- ensure dependencies are up-to-date even when PatchConfig.props is empty
dougbu added a commit that referenced this issue Mar 8, 2021
- #30279
- ensure dependencies are up-to-date even when PatchConfig.props is empty
dougbu added a commit that referenced this issue Mar 12, 2021
- #30279
- ensure dependencies are up-to-date even when PatchConfig.props is empty
- change only CoreFx and core-setup dependencies
- also change transitive references that weren't mentioned before
dougbu added a commit that referenced this issue Mar 12, 2021
…30639)

* Correct and streamline internal builds
- correct manual internal builds
    - need to explicitly set `$(BuildNumber)` property in all build steps
    - rename `$(BuildScriptArgs)` to avoid circular reference
- skip tests by default in internal non-PR builds

nits:
- include new `$(BuildNumberArg)` in `$(SharedFxArgs)`
- run all test jobs in internal pull requests

* Always skip tests in SharedFx and later jobs
- however test\SharedFx.UnitTests\SharedFx.UnitTests.csproj always runs as part of `BuildSharedFx` target

* Include transitive refs in meta-package .nuspec files
- #30279
- ensure dependencies are up-to-date even when PatchConfig.props is empty
- change only CoreFx and core-setup dependencies
- also change transitive references that weren't mentioned before

* Bump a few patch versions
@dougbu
Copy link
Member

dougbu commented Mar 13, 2021

@dougbu dougbu closed this as completed Mar 13, 2021
@dougbu dougbu added the Done This issue has been fixed label Mar 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests

2 participants