disable restore in vs for projects targeting platform specific old frameworks #44932

Anipik · 2020-11-19T06:00:42Z

Fixes #32205

Command line restore still works.
disables these flags just for these slns, not on the whole repo level.
the error message now will be to just run restore for test projects from the cli for older platform specific frameworks

official docs https://docs.microsoft.com/en-us/nuget/consume-packages/package-restore

ghost · 2020-11-19T06:00:47Z

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #32205

Command line restore still works.
disables these flags just for these slns, not on the whole repo level.
the error message now will be to just run restore for test projects from the cli for older platform specific frameworks

official docs https://docs.microsoft.com/en-us/nuget/consume-packages/package-restore

Author:	Anipik
Assignees:	-
Labels:	`area-Infrastructure-libraries`
Milestone:	-

ViktorHofer · 2020-11-19T15:43:59Z

I think it would be great if we could also add a script that automats and eventually protects this (in combination with my slngen script):

Grep all src project TargetFrameworks and search for duplicate tfms < net5.0, save that information in a list.
Scan if solution files contain one of the items from the list.
Add the NuGet.config template to the leafs that are affected.

If you don't mind, I will push into your branch to add that logic.

src/libraries/Microsoft.Win32.Registry.AccessControl/NuGet.config

Anipik · 2020-11-19T16:35:24Z

If you don't mind, I will push into your branch to add that logic.

sure go ahead

…der frameworks

ViktorHofer · 2020-11-20T17:29:27Z

Ok, I'll get to it on Monday. Let's keep this open till then so that I can verify your change with the script.

Anipik · 2020-11-30T18:05:10Z

@ViktorHofer any update on this one ?

ViktorHofer · 2020-11-30T22:37:21Z

hopefully I'll get to it tomorrow.

ViktorHofer · 2020-12-02T17:20:30Z

@Anipik just pushed the code into your PR. Seems like there were a few NuGet.config files missing which are now also part of the PR.

@akoeplinger I also addressed your feedback and converted the ps1 script into a proj file.

ViktorHofer · 2020-12-02T17:23:29Z

Ok not quite there. System.Windows.Extensions is not discovered by the script.

ViktorHofer · 2020-12-02T17:25:14Z

OK the script is working fine, it just didn't update the file as it was already there with the same content. Here's the list of slns in which restore is now disabled:

