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

.NET Standard 2.0 build for Terminal.Gui #3445

Open
halvarsson opened this issue Apr 30, 2024 · 14 comments
Open

.NET Standard 2.0 build for Terminal.Gui #3445

halvarsson opened this issue Apr 30, 2024 · 14 comments

Comments

@halvarsson
Copy link

Is your feature request related to a problem? Please describe.
Since 1.10.1 Terminal.Gui is only build for .NET Standard 2.1, and this caused us to get locked on the previous version while running Terminal.Gui from a Powershell module that works in both Windows Powershell & in Powershell. We are doing this today in psedit

I have recently upgraded the project to run with 1.10.0, but unfortunately there seem to be some new issues that are resolved in later versions

Describe the solution you'd like
It would be nice if the build could also target .NET Standard 2.0, in addition to the .NET Standard 2.1

Describe alternatives you've considered
I have tried building Terminal.Gui under .NET Standard 2.0, and there seem to be no issues running this as far as I can tell, and it works fine on psedit with latest release of Terminal.Gui

Another option would of course be for us to produce two separate builds, one targetting .NET Framework to work on Windows Powershell, and another targetting .NET Core that would work in Powershell, or I could build it myself and just commit this to the repo, but I would like to avoid either of these solution if possible

Additional context
I tried reading from the various PRs/commits for changing to only build on 2.1, but I was unable to find the exact reasoning behind dropping the support for 2.0

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 2, 2024

Could you do a git blame and find when the TFM changed? I'd be happy to go from there and try to determine why that TFM, specifically, was selected. If there was a specific reason, it's possible there may be a polyfill to work around that so it can be put back to 2.0 for guaranteed .Net Framework compatibility.

@halvarsson
Copy link
Author

halvarsson commented May 2, 2024

That would be great

This seems to be the commit where it was changed, and it was part of commits where project was upgraded from .NET6.0 to .NET7.0 and .NET Standard 2.0 to 2.1 a242af5

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 3, 2024

It's always frustrated me that netstandard 2.1 is 2.1 and not a full version number higher, especially since it was for Mono, not .net Framework, and 2.0 -> 2.1 just... Really does not get across how you're about to walk into a minefield if you need to support .net Framework.

I am not certain if this will fix it at your stage of the build, but it might, depending on exactly why it hates the newer versions. If you can, please try this if you can without too much difficulty: Build against a version that you currently can't use because of netstandard2.1, but add this design-time-only polyfill reference to your csproj and let me know if it does work:

        <PackageReference Include="Meziantou.Polyfill" Version="1.0.38" PrivateAssets="all" />

It won't add any additional runtime dependencies - just adds the missing stuff, if possible to do so, so that it can compile. I'm actually using it for Roslyn source generators in Terminal.Gui V2, since Visual Studio is a .net Framework host, so the generators themselves have to be netstandard2.0 even if they write code for .net 8 and C# 12. 😒 As long as it's not a binary behavior incompatibility, the polyfill adds enough that the compiled IL is useable in netstandard 2.0. It's the entire purpose of that project, in fact. :)

If it's just barfing at it because the TFM doesn't match, and refusing to go from there, well then that's a different story and we'll have to make it happy from here if we can.

There are of course other alternatives, but probably not any that are particularly palatable (submodules and stuff like that, which...yuck). I'm hoping that the polyfill will do what you need but, if not, my next step would be for us to try to use it while targeting netstandard2.0. If that also doesn't work... Then the fun begins. 😅

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 3, 2024

Oh wait. Before all that...

Which version of .net Framework are you targeting?

We also have the 472 TFM in there explicitly, so it should work if you're at least targeting that.

