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

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

Open
nkolev92 opened this issue Sep 11, 2017 · 86 comments

Comments

@nkolev92
Copy link
Member

commented Sep 11, 2017

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
Overall Issue Description:

  • #4942 After enabling PackageReference support for projects, some packages may not install or work correctly

Likely Project System Issues

  • dotnet/project-system#3042 - ContentFiles "Any" folder issue with classic csproj

  • dotnet/project-system#3431 Display ContentFiles in Legacy PR

  • #2365 - Some assemblies in NuGet packages should be linked, not referenced (Pertains to interop assemblies and the knowledge of whether an assembly needs to be referenced. Lost when moved to the transitive world)

  • dotnet/project-system#2859 - Project System side ASK for embedded interops

  • PP transforms - supported, but csproj.dll issue (link?) (see related #5880)

  • ContentFiles doesn't work with legacy PackageReference. Some issues in NuGet but also needs to coordinate with Project System as well. (#5958 dotnet/project-system#2861)

  • CPS projects can't handle contentFiles from separate packages using the same file name. (#5048)

  • https://devdiv.visualstudio.com/DevDiv/NuGet/_workitems/edit/567298 - Wildcard support in legacy project system Won't fix.

  • Internal bug 465204 - NuGet Build Tasks don’t put proper metadata on satellite assemblies (potentially a restore issue)

NuGet Migration Issues:

  • #5963 Come up with a strategy for packages that have install.ps1/uninstall.ps1 .
  • #5954 XDT Transforms don't work for transitive restore
  • NuGet.Config RepositoryPath controls Solution package directory for PC users. PR doesn't have a Solution package directory. (RestorePackagesPath can set the location of UserPackageFolder that is used by PC and PR codepaths) (Nikolche to confirm 2.5)
  • In order to pack normal PackageRef projects, you need to add a PR to NuGet.Build.Tasks.Pack, but there are still gaps (includereferencedprojects) - at that point, dotnet.exe pack will work, but nuget.exe pack won't. Related - #6285
  • EntityFramework story - #5974 & aspnet/EntityFramework6#189
  • what data should we collect to ensure we learn and adapt (this needs more thought)
  • #6009 - Update documentation for package reference

NuGet PC to PR Migration Tool Issues

  • #5488 - Provide migration tool to move from packages.config project to packagereference
  • #5879 - Add support for solution level PC -> PR migration
  • Ensure the user understands changes made for them (and give them options to tweak our behavior if necessary???)
  • P0 - enable project level migration (which needs to be able to walk up towards the root, changing each to PR.
    Changes to NuGet PM UI to adapt to transitive:
  • Transitive dependency management, list transitive package in a flatlist - #5887
  • Rework the UI - #6530

VS Extensibility Team Issues

  • VS SDK project support : #5899

C++ Team Issues

  • Find bug to link Internal C++ bug# 188588

WPF Project Compat Issues: (likely fixable by NuGet)

  • #5894 - PackageReference broken in WPF projects due to tmp_proj not importing Package-supplied build authoring

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

  • #5414 - Update NuGet.VisualStudio for PackageReference projects
  • #5971 Enable the build bootstrapping scenario which is supported by PC today. See configure.ps1 in NuGet repo. (dotnet install tool foo.bar -- may help solve this)

Work with package owners (see related item# #5960)

  • Analyze how many packages have install/uninstall.ps1 and analyze the impact this change would have.
  • Analyze contentfiles vs content packages - #5048
  • Analyze lib/foo.dll packages
  • #6623 Jquery/Bower + Alternative Package Manager Providers

Validation to improve ecosystem for new push/ingested packages

  • once we clean up, how do we keep it clean? Validation at pack/push/ingest time?

Package Reference experience issues

  • #5939 - Install/Uninstall operations from PMC do not refresh PM UI for legacy project system
  • dotnet/project-system#2129 - Improve PackageReference support in old project system - $(MyVersion) in version string.
  • #4778 Error experience with unavailable frameworks, PR.
  • #4989 - Documentation issue.
  • #4488 - PackageRef project opt-in technique

@nkolev92 nkolev92 changed the title Collector Task of Packages Config -> Package Reference migration related issues Collector Task of Packages Config -> Package Reference migration issues Sep 12, 2017

@nkolev92 nkolev92 changed the title Collector Task of Packages Config -> Package Reference migration issues Collector Task of Packages Config -> Package Reference migration tasks Sep 12, 2017

@rrelyea rrelyea changed the title Collector Task of Packages Config -> Package Reference migration tasks Packages Config -> Package Reference migration effort Sep 13, 2017

@AArnott

This comment has been minimized.

Copy link
Contributor

commented Sep 15, 2017

You might want to add #5894 to your list of issues to be fixed. Since NuGet contains WPF assets, this could bite you.

@nkolev92

This comment has been minimized.

Copy link
Member Author

commented Sep 15, 2017

Done.
Thanks @AArnott.

@rrelyea rrelyea added this to the 4.5 milestone Sep 21, 2017

@natidea

This comment has been minimized.

Copy link

commented Sep 29, 2017

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

@mungojam

This comment has been minimized.

Copy link

commented Oct 15, 2017

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

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2017

@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

This comment has been minimized.

Copy link

commented Oct 16, 2017

@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

This comment has been minimized.

Copy link

commented Oct 16, 2017

@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

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2017

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

This comment has been minimized.

Copy link

commented Oct 17, 2017

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

This comment has been minimized.

Copy link

commented Feb 12, 2018

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

@rohit21agrawal

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2018

@awesley #6285 is tracking that work and is included in the list in the OP.

@karann-msft karann-msft changed the title Packages Config -> Package Reference migration effort packages.config (PC) to PackageReference (PR) Upgrader Feb 21, 2018

@hyankov

This comment has been minimized.

Copy link

commented Mar 20, 2019

Work faster.

@rrelyea rrelyea removed this from the 5.1 milestone Apr 30, 2019

@rrelyea rrelyea added Epic and removed Type:Feature labels Apr 30, 2019

@camainc

This comment has been minimized.

Copy link

commented Jun 3, 2019

There is a VSIX extension that supposedly allows migration of ASP.net web projects. However it breaks when trying to migrate VB.Net projects. We are trying to migrate away from VB.Net completely but it takes time. We really would like to change from package.config to the package reference format. Is there some way to do this manually for now?

@rohit21agrawal

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

for the sole purpose of enabling and completing the migration, you can remove the following guids from your project file:

{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}
{349C5851-65DF-11DA-9384-00065B846F21}
{E24C65DC-7377-472B-9ABA-BC803B73C61A}

The presence of the above GUIDs causes NuGet to not show the Migrate dialog on right clicking the project.

NOTE: Might cause certain other functionality to stop working. Restore them after migration is complete.

@camainc

This comment has been minimized.

Copy link

commented Jun 4, 2019

THanks, that allowed me to do the migrations.

@hyankov

This comment has been minimized.

Copy link

commented Jun 4, 2019

Does the package manager work for that project, though, after this hack? Like, restore and update?

@camainc

This comment has been minimized.

Copy link

commented Jun 4, 2019

Seems to be.

@jemiller0

This comment has been minimized.

Copy link

commented Jul 31, 2019

When is Microsoft going to add support for PackageReference for ASP.NET Web Forms packages??? I suppose never, since they are throwing Web Forms under the bus. Thanks a lot Microsoft.

@hyankov

This comment has been minimized.

Copy link

commented Jul 31, 2019

I have successfully used the work-around described above and so far I haven't seen any issue. Makes me wonder why isn't it working by design for the web projects? What issue do I have with them that I am not yet seeing?

@CoolDadTx

This comment has been minimized.

Copy link

commented Aug 5, 2019

If you switch an existing project from package.config (PC) to package reference (PR) then you'll have no issues. In fact any project that supports PC supports PR as it is a built time featue. Your builds will work just fine.

The issue with PR is that it doesn't support all the scenarios that PC installs did and this is where things go wrong. Specifically PR doesn't support running install/uninstall scripts when a package is added/removed. Therefore any package that makes changes to a project during install (such as modifying the config file, adding project files, etc) don't work in PR. If you install a package using PR that has an install script then it adds the package but doesn't do anything else.

This is why ASP.NET apps aren't formally supported yet. For many ASP.NET-specific packages they need to update the system.web or system.webserver sections of the config file and/or add files to the project that can then be compiled into code. That simply doesn't work in PR. Since it can be very hard to know what a package does without opening it up and looking at the install script the current recommendation has been to stick with PC. In some cases PR has been updated to support some features but because of transient package resolution this doesn't work unless you explicitly include a package. That defeats one of the benefits of PR.

So yes you can use PR with any project that supports PC but you have to be aware of any install scripts that won't run and manually make the changes yourself. If you have the expertise to do that then go for it. However be aware that future updates to packages may change things so each time you update a package you need to review the scripts. This adds an extra layer of work for little gain in most cases. Hence most people stick with PC. Note that you can mix this in the solution. For example at my company we use PR for everything except ASP.NET and WCF projects. These projects stick with PC while the rest of the code in the solution use PR.

@binki

This comment has been minimized.

Copy link

commented Aug 6, 2019

If it were possible to generate web.config at build-time with MSBuild targets hooks for incrementally building it, which people would only do if MS lead the effort by changing the templates to do so, it would be possible for ASP.NET-style projects to switch to PR in a clean way.

@CoolDadTx

This comment has been minimized.

Copy link

commented Aug 6, 2019

@binki, you can already do that. MSBuild tasks can generate/update config files at build time. That is how the AutoGenerateBindingRedirects property works. Packages can integrate into the build process with custom .props and .targets files. This is how the more complex packages work. As such a package can integrate into the build at any point in the process just like anything else.

What cannot be done via MSBuild is anything that is needed at design time such as generating user-editable files. Adding files to a project at build time is easily handled in MSBuild and, once again, already done by the SDK format when generating assembly info files. But design time (e.g. before the build) changes cannot be done.

@binki

This comment has been minimized.

Copy link

commented Aug 6, 2019

@CoolDadTx I understand that. What I’m saying is that web.config is considered a “user-editable file”. It doesn’t have to be if the ASP.NET tooling were updated to make it a generated file. Of course, that would be very difficult to do and would break a lot of things which assume it is user-editable.

@CoolDadTx

This comment has been minimized.

Copy link

commented Aug 6, 2019

@binki, web.config has to be editable because devs do edit it. It is no different than an app.config in every other project type. You said making this auto-generate would solve the PR issue for ASP.NET. I fail to see how this feature (which already works) solves the problem with migrating ASP.NET apps to PR. Config files are easy to change at build time. This isn't what makes updating ASP.NET apps hard. It's the other stuff like updating bundling rules when adding content files or modifying the config file when there are user-settable options inside it (e.g. identity providers).

@hyankov

This comment has been minimized.

Copy link

commented Aug 6, 2019

Can't merge conflicts be detected and the dev prompted to solve them? Worst case, can't the user-modified sections which the PR changes be commented out (rather than wiped out), so the dev can see them after adding a package?

@CoolDadTx

This comment has been minimized.

Copy link

commented Aug 6, 2019

@hyankov Who is your comment directed to? I don't understand what you mean by a merge conflict as that would only apply to a PR. Since packages cannot modify configs at install time anymore (because there is no install) then the only file changed when adding a package is the project file (to add the package reference element).

@jemiller0

This comment has been minimized.

Copy link

commented Aug 6, 2019

Is there a right-click menu to convert a ASP.NET Web Forms application project to use PackageReference yet? If not, the problem is not fixed. Which isn't a surprise because it is clear that Microsoft could give a flying you know what about Web Forms. They already threw it under the bus with .NET Core. Instead, they want you to port your code to unreleased vaporware.

@binki

This comment has been minimized.

Copy link

commented Aug 7, 2019

@CoolDadTx

@binki, web.config has to be editable because devs do edit it. It is no different than an app.config in every other project type. You said making this auto-generate would solve the PR issue for ASP.NET. I fail to see how this feature (which already works) solves the problem with migrating ASP.NET apps to PR. Config files are easy to change at build time. This isn't what makes updating ASP.NET apps hard. It's the other stuff like updating bundling rules when adding content files or modifying the config file when there are user-settable options inside it (e.g. identity providers).

web.config doesn’t have to be editable. It could be built from other files, some which are user-edited and others which are transformations applied incrementally by targets from <PackageReference/>. What you’re saying is something like “web.config has to be editable because it currently is editable”. What I’m saying is that the <PackageReference/> approach would be a lot easier if Visual Studio were willing/able to run targets to generate web.config, default to web.config being in .gitignore, have a user-friendly way to store the edits that you would need to make to web.config in different files which automatically get applied to the web.config as it is generated, and public well-specified MSBuild targets that would collect information for generating web.config and run when it needs to be generated (such as after the user edits the user-editable files).

The same approaches can be extended to bundling rules. IMO, the system is broken if a file is both autogenerated and requires post-generation user edits before it works for them.

Making web.config built would make a lot of projects’ git repos a ton cleaner because they would be able to have less generated code in them. This gets us closer to the ideal of 0 boilerplate. It would allow projects’ git repos to contain exactly the code that is specific to that project and omit things which are needed for every project like FSharp assembly binding redirects (for non-ASP.NET projects, those are already autogenerated at build-time because app.config is transformed while it is being copied to bin. In ASP.NET projects, the web.config in the root of the repository has to be post-transform because that is where IISExpress looks when serving the project—which is one of the few reasons why VS still must implement the functionality where it can edit app.config/web.config for you if you right-click the error message about missing binding redirects).

@CoolDadTx

This comment has been minimized.

Copy link

commented Aug 7, 2019

@binki, while I agree that separating web.config into separate pieces (which you can already do by the way) would make it easier to autogenerate stuff. This doesn't really have anything to do with PR in ASP.NET projects. Your thoughts sound more like a feature request and therefore should probably be posted in the ASP.NET team. Breaking up web.config would not solve the PR issue with ASP.NET projects. It would simply lessen the things that need to be done to get it to work.

@binki

This comment has been minimized.

Copy link

commented Aug 7, 2019

@CoolDadTx

This doesn't really have anything to do with PR in ASP.NET projects.

I’m convinced it is a big part of it. The installation/uninstallation scripts that packages.config run primarily do things like edit app.config/web.config. Bundled assets are already solved. However, there is no way for a <PackageReference/> NuGet to edit app.config upon installation—which is a good thing. It enforces less boilerplate. .csproj files no longer generate a bunch of churn when updating or adding/removing packages to a project.

separating web.config into separate pieces (which you can already do by the way)

Where are the docs on this? I don’t expect NuGet package authors to write targets to work with one person’s way of autogenerating configuration files because there is not yet a right way to do so. Microsoft needs to make a guide on how to write a NuGet which can be installed into an ASP.NET project as <PackageReference/> and still work without requiring the user to read and manually do the steps outlined in install.ps1.

@CoolDadTx

This comment has been minimized.

Copy link

commented Aug 7, 2019

@binki, I'm afraid we'll have to disagree on this. Separating out the config file introduces way more problems then it solves (especially at runtime where multiple files are now needed) just to get PR to work and there are still cases where PR won't work with a package. Since ASP.NET is deprecated in lieu of ASP.NET Core (which focuses on programmatic configuration and JSON instead of web.config) I doubt MS would take the hit to implement such a feature vs just forcing everyone down the ASP.NET Core route.

Where are the docs on this?

configSource is what handles this. Some larger apps use it to separate their configurations out. Personally I find it harder to work with but I can see the draw for really large apps.

@binki

This comment has been minimized.

Copy link

commented Aug 7, 2019

Where are the docs on this?

configSource is what handles this. Some larger apps use it to separate their configurations out. Personally I find it harder to work with but I can see the draw for really large apps.

I know what configSource is and have looked at it. But what you linked is not a protocol for writing a <PackageReference/>-compatible NuGet package intended for consumption by ASP.NET.

Since ASP.NET is deprecated in lieu of ASP.NET Core

OK, so I am reading that is: MS does not intend to fix this issue. Thanks for the update!

@CoolDadTx

This comment has been minimized.

Copy link

commented Aug 7, 2019

OK, so I am reading that is: MS does not intend to fix this issue. Thanks for the update!

I don't work for MS so I cannot answer the official word on the matter but if you read all the information coming out, including .NET 5 then the writing is on the wall.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.