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

[master] Update dependencies from dotnet/msbuild #16350

Merged

Conversation

dotnet-maestro[bot]
Copy link
Contributor

@dotnet-maestro dotnet-maestro bot commented Mar 13, 2021

This pull request updates the following dependencies

From https://github.com/dotnet/msbuild

  • Subscription: bf9f701b-6ee8-4142-55c5-08d8c8769860
  • Build: 20210316.7
  • Date Produced: 3/16/2021 8:34 PM
  • Commit: 17677364d656a98ce4c6eff73b49eddf24f0fd72
  • Branch: refs/heads/main

…0313.4

Microsoft.Build.Localization , Microsoft.Build
 From Version 16.10.0-preview-21162-01 -> To Version 16.10.0-preview-21163-04
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@sfoslund
Copy link
Member

@Forgind @benvillalobos it looks like we're seeing a real error here and in #16349:

C:\code\sdk\artifacts\bin\redist\Debug\dotnet\sdk\6.0.100-dev\NuGet.targets(131,5): The given key '16414' was not present in the dictionary. [C:\code\sdk\artifacts\tmp\Debug\It_builds_non---266F020C\NetStandardAndNetCoreApp\NetStandardAndNetCoreApp.csproj]

@benvillalobos
Copy link
Member

First-pass investigation:

The failing task in nuget.targets is:

    <RestoreTask
      RestoreGraphItems="@(_RestoreGraphEntryFiltered)"
      RestoreDisableParallel="$(RestoreDisableParallel)"
      RestoreNoCache="$(RestoreNoCache)"
      RestoreIgnoreFailedSources="$(RestoreIgnoreFailedSources)"
      RestoreRecursive="$(RestoreRecursive)"
      RestoreForce="$(RestoreForce)"
      HideWarningsAndErrors="$(HideWarningsAndErrors)"
      Interactive="$(NuGetInteractive)"
      RestoreForceEvaluate="$(RestoreForceEvaluate)"
      RestorePackagesConfig="$(RestorePackagesConfig)"/>
  </Target>

Not sure what 16404 is referring to. $(HideWarningsAndErrors) makes me think it could be warningsaserrors. Is it possible this test was expecting previous WarningsAsErrors functionality?

What changed: Say a task logged a warning that was turned into an error and returned !Log.HasLoggedErrors. After this change, the task now properly returns false and the build would fail. Previously HasLoggedErrors didn't know when it logged a warning that turned into an error.

@Forgind
Copy link
Member

Forgind commented Mar 15, 2021

What BenVillalobos said sounds right. Looking at the other PRs that it could theoretically be, most of them prevented exceptions from being thrown (i.e., added a try/catch, deleted an error, etc.) or had no code impact in any scenario here.

What test failed? Does it assume the build will succeed but include an error?

…0316.2

Microsoft.Build.Localization , Microsoft.Build
 From Version 16.10.0-preview-21162-01 -> To Version 16.10.0-preview-21166-02
@sfoslund
Copy link
Member

What test failed?

GivenThatWeWantToBuildACrossTargetedLibrary.It_builds_nondesktop_library_successfully_on_all_platforms is failing, and I don't think it's expected to have any errors or warnings, it's just building a relatively simple project with no changes:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>netcoreapp1.1;netstandard1.5</TargetFrameworks>
  </PropertyGroup>
  <PropertyGroup Condition="'$(TargetFramework)' == 'netcoreapp1.1'">
    <OutputType>exe</OutputType>
    <DefineConstants>NETCOREAPP;$(DefineConstants)</DefineConstants>
    <AssetTargetFallback>dnxcore50</AssetTargetFallback>
  </PropertyGroup>
  <ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp1.1'">
    <PackageReference Include="Newtonsoft.Json" Version="7.0.1" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)' == 'netstandard1.5'">
    <PackageReference Include="Newtonsoft.Json" Version="9.0.1" />
  </ItemGroup>
</Project>

@Forgind
Copy link
Member

Forgind commented Mar 16, 2021

Could it be sending a warning because .NET Core 1.1 is out of support?

@sfoslund
Copy link
Member

Could it be sending a warning because .NET Core 1.1 is out of support?

That happens with a lot of the SDK tests, I don't think it's the cause here.

@nkolev92 have you seen this error before?

C:\code\sdk\artifacts\bin\redist\Debug\dotnet\sdk\6.0.100-dev\NuGet.targets(131,5): The given key '16414' was not present in the dictionary. [C:\code\sdk\artifacts\tmp\Debug\It_builds_non---266F020C\NetStandardAndNetCoreApp\NetStandardAndNetCoreApp.csproj]

@Forgind
Copy link
Member

Forgind commented Mar 16, 2021

It looks like it was a warnings-as-errors problem. We have a dictionary access without checking that the key is actually in the dictionary here. I should be able to make a PR to fix it.

@marcpopMSFT
Copy link
Member

Build with the fix finished a few hours ago but the PR hasn't updated. Let's check back tomorrow and if still not, we can check if the default branch rename affected the subscriptions.

…0316.7

Microsoft.Build.Localization , Microsoft.Build
 From Version 16.10.0-preview-21162-01 -> To Version 16.10.0-preview-21166-07
@dotnet-maestro dotnet-maestro bot merged commit 6649a4d into master Mar 17, 2021
@dotnet-maestro dotnet-maestro bot deleted the darc-master-78248a99-d47b-46b4-94c4-21364cb57a2f branch March 17, 2021 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants