[master] Update dependencies from nuget/nuget.client #12897

dotnet-maestro · 2020-08-12T17:59:43Z

This pull request updates the following dependencies

From https://github.com/NuGet/NuGet.Client

Subscription: a3c26c65-d5fb-4ade-e67f-08d808024d62
Build: 5.8.0.6776
Date Produced: 8/12/2020 12:22 AM
Commit: 62e098bfb22998fa638ce1a6dff5f91276770500
Branch: dev

Updates:
- NuGet.Build.Tasks: from 5.8.0-preview.1.6753 to 5.8.0-preview.2.6776

…5.8.0.6776 NuGet.Build.Tasks From Version 5.8.0-preview.1.6753 -> To Version 5.8.0-preview.2.6776

dotnet-maestro · 2020-08-12T18:04:45Z

Auto-Merge Status

This pull request has not been merged because Maestro++ is waiting on the following merge policies.

❓ Waiting on checks: dotnet-sdk-public-ci, dotnet-sdk-public-ci (Build Darwin Build_Release), dotnet-sdk-public-ci (Build Darwin Build_Debug), dotnet-sdk-public-ci (Build Ubuntu_16_04 Build_Release), dotnet-sdk-public-ci (Build Windows_NT_FullFramework Build_Release), dotnet-sdk-public-ci (Build Ubuntu_16_04 Build_Debug), dotnet-sdk-public-ci (Build Windows_NT Build_Release), dotnet-sdk-public-ci (Build Windows_NT_TestAsTools Build_Debug), dotnet-sdk-public-ci (Build Windows_NT_FullFramework Build_Debug), dotnet-sdk-public-ci (Build Windows_NT Build_Debug)
❌ Standard Merge Policies There are reviews that have requested changes.
✔️ Standard Merge Policies Succeeded - No version downgrade detected.

sfoslund

Requesting changes to avoid automerge

sfoslund · 2020-08-13T16:03:09Z

For test It_fails_gracefully_if_targetframework_should_be_targetframeworks:

F:\workspace_work\1\s\artifacts\bin\redist\Debug\dotnet\sdk\5.0.100-ci\NuGet.targets(128,5): error : Invalid restore input. Invalid target framework 'unsupported'.

It looks like we might have 2 separate issues here, restore is failing before we can get to build, where we should be erroring in this test.

@dsplaisted @nkolev92

dotnet-maestro · 2020-08-13T16:07:16Z

Auto-Merge Status

This pull request has not been merged because Maestro++ is waiting on the following merge policies.

❌ Standard Merge Policies Unsuccessful checks: dotnet-sdk-public-ci (Build Darwin Build_Debug), dotnet-sdk-public-ci (Build Ubuntu_16_04 Build_Debug)
❌ Standard Merge Policies There are reviews that have requested changes.
✔️ Standard Merge Policies Succeeded - No version downgrade detected.

sfoslund · 2020-08-13T17:25:52Z

Update: b50756f should fix the issue from this comment. NuGet started looking at the TargetFrameworkIdentifier with this change, so we need to update the logic to take this into account.

dotnet-maestro · 2020-08-13T17:28:00Z

Auto-Merge Status

This pull request has not been merged because Maestro++ is waiting on the following merge policies.

❓ Waiting on checks: dotnet-sdk-public-ci, dotnet-sdk-public-ci (Build Darwin Build_Release), dotnet-sdk-public-ci (Build Darwin Build_Debug), dotnet-sdk-public-ci (Build Windows_NT_FullFramework Build_Release), dotnet-sdk-public-ci (Build Windows_NT Build_Release), dotnet-sdk-public-ci (Build Ubuntu_16_04 Build_Release), dotnet-sdk-public-ci (Build Windows_NT_FullFramework Build_Debug), dotnet-sdk-public-ci (Build Windows_NT Build_Debug), dotnet-sdk-public-ci (Build Ubuntu_16_04 Build_Debug)
❌ Standard Merge Policies There are reviews that have requested changes.
✔️ Standard Merge Policies Succeeded - No version downgrade detected.

marcpopMSFT · 2020-08-13T17:47:31Z

src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs

+        public static NuGetFramework GetTargetFramework(string targetFrameworkMoniker, string targetPlatformMoniker)
+        {
+            var targetFramework = NuGetUtils.ParseFrameworkName(targetFrameworkMoniker);
+            if (targetPlatformMoniker != null && !targetPlatformMoniker.Equals(string.Empty))


Any reason not to use !string.IsNullOrEmpty here?

zkat · 2020-08-13T18:06:56Z

@sfoslund Is there an issue for this on the dotnet side that we can track as blocking our insertion on our own end?

dotnet-maestro · 2020-08-13T18:08:26Z

Auto-Merge Status

This pull request has not been merged because Maestro++ is waiting on the following merge policies.

❌ Standard Merge Policies Unsuccessful checks: dotnet-sdk-public-ci, dotnet-sdk-public-ci (Build Windows_NT_FullFramework Build_Release), dotnet-sdk-public-ci (Build Windows_NT_FullFramework Build_Debug)
❌ Standard Merge Policies There are reviews that have requested changes.
✔️ Standard Merge Policies Succeeded - No version downgrade detected.

nkolev92 · 2020-08-13T18:28:31Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ComposeStore.targets

    <FilterResolvedFiles  AssetsFilePath="$(ProjectAssetsFile)"
                          ResolvedFiles ="@(_ResolvedCopyLocalPublishAssets)"
                          PackagesToPrune="$(PackagesToPrune)"
                          TargetFramework="$(TargetFrameworkMoniker)"


I think you want TargetFramework here.

Then you can ask NuGet about the framework based on the alias.

nkolev92 · 2020-08-13T18:31:09Z

src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs

+            {
+                try
+                {
+                    var targetPlatform = NuGetUtils.ParseFrameworkName(targetPlatformMoniker);


In https://github.com/NuGet/NuGet.Client/compare/dev-nkolev92-tfmImprovements?expand=1 I am adding a ParseComponents method that should take a TFM & TPM.

nkolev92 · 2020-08-13T18:31:38Z

src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs

+
+    internal static class NuGetTargetFrameworkUtils
+    {
+        public static NuGetFramework GetTargetFramework(string targetFrameworkMoniker, string targetPlatformMoniker)


For all tasks that make use of the assets file, ideally instead of looking it up by framework, you'd look up by the alias.

dsplaisted · 2020-08-13T18:20:21Z

src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs

+
+                    return constructedFramework;
+                }
+                catch


We should generally avoid generic catch blocks like this. What exception are we expecting?

I was seeing an exception with invalid target platform versions without this change, which was odd because I couldn't repro it locally. In general I agree, but as this is a work around and should be removed soon I think it should be okay.

dsplaisted · 2020-08-13T18:32:18Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.TargetFrameworkInference.targets

  -->
  <Target Name="_CheckForUnsupportedTargetFramework"
-          BeforeTargets="_CheckForInvalidConfigurationAndPlatform;RunResolvePackageDependencies;GetFrameworkPaths;GetReferenceAssemblyPaths"
+          BeforeTargets="_CheckForInvalidConfigurationAndPlatform;RunResolvePackageDependencies;GetFrameworkPaths;GetReferenceAssemblyPaths;Restore"


@zkat @nkolev92 What kind of impact will this have?

As a result of NuGet/NuGet.Client#3548, NuGet restore now fails when you have a (singular) TargetFramework property with a semicolon in it, with the message:

F:\workspace_work\1\s\artifacts\bin\redist\Debug\dotnet\sdk\5.0.100-ci\NuGet.targets(128,5): error : Invalid restore input. Invalid target framework 'unsupported'.

So now we will run our target to check for that case before restore, so we generate a better error message.

Will this cause problems for scenarios where restore could succeed, or partially succeed when there was an invalid TargetFramework? What about VS restore, which doesn't even run the Restore target?

@clairernovotny, the comment in the updated test seems to indicate that MSBuild.Sdk.Extras might be impacted by this. Can you comment on that?

Sorry, which comment in which test?

@clairernovotny https://github.com/dotnet/sdk/pull/12897/files#diff-afae3e8319c39f94ba88319480ed1917

NuGet won't restore if it ever detects an "unsupported" framework.

NuGet could probably try to throw a better error though.

As far as hinging it off of restore, it won't run in VS.

Fairly sure this was to ensure that we could define the right additional properties from the TFM so that restore would work.

For example, it'd set TPV, TFV and others: https://github.com/novotnyllc/MSBuildSdkExtras/blob/master/Source/MSBuild.Sdk.Extras/Build/Platforms/Windows.targets

I don't think it ever explicitly cared/supported having an extra semi-colon in a TargetFramework element.

The above invalid target framework 'unsupported' is exactly what we're hitting at the moment in dotnet/aspnetcore and dotnet/aspnetcore-tooling with VS 16.8.0 preview 2.0 (Int Preview) installed. Will the ;Restore addition here fix that❔

/fyi builds in the dotnet/aspnetcore repo primarily use dotnet msbuild. We hit this error there only when attempting to use desktop msbuild for *.vcxproj and *.wixproj projects. Errors go away when only the external preview version of VS (16.8.0 preview 1.0) is visible to the build.

(Believe users of the "master" version of VS also see the above error.)

Adding ;Restore will cause a different failure if the issue is that TargetFramework is being used instead of TargetFrameworks otherwise, this is a different issue.

I haven't checked aspnetcore-tooling but aspnetcore does not use @(TargetFramework) items with semicolons in them. We do have a mix of projects, some using @(TargetFramework) and those that are multi-targeted using @(TargetFrameworks). In addition, the only projects given to desktop msbuild in the aspnetcore failure case are *.proj and *.vcxproj projects. So, if it isn't this bug that's breaking us, what is❔

My error for the aspnetcore case with the Int Preview version of Visual Studio and .NET SDK version 5.0.100-rc.1.20379.10:

C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.targets(128,5): error : Invalid restore input. Invalid target framework 'unsupported'. Input files: C:\Users\dougbu\.nuget\packages\microsoft.dotnet.arcade.sdk\5.0.0-beta.20403.5\tools\Tools.proj. [C:\Users\dougbu\.nuget\packages\microsoft.dotnet.arcade.sdk\5.0.0-beta.20403.5\tools\Tools.proj]

The Tools.proj project contains <TargetFramework>net472</TargetFramework> and doesn't use an SDK. Is not specifying an SDK no longer supported❔

If this fix does address our problem (even if accidentally), will we need more of NuGet/Home#9911 to build successfully or just an update of our .NET SDK to a verioin with this fix in it❔

jmarolf · 2020-08-14T17:22:56Z

seeing this failure in the test run for test Analysis_is_enabled_when_analysislevel_set_to_5:

Using shared compilation with compiler from directory: C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\MSBuild\Current\Bin\Roslyn
CSC : error CS1900: Warning level must be in the range 0-4 [F:\workspace\_work\1\s\artifacts\tmp\Release\Analysis_is_e---B4F1A654\HelloWorld\HelloWorld.csproj]
Done Building Project "F:\workspace\_work\1\s\artifacts\tmp\Release\Analysis_is_e---B4F1A654\HelloWorld\HelloWorld.csproj" (WriteValuesToFile target(s)) -- FAILED.

suggesting that these tests are running against a version of the compiler that is too old. We restricted the version of MSBuild on these tests to be "16.8" but that is apparently not enough. The version of MSBuild in VS 16.8 preview 1 is not new enough.

dotnet-maestro · 2020-08-14T17:27:51Z

Auto-Merge Status

This pull request has not been merged because Maestro++ is waiting on the following merge policies.

✔️ Standard Merge Policies Succeeded - Successful checks: dotnet-sdk-public-ci, dotnet-sdk-public-ci (Build Ubuntu_16_04 Build_Release), dotnet-sdk-public-ci (Build Ubuntu_16_04 Build_Debug), dotnet-sdk-public-ci (Build Darwin Build_Release), dotnet-sdk-public-ci (Build Windows_NT Build_Release), dotnet-sdk-public-ci (Build Windows_NT_FullFramework Build_Release), dotnet-sdk-public-ci (Build Darwin Build_Debug), dotnet-sdk-public-ci (Build Windows_NT Build_Debug), dotnet-sdk-public-ci (Build Windows_NT_TestAsTools Build_Debug), dotnet-sdk-public-ci (Build Windows_NT_FullFramework Build_Debug)
❌ Standard Merge Policies There are reviews that have requested changes.
✔️ Standard Merge Policies Succeeded - No version downgrade detected.

Sarah is OOF

Update dependencies from https://github.com/NuGet/NuGet.Client build …

b1aad43

…5.8.0.6776 NuGet.Build.Tasks From Version 5.8.0-preview.1.6753 -> To Version 5.8.0-preview.2.6776

sfoslund previously requested changes Aug 12, 2020

View reviewed changes

sfoslund force-pushed the darc-master-cd078d91-3c6f-4fe3-87e4-20a36a14c00e branch from bd8996e to 004f651 Compare August 13, 2020 15:54

sfoslund added 2 commits August 13, 2020 10:18

Adding work around for NuGet TFM/TPM lock file get target bug

97dbca8

Update tests for new nuget restore target framework logic

b50756f

sfoslund force-pushed the darc-master-cd078d91-3c6f-4fe3-87e4-20a36a14c00e branch from 004f651 to b50756f Compare August 13, 2020 17:24

marcpopMSFT reviewed Aug 13, 2020

View reviewed changes

nkolev92 reviewed Aug 13, 2020

View reviewed changes

dsplaisted approved these changes Aug 13, 2020

View reviewed changes

Reverting #12812

58d97f3

jmarolf mentioned this pull request Aug 14, 2020

allow user to specify 'latest', 'preview', or 'none' #12898

Merged

jmarolf mentioned this pull request Aug 14, 2020

Build.cmd throws error dotnet/roslyn-analyzers#4018

Closed

Update xcopy-msbuild

4753910

sfoslund force-pushed the darc-master-cd078d91-3c6f-4fe3-87e4-20a36a14c00e branch from 3a105ea to 4753910 Compare August 14, 2020 18:50

nkolev92 mentioned this pull request Aug 14, 2020

NuGet Target framework information breaking change coordination NuGet/Home#9911

Closed

7 tasks

Update test assertion for empty target platform moniker

19a636d

sfoslund force-pushed the darc-master-cd078d91-3c6f-4fe3-87e4-20a36a14c00e branch from a936110 to 19a636d Compare August 14, 2020 23:19

William Li added 2 commits August 15, 2020 22:49

Revert unnecesary file

1b91b02

MSBuild version is file version. Better test fail error

bf6be1a

William Li added 2 commits August 16, 2020 09:11

Apply msbuild requirement to non fullframework

5471959

Format to trigger CI

a325cff

wli3 merged commit d5024a1 into master Aug 16, 2020

dotnet-maestro bot deleted the darc-master-cd078d91-3c6f-4fe3-87e4-20a36a14c00e branch August 16, 2020 21:23

dougbu mentioned this pull request Aug 20, 2020

Update SDK dotnet/aspnetcore#25081

Closed

runfoapp bot mentioned this pull request Sep 29, 2020

Test initial triage jaredpar/runfo#50

Closed

[master] Update dependencies from nuget/nuget.client #12897

[master] Update dependencies from nuget/nuget.client #12897

Uh oh!

Conversation

dotnet-maestro bot commented Aug 12, 2020

From https://github.com/NuGet/NuGet.Client

Uh oh!

dotnet-maestro bot commented Aug 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Auto-Merge Status

Uh oh!

sfoslund left a comment

Choose a reason for hiding this comment

Uh oh!

sfoslund commented Aug 13, 2020

Uh oh!

dotnet-maestro bot commented Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Auto-Merge Status

Uh oh!

sfoslund commented Aug 13, 2020

Uh oh!

dotnet-maestro bot commented Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Auto-Merge Status

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zkat commented Aug 13, 2020

Uh oh!

dotnet-maestro bot commented Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Auto-Merge Status

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clairernovotny Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmarolf commented Aug 14, 2020

Uh oh!

dotnet-maestro bot commented Aug 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Auto-Merge Status

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

dotnet-maestro bot commented Aug 12, 2020 •

edited

Loading

dotnet-maestro bot commented Aug 13, 2020 •

edited

Loading

dotnet-maestro bot commented Aug 13, 2020 •

edited

Loading

dotnet-maestro bot commented Aug 13, 2020 •

edited

Loading

clairernovotny Aug 13, 2020 •

edited

Loading

dotnet-maestro bot commented Aug 14, 2020 •

edited

Loading