But also (and this has been part of considerations we've made in the past for target frameworks), are you intending to support 4.6.2, specifically? That hasn't been the default since Win 10 1607 and that's end of support except for extended support customers, as far as I can determine. 🤔 And 4.6.1 is way past end of support.

That would be the kind of situation where I, personally (ie this is me and has nothing to do with this project), would draw the line for my users and tell them they can either remain on the latest previous version that works for them or upgrade to something less than 8 years old and in mainstream support if they want to use newer versions of my stuff. That includes that they can stay on their ancient windows version, but just have to update .NET Framework, too.

But, aside from my personal view, there's also this: https://learn.microsoft.com/en-us/dotnet/core/compatibility/core-libraries/7.0/old-framework-support

That suggests that, even with our net7.0 target, we may be able to drop to netstandard 2.0 and framework 4.6.2 anyway, without binary incompatibility, at least, which I'll try out this weekend, unless the polyfill solves the problem by itself. (🤞)

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 3, 2024

Also, for your modules, if you ever need PSCore features, PSCore is on every supported version of Windows, today. If you launch pwsh before doing anything else, you'll have a guarantee you're in PSCore.

And, even if you're supporting clear back to Windows 10 1607 with .net framework 4.6.2, that version of windows also has PSCore 7.2 installed by default, so you can depend on it being there.

PSCore is required for cross-platform support, as well, anyway.

And if PSCore is there, .net 6.0 or higher is also there, which the compatibility article for 7.0 above says should work for ya. In particular, this line:

.NET 7 core libraries packages are supported for use in projects targeting .NET Framework 4.6.2 and later, .NET 6 and later, or .NET Standard 2.0 or later.

So, unless you're targeting something older explicitly, it looks like it should work, to me. 🤔

@halvarsson
Copy link
Author

halvarsson commented May 3, 2024

I am only targeting .NET Standard 2.0, and the reasoning for this is that this is supported from both .NET Framework and .NET Core. This allows us to build the same PowerShell module and load this in both Windows PowerShell (runs on top of .NET Framework, shipped ootb on Windows 11) & PowerShell (runs on .NET 8, installed separately on Windows, Linux, MacOS etc.)

I could of course just use pwsh, but the issue is that this is needs to be installed manually, while Windows PowerShell is always available on any Windows servers where I might need it. And I agree that support older versions of .NET Framework does not make sense, I think it working on 4.7.2 and above is fine, since that covers all OS from Microsoft that are still supported

See below, I am loading the same DLL, and get identical behavior from both (even if they are on different underlying versions of .NET)

image

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 5, 2024

OK, so basically it's just resolving things improperly, then, since we do multi-target to net472.

You can explicitly control resolution for nuget packages, instead of letting msbuild/nuget try to resolve it and end up picking the wrong one, by defining the fallback TFM used for dependency resolution. Here's what that would look like for your csproj, to make the net472 build grab the net472 nuget package specifically, and not change anything about other TFMs:

<AssetTargetFallback Condition=" '$(TargetFramework)'=='net472' ">
    net472;netstandard2.0;
</AssetTargetFallback>

That should make it pull the one you need.

The build environment does, of course, have to have the .net Framework 4.7.2 targeting pack, specifically, whether or not it has that specific version of the SDK installed, or things also can have other ways of not doing your bidding. Without that, since you probably have the 4.8 SDK, a specific target in the mass of .targets files crammed together in the virtual msbuild file at build time could also have subtle impacts on things.

NuGet's handling of a project's TargetFrameworks element isn't quite as straightforward as one might assume it to be. I came across a deep dive over some specifics recently. I'll see if I can find it to link here. But basically nuget doesn't always grab individual versions for every TFM in the plural tag, for every referenced package, and it's really annoying. The above workaround usually takes care of it.

I'm not sure how it behaves when using the project.deps.json format like that, though. The recommendation from MS is to migrate to PackageReference, in any case.

But the resolution override above should in theory take care of you, if using the csproj and PackageReference for nuget packages, since the dependencies aren't hard linked into the Terminal.Gui dll. While tracing it, I was at first suspicious of the System.Management 6.0 reference in Terminal.Gui.csproj, but netstandard2.0 components are available all the way down with that. But that does add to the difficulty it could have in dependency resolution, since that package doesn't explicitly define net472, nor do its dependencies. But they do define netstandard2.0.

And that's why I put both net472 and netstandard2.0 in the above xml - in the hopes that it will be enough to make it pick those versions, if that or anything else is what's confusing it.

But...

Another thought just occurred to me...

We started strong name signing at some point recently, which very well could theoretically create an issue for some configurations, since that does require a specific fully-qualified assembly name (which includes version) for all components, all the way up the chain.

Let me know if the workaround is the magic sauce you needed. I'm trying to find a solution that won't require another release and which is viable for anyone in your specific situation, to avoid any unexpected side-effects for anyone else.

However, if you do not include net472 in your TFMs, there's no resolvable version of Terminal.Gui, as-is, because there isn't an explicit netstandard2.0 TFM here right now. But letting nuget and powershell do things automatically is better anyway than trying to force netstandard2.0, which is more restrictive than net7.0 and net472, since it stopped at 462 and anything since then from framework or core isn't netstandard of any version.

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 5, 2024

Also, the workaround will not work if you don't have net472 declared.

This sort of thing is one of the many reasons netstandard was abandoned and Multi-Targeting was chosen as the preferred means of doing things since then. It works when it works, and might give you a single binary that runs anywhere. But when it doesn't, it's a pain to handle. And the tools all handle multi-targeting, so it shouldn't affect your users to do that instead of targeting only netstandard. And any other dependency dropping netstandard2.0 would have the same effect on you, as well, so breaking that limit seems like a win IMO.

@halvarsson
Copy link
Author

Thanks for looking into this. We only have .NET Standard 2.0 declared, and there's no users directly referencing our library, but its built as a PowerShell Standard Library Module that's distributed from PSGallery

I did however find a workaround to targeting multiple editions, and it would just require us including net472 / netstandard2.1 dll in the package, but at least it would allow us to keep it as the same module (and not require multiple versions)

I mainly created this issue cause I couldn't see why .NET Standard 2.0 support was dropped, but based on your descriptions it seems you had your reasons for doing it

I tried implementing the described workaround, with .NET Standard 2.1 + .NET Framework 4.7.2 on latest version of Terminal.Gui

<TargetFrameworks>netstandard2.1;net472</TargetFrameworks>

So I think this would be an acceptable workaround, I just need to reorganize our builds a bit to copy around the files and implement the logic for which dlls to load depending on the PowerShell edition

image

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 6, 2024

Hmmm.

Well, just to potentially save you some effort:

That shouldn't, in theory, be necessary at all. Multi-targeting "just works" without manual intervention, and a consuming system will grab the version appropriate for itself.

As long as you publish your nuget and psgallery artifacts for all targets, it should be all good.

The only "drawback" to that is that, if a user uses both Windows PowerShell and PowerShell, they'll end up with two copies of the module on their machine, but that should be transparent to them anyway, and is already how most modules work that have multiple target environments.

For example, if I were to open up windows PS and then run install-module to install your module, it'll go out and automatically get the one defined for Windows PowerShell.

Then, if I were to open pwsh and run install-module, it'll go out and get the one for Core.

The logic for what to get and load is what a manifest is for and shouldn't require manual loading of stuff for the most part. Just happens automatically on import. If you do need specific operations to occur aside from that, the manifest can declare stuff to run at import time as well.

Have a look at the Terminal.Gui v2_develop branch, in the Scripts folder, for examples.

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 6, 2024

By the way, I like the idea behind your module. Seems like a nice enhancement on top of a powershell environment. :)

