search

NuGet / Home

Repo for NuGet Client issues
Other
1.49k stars 250 forks source link

packages.config (PC) to PackageReference (PR) Migration #5877

Open nkolev92 opened 7 years ago

nkolev92 commented 7 years ago

Status: In Progress

Issue to track new feature work around enabling packages.config (PC) to PackageReference (PR) upgrader workflow.

Here is the link to the spec.

Feel free to comment below with your feedback.

Related issues

https://github.com/NuGet/Home/issues/12388

Overall Issue Description:

Likely Project System Issues

NuGet Migration Issues:

NuGet PC to PR Migration Tool Issues

VS Extensibility Team Issues

C++ Team Issues

WPF Project Compat Issues:

Clean up our own Repro to remove the need for install.ps1

Work with package owners (see related item# https://github.com/NuGet/Home/issues/5960)

Validation to improve ecosystem for new push/ingested packages

Package Reference experience issues

AArnott commented 7 years ago

You might want to add https://github.com/NuGet/Home/issues/5894 to your list of issues to be fixed. Since NuGet contains WPF assets, this could bite you.

nkolev92 commented 7 years ago

Done. Thanks @AArnott.

natidea commented 6 years ago

As I understand it, 465204 is also a scenario that works with packages.config but not PackageReferences

mungojam commented 6 years ago

roslyn 22095 is one that is stopping us from migrating. I had it as a nuget issue originally. If nuget could have a type of lib that ended up being output to bin but would be flagged as not having its API exposed to consuming projects, then it wouldn't be a problem.

AArnott commented 6 years ago

@mungojam NuGet packages can already depend on another package without exposing the API from that package to their own consumer. I think that that problem is solved at the right place right now because, for example, if package A depends on package B, we might say the ideal situation is that package A hides package B from the compiler of the app project if and only if no APIs from package B are exposed from package A's API.

Let's take an example:

Package B defines public class Tree and Package A derives from it: public class AppleTree : Tree. It is important then that anyone referencing Package A automatically get Package B's types so the compiler doesn't emit an error when using AppleTree telling the user to add a reference to resolve Tree as a base type. That would be a bad experience for Package A's consumer if using it was broken by default. Also, considering Package A's public API itself depends on Package B, package A cannot remove its dependency on Package B without it being a binary breaking change, so it's unlikely they would do that--at least not without a major version increment per semver.

But now consider Package A only defines internal class AppleTree (note it is internal). Package B is now an implementation detail of Package A. Now it is appropriate for Package A to specify B as a dependency in their nuspec, including the attribute to omit B as a reference to the compiler that builds the app. Now the app can't accidentally take a dependency on B without the app developer adding a PackageReference to B explicitly, thereby mitigating the problem you're calling out.

The issue that remains, I think, is that most packages out there today don't do due diligence to mark their dependencies carefully in this way. And even if a package author wanted to, it's not trivial to audit your public API for dependencies of this kind, nor to maintain that properly over time. So tooling to automatically discern between public dependencies and internal dependencies, and possibly automatically setting the appropriate package dependency metadata may be the best way to solve your problem in general.

mungojam commented 6 years ago

@AArnott Thanks for reviewing it, you've summed it up perfectly. I may be missing a trick then, how do we specify that a particular dependency is only to be added as an internal dependency and not exposed to compiler further up the chain?

I agree that tooling would definitely solve it nicely if the nuget mechanism exists already.

mungojam commented 6 years ago

@AArnott Is this the mechanism you are referring to, using contentFiles?

<contentFiles>
  <file include="bin/Release/SomeInternalDependency.dll" buildAction="None" copyToOutput="true" flatten="true" />
</contentFiles>

I might see what happens with a wildcard in the include. Maybe we could do that for the packages we make and then add external dependencies explicitly in an exclude.. sounds yukky, but might work.

AArnott commented 6 years ago

No, don't use contentFiles for this. When building your own package via csproj with PackageReference items, you add an attribute like this (if this were package A):

  <PackageReference Include="PackageB" Version="1.0" PrivateAssets="compile" />

This makes it so your own package A consumes and references package B in your compiler references, but folks consuming package A will not get package B in their assembly references.

Documented here

mungojam commented 6 years ago

Thanks, that does work, actually using "compile;contentfiles;analyzers;build" to keep the default behaviour of PrivateAssets for the other types.

So, I will raise a feature request for the tooling improvement that you suggested, recognising that it won't be an easy thing to implement.

By the way, I had tried PrivateAssets previously and hit various errors. Some my own doing (package name clash). Other issues I ran into have been raised as issues already against the project system I think. I had to keep closing VS 15.4 and deleting obj/bin/.vs folders for various changes to package references to work. I hope that is fixed soon as it is a blocker for us too.

awesley commented 6 years ago

Is it the case that #4491 should be included in this checklist?

rohit21agrawal commented 6 years ago

@awesley https://github.com/NuGet/Home/issues/6285 is tracking that work and is included in the list in the OP.

khellang commented 6 years ago

I can't see this mentioned anywhere, but I assume this would work just as well from the CLI, right? Remember, not everyone uses Visual Studio these days 😉

nkolev92 commented 6 years ago

@khellang packages.config installation/uninstallation works correctly only in Visual Studio.

Unfortunately the migration will only work in VS due to that limitation.

IanMercer commented 6 years ago

Do we really need a backup? In 2018? For source code? It's just one more annoying directory to delete before commit.

clairernovotny commented 6 years ago

@IanMercer you would be surprised...

khellang commented 6 years ago

@IanMercer I think a lot of people (most?) already have backup folders ignored. It's even part of GitHub's .gitignore template for Visual Studio 😄

sliepie commented 6 years ago

Is this gonna work for Asp.Net Website (WSP) projects? These projects have no csproj file and currently they have support for NuGet using a packages.config file. If packages.config files are deprecated/removed in the feature, WSP projects have no story for using NuGet?

rohit21agrawal commented 6 years ago

@JarnoNijboer currently, the migrator will not be enabled for website projects. But that does not mean that packages.config is going to be deprecated, it will continue to be supported until there is a clear path forward to move to PackageReference

rohit21agrawal commented 6 years ago

while the migrator will be available in 15.7, some of the issues listed in the OP still need to be worked on. Moving this to 4.8

CoolDadTx commented 6 years ago

I ran it against one of my class library projects and it added a lot more than just the package reference stuff. It added entries for PublishUrl, Intall.., Update..,Application.., etc. It also added Bootstrap entries. This is way more than I wanted. Just replace the package references - why is it making non-package related changes as well.

From:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
  <PropertyGroup>
    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
    <OutputType>Library</OutputType>
    <TargetFrameworkVersion>v4.6.1</TargetFrameworkVersion>
    ...
  </PropertyGroup>

To:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
  <PropertyGroup>
    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
    <OutputType>Library</OutputType>
    <TargetFrameworkVersion>v4.6.1</TargetFrameworkVersion>
    ...
   <PublishUrl>publish\</PublishUrl>
    <Install>true</Install>
    <InstallFrom>Disk</InstallFrom>
    <UpdateEnabled>false</UpdateEnabled>
    <UpdateMode>Foreground</UpdateMode>
    <UpdateInterval>7</UpdateInterval>
    <UpdateIntervalUnits>Days</UpdateIntervalUnits>
    <UpdatePeriodically>false</UpdatePeriodically>
    <UpdateRequired>false</UpdateRequired>
    <MapFileExtensions>true</MapFileExtensions>
    <ApplicationRevision>0</ApplicationRevision>
    <ApplicationVersion>1.0.0.%2a</ApplicationVersion>
    <IsWebBootstrapper>false</IsWebBootstrapper>
    <UseApplicationTrust>false</UseApplicationTrust>
    <BootstrapperEnabled>true</BootstrapperEnabled>
  </PropertyGroup>

And added this-

<ItemGroup>
    <BootstrapperPackage Include=".NETFramework,Version=v4.6.1">
      <Visible>False</Visible>
      <ProductName>Microsoft .NET Framework 4.6.1 %28x86 and x64%29</ProductName>
      <Install>true</Install>
    </BootstrapperPackage>
    <BootstrapperPackage Include="Microsoft.Net.Framework.3.5.SP1">
      <Visible>False</Visible>
      <ProductName>.NET Framework 3.5 SP1</ProductName>
      <Install>false</Install>
    </BootstrapperPackage>
  </ItemGroup>
rohit21agrawal commented 6 years ago

@CoolDadTx that definitely doesn’t sound right. Any chance you can give us a repro project or your list of packages in packages.config ?

joelweiss commented 6 years ago

@CoolDadTx I had a similar issue whenever I made changes to project file these tags were added.

The problem was, my project once had ClickOnce and after removing ClickOnce there were still some stuff left behind and it tried to add again all the removed ClickOnce tags.

My solution was to make sure all ClickOnce tags were removed and the tags stopped reappearing.

Maybe your cause is similar.

CoolDadTx commented 6 years ago

Do you know what element triggered ClickOnce because this was a class library project that had no references to ClickOnce at any point? The only element I see that might have been auto-added when the project was created was TargetFrameworkProfile.

CoolDadTx commented 6 years ago

@rohit21agrawal

Packages config:

<?xml version="1.0" encoding="utf-8"?>
<packages>
  <package id="AutoMapper" version="6.0.2" targetFramework="net461" />
  <package id="EntityFramework" version="6.1.3" targetFramework="net452" />
  <package id="Fsmb" version="2.0.1.0" targetFramework="net451" />
  <package id="Fsmb.Apollo" version="5.0.17009.1" targetFramework="net461" />
  <package id="Fsmb.Augusta" version="2.5.17177.1" targetFramework="net461" />
  <package id="Fsmb.Augusta.Cdc" version="2.5.17052.1" targetFramework="net461" />
  <package id="Fsmb.Augusta.EntityFramework" version="2.5.17177.1" targetFramework="net461" />
  <package id="Fsmb.EnterpriseServices.Logging" version="3.0.17083.3" targetFramework="net461" />
  <package id="Fsmb.EnterpriseServices.Reporting" version="1.0.14065.5" targetFramework="net452" />
</packages>

We have a private VSTS repo where most of the packages come from but the rest are in NuGet.

rohit21agrawal commented 6 years ago

@CoolDadTx oh, I might not be able to test this out if your packages are coming from a private VSTS repo.

can you try this: 1) Create a plain old packages.config based Class Library (Full .NET Framework). Check the project file. 2) Install the packages you listed above 3) Check the project file for any changes after installation. 4) Migrate from packages.config to PackageReference . 5) Look for any further changes.

