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

Provide an obsolete warning when DotNetCliToolReference is used to packages that are now built-in #2911

Closed
natemcmaster opened this issue Feb 28, 2018 · 13 comments
Assignees
Labels
Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one External This is an issue in a component not contained in this repository. It is open for tracking purposes.

Comments

@natemcmaster
Copy link
Contributor

natemcmaster commented Feb 28, 2018

Follow-up to aspnet/DotNetTools#399.

DotNetCliToolReference to dotnet-watch, dotnet-user-secrets, and others aren't needed and won't work anymore. However, when users upgrade their project, dotnet restore will continue to download and install the tools package silently. We should look into ways for providing some kind of warning or output on restore that indicates the user that the reference is no longer required to acquire those tools.

@poke
Copy link
Contributor

poke commented Feb 28, 2018

Just wondering: Will those tool references actually stop working or is it just that they are being replaced by a new tool system, so you just cannot retrieve new versions of those tools?

Or asked differently: Will other CLI tools using DotNetCliToolReference also stop working, requiring them to migrate to the new system?

@natemcmaster
Copy link
Contributor Author

Will those tool references actually stop working or is it just that they are being replaced by a new tool system

The tools will stop working. Because dotnet-watch et. al. are bundled, those will override the version that use to be delivered via DotNetCliToolReference.

Will other CLI tools using DotNetCliToolReference also stop working, requiring them to migrate to the new system?

No. DotNetCliToolReference will continue to work. We are just no longer using this mechanism to deliver the following packages:

  • Microsoft.DotNet.Watcher.Tools (aka dotnet-watch)
  • Microsoft.Extensions.SecretManager.Tools (aka dotnet-user-secrets)
  • Microsoft.Extensions.Caching.SqlConfig.Tools (aka dotnet-sql-cache)

There are other packages from ASP.NET Core that are not built-in, such as EF tools.

@Eilon
Copy link
Member

Eilon commented Feb 28, 2018

@natemcmaster where would such a warning go?

@natemcmaster
Copy link
Contributor Author

Three options:

  1. Wait for NuGet to support obsoleting a package Enable server side deprecation of obsolete/legacy NuGet Packages NuGet/Home#2867
  2. Add something to Microsoft.NET.Sdk so we can issue a warning on restore
  3. Add a bit of code into new versions of dotnet watch and user-secrets that detect if the project they operate on still has a DotNetCliToolReference to an old version of the same tool

I'd prefer (1) but likely won't happen soon. (2) is the next best, but I think we're unlikely to convince the SDK guys this is important enough. (3) is probably the most viable, but doesn't work for dotnet-sql-cache which doesn't operate on the project.

@poke
Copy link
Contributor

poke commented Feb 28, 2018

The tools will stop working. Because dotnet-watch et. al. are bundled, those will override the version that use to be delivered via DotNetCliToolReference.

But if the bundled versions override the one from the DotNetCliToolReference, and the bundled tools work the same way as the old ones, then there’s no actual harm in just keeping the tool references in the projects, right?

@natemcmaster
Copy link
Contributor Author

That's a good point. "The tools will stop working" is vague. What I meant was that resolving "dotnet watch" to the dotnet-watch.dll as specified in DotNetCliToolReference doesn't work. You are automatically lifted to the bundled version. For the most part, these tools have preserved the same command line syntax that existed in all stable releases, so I expect this won't often be an issue, so no harm done from that perspective. The harm done, however, is that the CLI is no longer honoring the specification in DotNetCliToolReference. That's the piece I want to warn users about with some kind of message.

@Eilon
Copy link
Member

Eilon commented Feb 28, 2018

Can we get an issue logged for (2)? That would be helpful for all tools in the world that become global tools, no? We can only change the code in our own tools here.

For (3), should we move this issue to DotNetTools, plus have a clone of the issue for any other repo that has a formerly-local-and-now-global tool?

@natemcmaster
Copy link
Contributor Author

(2) - dotnet/sdk#2014. The scope of this warning should just be limited to DotNetCliToolReference to our tools that are now built-in commands. AFAIK there are no plans to drop DotNetCliToolReference completely from the CLI/SDK.

(3) - if dotnet/sdk#2014 doesn't happen, we can do a less-optimal, but hopefully still helpful message in dotnet-watch/user-secrets. Let's wait to see what happens on dotnet/sdk#2014

@Eilon
Copy link
Member

Eilon commented Mar 1, 2018

Ok

@natemcmaster
Copy link
Contributor Author

Ping @muratg @DamianEdwards - can we assign a milestone and priority on this one?

@muratg
Copy link
Contributor

muratg commented Mar 15, 2018

@natemcmaster If the SDK feature request is implemented, we won't need to change the tools themselves, right?

@natemcmaster
Copy link
Contributor Author

Correct. It sounds like the SDK team is open to adding the obsolete message to Microsoft.NET.Sdk, but we should make a PR to added it.

@natemcmaster natemcmaster self-assigned this Mar 22, 2018
@natemcmaster natemcmaster added Done This issue has been fixed External This is an issue in a component not contained in this repository. It is open for tracking purposes. enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Mar 22, 2018
@natemcmaster
Copy link
Contributor Author

This was done in dotnet/sdk#2064 and should be in the SDK for preview2.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one External This is an issue in a component not contained in this repository. It is open for tracking purposes.
Projects
None yet
Development

No branches or pull requests

4 participants