gui-cs / TerminalGuiDesigner

Forms Designer for Terminal.Gui (aka gui.cs)
MIT License
423 stars 28 forks source link

Add net8.0 configurations and conditional compilation #255

Closed dodexahedron closed 9 months ago

dodexahedron commented 9 months ago

Added build configurations for .net 8.0. If one of the two -net8.0 configurations is selected, framework target and relevant dependencies are switched to 8.0. Otherwise, the old values are used.

Addresses #254

dodexahedron commented 9 months ago

There are, of course, other ways to achieve multi-targeting.

I chose the method I did because there was an assembly with its version in its namespace name (the Basic.Reference.Assemblies.Net70 package), which needs to be swapped out and conditional compilation applied for the one line where it is used, and this was just a simple way to do it.

Another option I just realized is available would be to use a global using to alias the reference to Net70 or Net80 as something else (like NetReferenceAssembly or something), so that conditional compilation is entirely handled by msbuild and preprocessor directives in the C# aren't necessary. But that's a newer language/framework feature, so that may not be desirable, for reasons of backward-compatibility for users building on or targeting pre-7.0 .net SDKs.

That would look something like this in the .csproj, replacing the current Choose block:

<Choose>
    <When Condition="'$(Configuration)'=='Debug-net8.0' Or '$(Configuration)'=='Release-net8.0'">
      <PropertyGroup>
        <TargetFramework>net8.0</TargetFramework>
        <DefineConstants>$(DefineConstants);Building_For_Dotnet_8</DefineConstants>
      </PropertyGroup>
      <ItemGroup>
        <PackageReference Include="Basic.Reference.Assemblies.Net80" Version="1.4.5" />
        <PackageReference Include="System.CodeDom" Version="8.0.0" />
        <Using Include="Basic.Reference.Assemblies.Net80">
          <Alias>NetReferenceAssemblies</Alias>
        </Using>
      </ItemGroup>
    </When>
    <Otherwise>
      <PropertyGroup>
        <TargetFramework>net7.0</TargetFramework>
      </PropertyGroup>
      <ItemGroup>
        <PackageReference Include="Basic.Reference.Assemblies.Net70" Version="1.4.5" />
        <PackageReference Include="System.CodeDom" Version="7.0.0" />
        <Using Include="Basic.Reference.Assemblies.Net70">
          <Alias>NetReferenceAssemblies</Alias>
        </Using>
      </ItemGroup>
    </Otherwise>
  </Choose>

And then the code on line 150 of CodeToView.cs (before this PR) would change to:

var references = new List<MetadataReference>(NetReferenceAssemblies.References.All);

Instead of the conditional preprocessor directives I added for this PR.

dodexahedron commented 9 months ago

Note this isn't a problem for Terminal.Gui itself - just this tool, because dotnet tools are pre-packaged framework-dependent assemblies, so the framework version rules, unfortunately, are strict, for them.

tznind commented 9 months ago

Thanks! I will need to update build.yml so that it publishes to nuget correctly.

Do you know if there is any way to snip a nupkg that includes multiple framework versions (e.g. 6,7,8)?

I have done a bit of internet searching but am drawing a blank.

I can update github action to dotnet publish -c 'Release-net8.0' but it would be better if I could also support 7 and 6

dodexahedron commented 9 months ago

I'm not entirely sure, but maybe using a <TargetFrameworks>net7.0,net8.0</TargetFrameworks> in a PropertyGroup in the csproj? 🤔

I was wondering that myself and would have done it if I knew for sure.

dodexahedron commented 9 months ago

If your build system can make multiple runs, you could always just have it run once for each target framework.

I think 6 should work with everything I did, since global usings came about in 6.

You'd just need to add another Choose block for any references that need to be to 6-compatible packages.

tznind commented 9 months ago

If I use TargetFrameworks it isn't going to do the conditional stuff.

But maybe I could reference all the target frameworks and use Environment.Version at the saving point ViewToCode to load the right one and just reference them all. Or maybe the reference assemblies are backwards compatible or something... I will have to investigate

tznind commented 9 months ago

Looks like <TargetFrameworks> will do the trick.

I have targetted only 8.0.0 reference assemblies... hopefully they will run under 7.0 too.

It seems to compile ok.

image

I've tagged the build with a pre-release version number -rc1 so can test that if it ships ok through CI.

Thanks for creating this PR, I have learned a cool new trick with the csproj I didn't know but if this works I think it will be simpler <3 and if not I can change back to this.

I will have to apply the same change to the v2 branch which is for supporting version 2 of Terminal.Gui which has substantial differences.

tznind commented 9 months ago

Closing for now as https://www.nuget.org/packages/TerminalGuiDesigner/1.1.0-rc1 looks like it will work without conditional compilation.

dodexahedron commented 9 months ago

Looks like <TargetFrameworks> will do the trick.

I have targetted only 8.0.0 reference assemblies... hopefully they will run under 7.0 too.

It seems to compile ok.

image

I've tagged the build with a pre-release version number -rc1 so can test that if it ships ok through CI.

Great!

Just for follow-up:

If you don't specify a version in a PackageReference element, it'll pull the latest version it supports (like for the System.CodeDom package), if the package name is the same. But, I think that particular package is backward compatible with 7, anyway, because VS was even offering the upgrade for it when targeting .net7, and it did compile.

You can also use the conditional stuff along with the TargetFrameworks element, even without additional build configurations, if you use a different property on the Condition, such as NET8_0 or other symbols defined by msbuild. But, of course, that's only really necessary if one of the 8.0-versioned dependencies is incompatible with .net7.0 at compile- or run-time. I only used build configurations because it was a simple way to do it with a 100% guarantee the symbol would be declared.

Thanks for creating this PR, I have learned a cool new trick with the csproj I didn't know but if this works I think it will be simpler <3 and if not I can change back to this.

Are you referring to the Choose element? Yeah, when I learned about it a while back I immediately loved it.

It's pretty handy for making much cleaner groupings of conditional elements, for sure. Otherwise, you end up with a bunch of PropertyGroups, etc., which EACH have a Condition attribute, which is annoying if you ever have to change the condition, especially since VS is quite happy to just make extra ones willy-nilly. However, the Condition attribute is valid on almost every element in an msbuild file (including csproj), so you can target things as granularly as your heart desires. 😅

dodexahedron commented 9 months ago

Oh another thing I learned around the same time I learned about Choose....

Did you know the expressions in the Condition attribute support more than just simple logical operators like that? You can even call things like EndsWith on a string, or Contains on a collection, for example, as if it were sorta-C#. It's super flexible, but the documentation is scattered at best.