If there is something confidential about your projects/packages that you can't share with us, I can work on getting a NDA so I can inspect where the migration is going wrong?

CoolDadTx commented 6 years ago

I tried a test scenario and couldn't replicate it. I wiped my changes from the original project and ran it again and it didn't do it this time. Strange, maybe some other extension was trying to do something as well. I'll consider this issue closed for now. Thanks.

rohit21agrawal commented 6 years ago

thanks @CoolDadTx for verifying! If you do have any other feedback/issues with the migrator, please do share with us!

CoolDadTx commented 6 years ago

@rohit21agrawal, I do have one complaint but I suspect there is nothing you can do about it. When the migrator is moving packages it is auto-generating an app.config file if one does not exist. I suspect this is because it is trying to uninstall a package that had transforms (i.e. EntityFramework) but I'm only guessing here. Nevertheless every project that has the migration tool run on it (that didn't have an app.config before) now has an empty one that has to be deleted. I replicated this by creating a class library that was using EntityFramework but had no config (because it was a library).

rohit21agrawal commented 6 years ago

@CoolDadTx installing EntityFramework to a packages.config based ClassLibrary actually ended up creating an app.config for me with the following content:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <configSections>

    <section name="entityFramework" type="System.Data.Entity.Internal.ConfigFile.EntityFrameworkSection, EntityFramework, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" requirePermission="false" />
  </configSections>
  <entityFramework>
    <defaultConnectionFactory type="System.Data.Entity.Infrastructure.LocalDbConnectionFactory, EntityFramework">
      <parameters>
        <parameter value="mssqllocaldb" />
      </parameters>
    </defaultConnectionFactory>
    <providers>
      <provider invariantName="System.Data.SqlClient" type="System.Data.Entity.SqlServer.SqlProviderServices, EntityFramework.SqlServer" />
    </providers>
  </entityFramework>