Copying NuGet.config template file for Microsoft.Extensions.Configuration.Xml
  Copying NuGet.config template file for Microsoft.Extensions.Configuration
  Copying NuGet.config template file for Microsoft.Extensions.Hosting
  Copying NuGet.config template file for Microsoft.Extensions.Logging.EventLog
  Copying NuGet.config template file for Microsoft.Extensions.Logging
  Copying NuGet.config template file for Microsoft.VisualBasic.Core
  Copying NuGet.config template file for Microsoft.Win32.Registry.AccessControl
  Copying NuGet.config template file for Microsoft.Win32.Registry
  Copying NuGet.config template file for System.Configuration.ConfigurationManager
  Copying NuGet.config template file for System.Data.Odbc
  Copying NuGet.config template file for System.Data.Oledb
  Copying NuGet.config template file for System.Diagnostics.EventLog
  Copying NuGet.config template file for System.Diagnostics.PerformanceCounter
  Copying NuGet.config template file for System.Diagnostics.Process
  Copying NuGet.config template file for System.DirectoryServices.AccountManagement
  Copying NuGet.config template file for System.DirectoryServices.Protocols
  Copying NuGet.config template file for System.DirectoryServices
  Copying NuGet.config template file for System.Drawing.Common
  Copying NuGet.config template file for System.IO.FileSystem.AccessControl
  Copying NuGet.config template file for System.IO.IsolatedStorage
  Copying NuGet.config template file for System.IO.Pipes.AccessControl
  Copying NuGet.config template file for System.IO.Pipes
  Copying NuGet.config template file for System.IO.Ports
  Copying NuGet.config template file for System.Management
  Copying NuGet.config template file for System.Net.Http.WinHttpHandler
  Copying NuGet.config template file for System.Net.HttpListener
  Copying NuGet.config template file for System.Net.Http
  Copying NuGet.config template file for System.Net.Mail
  Copying NuGet.config template file for System.Net.NameResolution
  Copying NuGet.config template file for System.Net.Requests
  Copying NuGet.config template file for System.Net.Security
  Copying NuGet.config template file for System.Net.Sockets
  Copying NuGet.config template file for System.Resources.Extensions
  Copying NuGet.config template file for System.Resources.ResourceManager
  Copying NuGet.config template file for System.Runtime.Caching
  Copying NuGet.config template file for System.Runtime.Serialization.Formatters
  Copying NuGet.config template file for System.Runtime
  Copying NuGet.config template file for System.Security.AccessControl
  Copying NuGet.config template file for System.Security.Cryptography.Algorithms
  Copying NuGet.config template file for System.Security.Cryptography.Cng
  Copying NuGet.config template file for System.Security.Cryptography.Csp
  Copying NuGet.config template file for System.Security.Cryptography.OpenSsl
  Copying NuGet.config template file for System.Security.Cryptography.Pkcs
  Copying NuGet.config template file for System.Security.Cryptography.ProtectedData
  Copying NuGet.config template file for System.Security.Cryptography.X509Certificates
  Copying NuGet.config template file for System.Security.Cryptography.Xml
  Copying NuGet.config template file for System.Security.Permissions
  Copying NuGet.config template file for System.Security.Principal.Windows
  Copying NuGet.config template file for System.ServiceProcess.ServiceController
  Copying NuGet.config template file for System.Text.Encoding.CodePages
  Copying NuGet.config template file for System.Threading.AccessControl
  Copying NuGet.config template file for System.Windows.Extensions

ViktorHofer · 2020-12-02T17:28:35Z

OK so PR is ready now. The scripts shows that the validation was important. One NuGet.config file was too much (Microsoft.Win32.SystemEvents) and about 25 were missing.

ericstj · 2020-12-02T17:42:16Z

src/libraries/slngen.proj

+          Outputs="%(Identity)">
+    <!-- Check if project file contains duplicate tfms -->
+    <PropertyGroup>
+      <ProjectFileContent>$([System.IO.File]::ReadAllText('%(SourceProject.Identity)'))</ProjectFileContent>


Why are we doing this instead of just calling a target in the project? Is it for perf? Is there a way to do a faster load of a project (eg: skipping imports or something) to improve perf, but still get MSBuild do the parsing/eval of the project?

There isn't a target in the SDK currently that returns the TargetFrameworks string in its raw form which we need in this case. I also wouldn't want to call into msbuild to eval/parse the project as it just isn't worth it.

Are you concerned with folks using properties in the TargetFrameworks string or using something that fools these regexs?

A couple small comments on the method, if you keep the regex approach, avoid raising the entire project content to a property, instead feed it directly into the regex. Also, it seems like your regex is already matching the TargetFrameworks tags, so you should be able to avoid using string.Replace calls by setting property to the captured content.

Also, it seems like your regex is already matching the TargetFrameworks tags, so you should be able to avoid using string.Replace calls by setting property to the captured content.

Unfortunately using Regex.Match as a property function doesn't allow to specify an inner capture group. It only returns the full capture which includes the boundaries.

Are you concerned with folks using properties in the TargetFrameworks string or using something that fools these regexs?

Not at all, I tested it with properties as well. It works with the inlined values or with the markers like $(NetCoreAppCurrent). I'm not concerned about derived properties as we don't have those today and it's less likely that we will add ones for anything older than net5.0.

If we would ever need full evaluation of it, we can switch to a different model of course :)

One more thing. I spoke with Rainer about this and there currently isn't a way from within msbuild to do this without writing a task which I wanted to avoid. There's a proposal though: dotnet/msbuild#3911.

