Add Spectre.Console latest (0.48.0) to Source Build #244
Conversation
| + <!-- <ItemGroup Label="Package References"> | ||
| <PackageReference Include="MinVer" PrivateAssets="All" Version="4.2.0" /> | ||
| <PackageReference Include="Microsoft.SourceLink.GitHub" PrivateAssets="All" Version="1.1.1" /> | ||
| <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.435"> | ||
| @@ -40,5 +40,5 @@ | ||
| <PackageReference Include="Roslynator.Analyzers" Version="4.1.2"> | ||
| <PrivateAssets>All</PrivateAssets> | ||
| </PackageReference> | ||
| - </ItemGroup> | ||
| + </ItemGroup> --> |
There was a problem hiding this comment.
We don't need these analyzers or build-time dependencies in the source-build build of Spectre.Console because they mainly exist to check logic and set versions.
We don't need to check logic here (we're assuming upstream is generally good), and we know the version of Spectre we're building (it's set in the repro-projects wrapper project) so we can skip all of these dependencies outright.
There was a problem hiding this comment.
This makes sense, I am wondering if it would be feasible to introduce a conditional based on a property that source-build could then set? If so this change could be pushed upstream. The advantage of this is it would eliminate the need for this patch and the potential maintenance burden of it.
|
|
||
| <Target Name="RepoBuild"> | ||
| <PropertyGroup> | ||
| <BuildCommandArgs>$(ProjectDirectory)/src/Spectre.Console/Spectre.Console.csproj</BuildCommandArgs> |
There was a problem hiding this comment.
We only take the main project - the Spectre analyzers aren't required, and we're not currently planning on using any functionality from the minor extensions packages that Spectre provides.
| + <!-- <ItemGroup Label="Package References"> | ||
| <PackageReference Include="MinVer" PrivateAssets="All" Version="4.2.0" /> | ||
| <PackageReference Include="Microsoft.SourceLink.GitHub" PrivateAssets="All" Version="1.1.1" /> | ||
| <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.435"> | ||
| @@ -40,5 +40,5 @@ | ||
| <PackageReference Include="Roslynator.Analyzers" Version="4.1.2"> | ||
| <PrivateAssets>All</PrivateAssets> | ||
| </PackageReference> | ||
| - </ItemGroup> | ||
| + </ItemGroup> --> |
There was a problem hiding this comment.
This makes sense, I am wondering if it would be feasible to introduce a conditional based on a property that source-build could then set? If so this change could be pushed upstream. The advantage of this is it would eliminate the need for this patch and the potential maintenance burden of it.
|
I could definitely check on that @MichaelSimons - I'll open a discussion on the Spectre repo and link it here for ease of discovery. |
|
I ran a license scan on the Spectre.Console repo and it all looks good except for one reference that I want to get clarification on. The file at https://github.com/spectreconsole/spectre.console/blob/main/examples/Console/Canvas/Mandelbrot.cs uses the GNU Free Documentation License 1.2. @omajid - Can you weigh in on that license? It's not explicitly listed as approved by OSI. |
If not permitted, one possible option would be to cloak the examples in the VMR mappings for source-build-externals. |
|
GNU Free Documentation License 1.2 should be a valid open source license given that it's from the GNU project. Specifically, Fedora allows this license for content (https://docs.fedoraproject.org/en-US/legal/allowed-licenses/#_allowed_documentation_licenses), and I guess I can argue that an example based on that content is still content? I will get more guidance from Fedora. I am a bit more worried about Spectre.Console's README referring to https://github.com/SixLabors/ImageSharp/blob/main/LICENSE in README. That's filtered out because we only use the Spectre.Console project and not the Spectre.Console.ImageSharp project, right? |
|
@omajid correct - we only plan to use the 'core' library, and we explicitly do not expect to use any encumbered libraries or portions of the library. |
|
I got more guidance from Fedora about the example code under the GFDL license: https://gitlab.com/fedora/legal/fedora-license-data/-/issues/432 Turns out it is not allowed :( |
|
I logged spectreconsole/spectre.console#1424. We'll have to cloak the example from the VMR for now. |
|
@mthalman for my own info, is there any doc on how we do cloaking? I did some searching for general strategies for masking directories in submodules, but I didn't really get anywhere. |
|
I don't know about any doc. But it's pretty straightforward. Here's an example of aspnetcore files being cloaked: https://github.com/dotnet/installer/blob/0cebb584bc76764eada385dc32bfcbd41365f098/src/VirtualMonoRepo/source-mappings.json#L73-L79. |
|
"Author" here 👋 I was under the impression that it was compatible since it's example code. Will take a look at this later tonight and remove it. |
|
It's been removed, so it should be safe to update the sub-module reference now. |
MSBuild and the SDK would like to use some of the interactivity and console layout features from Spectre without reinventing the wheel, and a prereq for that is Spectre.Console being part of source build.
This adds the latest package (0.48.0) tags to source build, but specifically just the primary project/package for the Spectre project. This project contains the vast majority of the UI/layout functions and should serve all of our needs.