</configuration>
CoolDadTx commented 6 years ago

@rohit21agrawal, correct. Now go to the project and delete the app.config file. Then go out to the file system and ensure it is gone. Save the project. Then run the package migration tool. Notice there is no app.config in the project but if you go out to the file system it has in fact created an empty app.config file. That config file will be pushed to Git if you happen to be using it.

tamirdresher commented 6 years ago

I tried running the tool on one of my projects and VS crashes. I attached to it from another VS instance to see why and i'm seeing an exception thrown with the message "Illegal characters in path." I can't figure out why this is happening.

This the stack trace:

    mscorlib.dll!System.IO.Path.CheckInvalidPathChars(string path, bool checkAdditional)    Unknown
    mscorlib.dll!System.IO.Path.GetFileName(string path = "D:\\tfs\\Testvs17\\.\t\t")   Unknown
    Microsoft.TeamFoundation.Common.dll!Microsoft.TeamFoundation.Common.FileSpec.GetFullPath(string path = "D:\\tfs\\Testvs17\\.\t\t", bool checkForIllegalDollar = false)  Unknown
>   Microsoft.TeamFoundation.VersionControl.Client.dll!Microsoft.TeamFoundation.VersionControl.Client.Client.GetLocalWorkspace(string localPath, bool throwIfNotFound = false)  Unknown
    NuGet.PackageManagement.VisualStudio.dll!NuGet.PackageManagement.VisualStudio.DefaultTFSSourceControlManager.DefaultTFSSourceControlManager(NuGet.Configuration.ISettings settings, EnvDTE80.SourceControlBindings sourceControlBindings)   Unknown
    NuGet.PackageManagement.VisualStudio.dll!NuGet.PackageManagement.VisualStudio.VsSourceControlManagerProvider.GetSourceControlManager.AnonymousMethod__4_0() Unknown
    mscorlib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.InvokeMoveNext(object stateMachine)  Unknown
    mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)   Unknown
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)   Unknown
    mscorlib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.Run()    Unknown
    Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.JoinableTaskFactory.SingleExecuteProtector.TryExecute()   Unknown
    Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.JoinableTaskFactory.SingleExecuteProtector..cctor.AnonymousMethod__20_0(object state) Unknown
    WindowsBase.dll!System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate callback, object args, int numArgs)  Unknown
    WindowsBase.dll!System.Windows.Threading.ExceptionWrapper.TryCatchWhen(object source = {System.Windows.Threading.Dispatcher}, System.Delegate callback, object args, int numArgs, System.Delegate catchHandler = null)  Unknown
    WindowsBase.dll!System.Windows.Threading.DispatcherOperation.InvokeImpl()   Unknown
    WindowsBase.dll!System.Windows.Threading.DispatcherOperation.InvokeInSecurityContext(object state)  Unknown
    WindowsBase.dll!MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(object obj)   Unknown
    mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)   Unknown
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)   Unknown
    mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state) Unknown
    WindowsBase.dll!MS.Internal.CulturePreservingExecutionContext.Run(MS.Internal.CulturePreservingExecutionContext executionContext = {MS.Internal.CulturePreservingExecutionContext}, System.Threading.ContextCallback callback, object state)    Unknown
    WindowsBase.dll!System.Windows.Threading.DispatcherOperation.Invoke()   Unknown
    WindowsBase.dll!System.Windows.Threading.Dispatcher.ProcessQueue()  Unknown
    WindowsBase.dll!System.Windows.Threading.Dispatcher.WndProcHook(System.IntPtr hwnd, int msg, System.IntPtr wParam, System.IntPtr lParam, ref bool handled)  Unknown
    WindowsBase.dll!MS.Win32.HwndWrapper.WndProc(System.IntPtr hwnd = {System.IntPtr}, int msg = 49946, System.IntPtr wParam = {System.IntPtr}, System.IntPtr lParam = {System.IntPtr}, ref bool handled = false)   Unknown
    WindowsBase.dll!MS.Win32.HwndSubclass.DispatcherCallbackOperation(object o) Unknown
    WindowsBase.dll!System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate callback, object args, int numArgs)  Unknown
    WindowsBase.dll!System.Windows.Threading.ExceptionWrapper.TryCatchWhen(object source = {System.Windows.Threading.Dispatcher}, System.Delegate callback, object args, int numArgs, System.Delegate catchHandler = null)  Unknown
    WindowsBase.dll!System.Windows.Threading.Dispatcher.LegacyInvokeImpl(System.Windows.Threading.DispatcherPriority priority, System.TimeSpan timeout, System.Delegate method, object args, int numArgs)   Unknown
    WindowsBase.dll!MS.Win32.HwndSubclass.SubclassWndProc(System.IntPtr hwnd = {System.IntPtr}, int msg = 49946, System.IntPtr wParam = {System.IntPtr}, System.IntPtr lParam = {System.IntPtr})    Unknown
rohit21agrawal commented 6 years ago

@tamirdresher this is because the API call to Path.GetFileName is throwing when it's getting " D:\tfs\Testvs17\.\t\t " The problem is the . in the path.

Do you know where is this path coming from?

tamirdresher commented 6 years ago

@rohit21agrawal I saw the path but i have no idea where is it coming from. The root folder for the *.sln is D:\tfs\Testvs17\ but i dont know where the . and the rest is coming from. I unbound the entire folder from TFS (VSTS) and now it works. Do you have clue why working with a project that is bound to TFS can cause this error?

rohit21agrawal commented 6 years ago

@tamirdresher I am investigating the issue - can you tell me if the solution packages folder is also bound to TFS?

UPDATE: I am unable to repro the issue in either case :/

Any chance you can create a repro TFS project and share it with me?

rohit21agrawal commented 6 years ago

@CoolDadTx looked at your issues. It's actually a package issue with EntityFramework where they end up creating that file on uninstall. You can repro it by following the same steps, except that instead of migrating , uninstall the EntityFramework package and you will see the empty app.config appear. Unfortunately, there is nothing we can do about it :( .

techie55 commented 6 years ago

Hi, I tried upgrading my VS 2017 project to PackageReference. I upgraded using the tool given here:https://marketplace.visualstudio.com/items?itemName=CloudNimble.NuGetPackageReferenceUpgrader Once the upgrade was done, I deleted the packages folder from my solution and then Restored Nuget Packages. Seeing the following issues:

  1. It again recreates the packages folder under my solution directory as well as the global-cache directory %userprofile%.nuget\packages. Shouldn't the packages be created under the global-cache %userprofile%.nuget\packages only?
  2. The build fails. For now I am going to revert the changes