Some of it seems like the kind of stuff MS might even slowly adopt, themselves, in PSCore (wouldn't that be nice?).

@dodexahedron
Copy link
Collaborator

One future warning, though:

Terminal.Gui 2.0 targets .net 8 and cannot be built against earlier versions of .net because of some source and binary capabilities required that can't be polyfilled.

@halvarsson
Copy link
Author

I think bundling multiple dlls and then conditionally load them when importing the manifest is the only way to have a multi edition module in PSGallery, its the suggested way from documentation for PSGallery, and its also how Microsoft does it on f.ex. PSScriptAnalyzer

I have a PR ironmansoftware/psedit#63 where I implemented it, but have some issues with CI that I need to figure out first. But on this branch I got it working on PowerShell 5.1 and in PowerShell 7.4 with Terminal.Gui 1.16.0

I am trying to get the module improved as much as possible before publishing a new version on PSGallery, as I recently merged in support for more languages, so it supports PowerShell, JSON and normal text so far. I have YAML support almost done and hope to add support for a few more languages.

But we had a lot of bugs related to Terminal.Gui, which you of course fixed long ago 😊 but due to us being on the earlier versions we've been unable to resolve them. I bumped us from 1.9.0 to 1.10.0 the other day, and this fixed a couple of bugs users had reported, but then it introduced new ones 😒

From what I can see from 1.16.0 then everything we encountered is resolved, so trying to get us onto this one

In regards to Terminal.Gui 2.0, I think we would move to it at some point in the future, but currently I get most of my value from having it work in Windows PowerShell, so that's where I put my effort currently. But I assume all the work being done should translate to v2 quite easily, since the editor logic etc. is decoupled from the Terminal.Gui specific code

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 7, 2024

I think bundling multiple dlls and then conditionally load them when importing the manifest is the only way to have a multi edition module in PSGallery, its the suggested way from documentation for PSGallery, and its also how Microsoft does it on f.ex. PSScriptAnalyzer

I have a PR ironmansoftware/psedit#63 where I implemented it, but have some issues with CI that I need to figure out first. But on this branch I got it working on PowerShell 5.1 and in PowerShell 7.4 with Terminal.Gui 1.16.0

I am trying to get the module improved as much as possible before publishing a new version on PSGallery, as I recently merged in support for more languages, so it supports PowerShell, JSON and normal text so far. I have YAML support almost done and hope to add support for a few more languages.

But we had a lot of bugs related to Terminal.Gui, which you of course fixed long ago 😊 but due to us being on the earlier versions we've been unable to resolve them. I bumped us from 1.9.0 to 1.10.0 the other day, and this fixed a couple of bugs users had reported, but then it introduced new ones 😒

From what I can see from 1.16.0 then everything we encountered is resolved, so trying to get us onto this one

In regards to Terminal.Gui 2.0, I think we would move to it at some point in the future, but currently I get most of my value from having it work in Windows PowerShell, so that's where I put my effort currently. But I assume all the work being done should translate to v2 quite easily, since the editor logic etc. is decoupled from the Terminal.Gui specific code

OOO! Something I learned fairly recently: If you have YAML support, you have JSON support and don't need two parsers, because YAML is formally a proper superset of JSON. So, any valid JSON document is implicitly already a valid YAML document, too (but not the other way around). Nifty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

2 participants