OK, really last comment. The TargetFramework.Sdk today mutates the TargetFrameworks property which is yet another reason why I don't want to depend on the evaluated values. As I see this project as a mitigator for a point in time problem, I'm ok with the solution.

That's fair. You might be able to use Regex replace instead of match in order to use a capture group.

That's possible but if you don't mind I would just leave as is as CI is already green and this would only be code clean-up (removing two lines). But appreciate the feedback :)

Sure, this is less blocking since this is just a manually run tool

src/libraries/slngen.proj

Anipik requested review from ViktorHofer, ericstj and safern November 19, 2020 06:00

Dotnet-GitSync-Bot added the area-Infrastructure-libraries label Nov 19, 2020

Anipik changed the title ~~disable vs restore in vs for projects targeting platform specific old frameworks~~ disablerestore in vs for projects targeting platform specific old frameworks Nov 19, 2020

Anipik changed the title ~~disablerestore in vs for projects targeting platform specific old frameworks~~ disable restore in vs for projects targeting platform specific old frameworks Nov 19, 2020

ViktorHofer reviewed Nov 19, 2020

View reviewed changes

src/libraries/Microsoft.Win32.Registry.AccessControl/NuGet.config Outdated Show resolved Hide resolved

disable vs restore in vs for projects targeting platform specific ol…

e2982e2

…der frameworks

runfoapp bot mentioned this pull request Nov 20, 2020

Can_Run_App_With_StatiHost installer test failure in CI #42101

Closed

ViktorHofer added 2 commits December 2, 2020 18:18

Add infra to auto-update solution files for restore

7f6ce64

Add NuGet.config files for solutions affected by restore issue

0057453

ViktorHofer closed this Dec 2, 2020

ViktorHofer reopened this Dec 2, 2020

Remove M.W32.SystemEvents NuGet.config

0714554

ericstj reviewed Dec 2, 2020

View reviewed changes

src/libraries/slngen.proj Outdated Show resolved Hide resolved

Add some docs

8f872d8

ViktorHofer approved these changes Dec 2, 2020

View reviewed changes

ViktorHofer merged commit 659c5fb into dotnet:master Dec 2, 2020

runfoapp bot mentioned this pull request Dec 3, 2020

FrameworkHiveSelection_GlobalHiveWithBetterMatch failed in CI #45510

Closed

ghost locked as resolved and limited conversation to collaborators Jan 1, 2021

Anipik deleted the nuget branch January 30, 2021 21:42

disable restore in vs for projects targeting platform specific old frameworks #44932

disable restore in vs for projects targeting platform specific old frameworks #44932

Uh oh!

Conversation

Anipik commented Nov 19, 2020

Uh oh!

ghost commented Nov 19, 2020

Uh oh!

ViktorHofer commented Nov 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Anipik commented Nov 19, 2020

Uh oh!

ViktorHofer commented Nov 20, 2020

Uh oh!

Anipik commented Nov 30, 2020

Uh oh!

ViktorHofer commented Nov 30, 2020

Uh oh!

ViktorHofer commented Dec 2, 2020

Uh oh!

ViktorHofer commented Dec 2, 2020

Uh oh!

ViktorHofer commented Dec 2, 2020

Uh oh!

ViktorHofer commented Dec 2, 2020

Uh oh!

ericstj Dec 2, 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!

ViktorHofer Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

ericstj Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

ViktorHofer Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

ViktorHofer Dec 2, 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!

ViktorHofer Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

ViktorHofer Dec 2, 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!

ericstj Dec 2, 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!

ViktorHofer Dec 2, 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!

ericstj Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ViktorHofer commented Nov 19, 2020 •

edited

Loading

ericstj Dec 2, 2020 •

edited

Loading

ViktorHofer Dec 2, 2020 •

edited

Loading

ViktorHofer Dec 2, 2020 •

edited

Loading

ericstj Dec 2, 2020 •

edited

Loading

ViktorHofer Dec 2, 2020 •

edited

Loading