jainaashish commented 6 years ago

@techie55 that tool is not provided from NuGet team so it might not be following the right pattern to upgrade all your existing packages from packages.config to PackageRef.

Please use latest VS preview release from https://www.visualstudio.com/vs/preview/ and then try upgrading your individual projects to PackageRef and let us know if you see any issue there. Please find more details in this blogpost

Falconne commented 6 years ago

When this tool is officially released, will it be possible to use the command line to invoke a migration, so it can be scripted? We have hundreds of solutions across many repos and doing this manually would be a pain.

flcdrg commented 6 years ago

Just checking - Did this ship in 15.7, or should I see it in the 15.8 preview (now that 15.7 has shipped)

(I ask as I can't see it in either of these)

jainaashish commented 6 years ago

It's shipped as part of 15.7. Please find more details from https://blog.nuget.org/20180409/migrate-packages-config-to-package-reference.html and if you still don't see the option then open an issue with repro steps.

techie55 commented 6 years ago

@jainaashish Same question as @flcdrg . I do not see the option in my Visual Studio Community version

Attaching the images to verify. visual_studio_version nuget

rohit21agrawal commented 6 years ago

The option is enabled only on certain projects and disabled for projects such as C++ and ASP.NET to name a few. Also, make sure you open the NuGet package manager UI or console first before checking for the migration option

techie55 commented 6 years ago

@rohit21agrawal We should probably update the documentation to add the following step: Make sure you open the NuGet package manager UI or console first before checking for the migration option After doing the above step, I can see the option Migrate packages.config to PackageReference

Any reason why this tool cannot be enabled for ASP.NET projects?

ensemblebd commented 6 years ago

Just curious if anyone else has tried this on an existing project? When I try it, it causes a complete removal of all Nuget packages. Followed by an "invalid nuget configuration" error post migration. Followed by a working Nuget Manager ui after restarting vstudio, with a [restore] button which looks like it works but restores nothing. Followed by manually having to re-add all nuget packages, which results in a new packages.config file, starting me back at the beginning.

The migration wizard is clearly non functional and vstudio integration has some issues. I was excited to hear about this new feature. But now, not so much. I'll come back when the kinks are worked out. good day to you.

rohit21agrawal commented 6 years ago

Can you share a repro project with us please where this happens? This is clearly not supposed to happen and we are sorry you had to face this inconvenience. Having said that, we are very interested in solving this issue and making this experience better for you, so any details you can share with us will be greatly appreciated

ensemblebd commented 6 years ago

No apologies necessary, I know you guys are working on things far more advanced than I can even comprehend. I'll see if I can get a reproduction repo built up and on github later today, and will post back. Might even find the issue that way, thank you :)

memao commented 6 years ago

I am having similar issues as ensemblebd when I use the tool. Sometimes the restore failed, it suggest me to roll back. Sometimes it shows the migration succeed and .html/backup files are generated, but actually when I look at the modified .csproj file, no package reference is added or no entry is removed. I have to manullay re-add all package reference or remove those .

It doesn't happen to all my project, only some of them.

rohit21agrawal commented 6 years ago

@memao can you share some of the projects with us? If there is any confidential information involved, we can set up a NDA and follow due process.

rohit21agrawal commented 6 years ago

@techie55 the reason I suggested opening the PM UI/Console first is because I realized we ended up shipping a bug, for which I will file a tracking issue and fix as appropriate. In the meanwhile, we will work on documenting this workaround so people don't get blocked on this! Thanks for your feedback!

techie55 commented 6 years ago

Hi, I feel this feature is not very useful because it is not implemented for Web and WebAPI projects. Most of our projects are web based and I would have loved to see this feature available for Web projects as well