search

AArnott / CodeGeneration.Roslyn

Assists in performing Roslyn-based code generation during a build.
Microsoft Public License
408 stars 60 forks source link

[RFC] dotnet-codegen tool published as Dotnet Tool (Local/Global) #155

Closed amis92 closed 4 years ago

amis92 commented 5 years ago

Since the DotNetCliToolReferences are being deprecated and the suggested replacement is Local Tools, I propose to follow the suggestion.

That is, move away from the current usage scenario and publish the dotnet-codegen package as the DotnetTool format.

The timeframe is probably to be around SDK v3 final release. However we can consider an earlier switch, because it's usable as-is, although it currently needs to be installed as a Global Tool instead of a per-repo/project Local Tool (that's the scenario enabled by SDK v3).

Thoughts/opinions?

Antaris commented 4 years ago

A single global tool makes sense, would there be any issues with it getting out of sync with the CodeGeneration.Roslyn* packages?

amis92 commented 4 years ago

Yes, a global tool wouldn't resolve any problems like that.

Making it a global/local tool is only for making it target netcoreapp3.0+ possible. No other gains here.

AArnott commented 4 years ago

The ideal to strive for IMO is that clone, restore, build works for a consumer. If that breaks because of some extra steps that must be injected (like a dotnet tool install command), then it'll be a hard sell for me.

LokiMidgard commented 4 years ago

Couldn't a local tool installed with a task while building? So clone, restore, build works.

AArnott commented 4 years ago

Several potential problems with that, @LokiMidgard:

  1. dotnet.exe might not be installed
  2. Assuming it's installed, where do you find it? What if there are multiple versions?
  3. Assuming you can find it, how do you detect if the tool is already installed?
  4. Assuming it's appropriate to attempt to install the tool, it incurs a network dependency during build.

The network dependency is the most concerning. It's so concerning in fact, that that's the main reason why restore and build are separate steps for almost all toolsets (that I'm familiar with, anyway). Having build require a network dependency to do something that should have happened during restore is pretty hacky, IMO. And something I'd like to avoid.

I'm not even sure where we are now, so maybe an extra step to install a tool beyond what restore does already is required for users of this project.

amis92 commented 4 years ago

I'm not even sure where we are now, so maybe an extra step to install a tool beyond what restore does already is required for users of this project.

It kind of is, not direct but still: DotNetCliToolReference we use right now to "install" the tool requires:

Switching dotnet-codegen to be a local tool as specified in https://github.com/dotnet/cli/issues/10288 yields the following changes:

If, at some point, we need to target netcoreapp3.0, there'll be no alternative, really. Right now we can just keep it as it is - but quite possibly in future we'll need to make the jump anyway.

amis92 commented 4 years ago

Note for self: This will be obsolete when #198 is merged; it'll solve most of what this proposal could fix, and in some ways it's better.