Skip to content
This repository has been archived by the owner on Dec 12, 2020. It is now read-only.

Create SDK package #113

Closed
amis92 opened this issue Jan 22, 2019 · 8 comments · Fixed by #204
Closed

Create SDK package #113

amis92 opened this issue Jan 22, 2019 · 8 comments · Fixed by #204
Assignees
Milestone

Comments

@amis92
Copy link
Collaborator

amis92 commented Jan 22, 2019

Summary

The SDK package would be used instead of the Microsoft.NET.Sdk Sdk by default, or in addition to it:

<Project Sdk="Microsoft.NET.Sdk">
  <Sdk Name="CodeGeneration.Roslyn.Sdk.Generators" />
  ...
</Project>

Inspired by PR #108 by @jarrodldavis

Problems solved

  • Packaging generator as NuGet for redistribution

Features

A project built with this SDK would:

And possibly other needed additions.

Additional ideas

It's possible that there would be additional SDKs (just ideas):

  • CG.R.Sdk.SdkPackage that'd help create Sdk package for a custom generator
  • CG.R.Sdk.Test that'd help in unit testing the custom generator
  • CG.R.Sdk which would replace BuildTime package - this would probably "fix" the version conflict for customers using generators that depend on different (but compatible) CG.R versions, as the given SDK has to have only one version in a solution (and project), declared by the customer himself in global.json.
@AArnott
Copy link
Owner

AArnott commented Jan 23, 2019

provide preset TargetFramework

No, I don't think so. VS has an implementation detail that depends on that property being set within the project file itself. Otherwise it'll load the project with the wrong project system in some cases.

@amis92
Copy link
Collaborator Author

amis92 commented Jan 23, 2019

Ahh yes, hacks and details. Besides TargetFramework, I think that DotNetCliToolReference also has weird behavior, as it works when included in project file or Directory.Build.props, and yet it doesn't when being added as part of build/<pkg>.props - I guess the restore won't run twice.

Will keep that in mind. Maybe a check with warning then. So, other than that, any suggestions? I'm very much not happy with names. All of them sound weird. Especially the possible CG.R.Sdk.SdkPackage.

@jarrodldavis
Copy link

jarrodldavis commented Jan 25, 2019

I think that DotNetCliToolReference also has weird behavior, as it works when included in project file or Directory.Build.props, and yet it doesn't when being added as part of build/<pkg>.props - I guess the restore won't run twice.

I think I remember running into that issue when initially trying to package up a generator and part of what led to my (admittedly convoluted) double-SDK solution in my pull request (#108). DotNetCliToolReference seems to work fine when included as part of an SDK.

Edit: yes I remember after a lot of spelunking through NuGet issues finding stuff like NuGet/Home#4013 and NuGet/Home#3604 where the NuGet team specifically prevented recursive package restores to prevent various issues.

@AArnott
Copy link
Owner

AArnott commented Jan 26, 2019

No other suggestions at this point.

I'm very much not happy with names. All of them sound weird. Especially the possible CG.R.Sdk.SdkPackage.

How about CG.R.Generator.Sdk?

@amis92 amis92 added this to the Backlog milestone Jun 30, 2019
amis92 added a commit that referenced this issue Dec 13, 2019
@amis92 amis92 modified the milestones: Backlog, 0.7 Dec 13, 2019
amis92 added a commit that referenced this issue Jan 5, 2020
* Engine uses `McMaster.NETCore.Plugins` package to resolve assemblies and dependencies, enabling generators to reference additional dependencies! 🎉
* Tasks deprecate `GeneratorAssemblySearchPaths` usage
* Tasks use `CodeGenerationRoslynPlugin` ItemGroup instead, which contains
  concrete assembly paths (instead of containing folder paths, as was previously)
* Engine targets `netcoreapp2.0` to reference McMaster package
* Tests use Amadevus.RecordGenerator NuGet generator for
  back-compat checks
* Tests.Generators use Bogus NuGet for NuGet dependency resolution check
* `CodeGeneration.Roslyn.Plugin.Sdk` MSBuild project Sdk created, to help build and package plugins correctly.
* Migrate to use VS2019/.NET Core SDK v3.1
* Rewritten Readme with a simpler demo and more advanced scenarios

* Change input assemblies check

Now the list of plugin assemblies is always read
from response file (plugin list),
the last modified time is calculated using those assemblies,
and the .AssemblyList.txt file is not created.

Also separated reading the results into another target,
and set Inputs and Outputs so that the MSBuild can fully skip
executing the target that invokes CLI tool.

* Rename BuildTime targets private items

* add comment to BuildTime targets

* feature: create Plugin.Sdk project

initial idea in #113

* fix GenerateCodeFromAttributesCore condition

_CodeGenToolVersionExitCode was compared to zero
via != instead of ==

* fix CGR1002 warning in Tests

* refactor and cleanup BuildTime files

* refactor Tests project file

* docs: readme demo and more for new Plugins.Sdk

* don't prefer shared types, use explicit list

this will allow different plugins to have conflicting dependencies

* refactor: rename dictionary to cachedPlugins

* fix: Use OutputItemType in Sample

* docs: Add changelog for plugins PR
@amis92
Copy link
Collaborator Author

amis92 commented Mar 25, 2020

A rewrite of the Plugin.Sdk is happening at #198.

@AArnott
Copy link
Owner

AArnott commented Mar 28, 2020

The SDK package would be used instead of the Microsoft.NET.Sdk Sdk by default, or in addition to it:

Please make it in addition to it. There can only be one top-level SDK, so if too many cool features want to be "the one", a project can't use more than one of them.

@amis92
Copy link
Collaborator Author

amis92 commented Mar 30, 2020

It's done as such. I'm also going to make it usable as a normal PackageReference as well, because it seems the MsbuildSdks still have poor support across tooling (looking at you, Omnisharp).

When used as a normal package, all props and targets will be the same, except there'll be no implicit PackageReference of CodeGeneration.Roslyn.

@amis92 amis92 linked a pull request Mar 31, 2020 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants