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

Add back .NET standard 2.0 support #175

Open
Drake53 opened this issue Aug 27, 2024 · 7 comments
Open

Add back .NET standard 2.0 support #175

Drake53 opened this issue Aug 27, 2024 · 7 comments

Comments

@Drake53
Copy link

Drake53 commented Aug 27, 2024

Support for .NET standard and .NET framework was dropped when Pidgin went from v2.5.0 to v3.0.0
Due to Microsoft dropping .NET 6 support soon and .NET 7+ only being supported on windows 10+, I am in the process of backporting my libraries and applications to .NET standard 2.0 and .NET framework. I tried to downgrade Pidgin to v2.5.0, but got many build errors from breaking changes in this library, so it would be nice if v3 could be updated to support .NET standard again.

@benjamin-hodgson
Copy link
Owner

benjamin-hodgson commented Aug 28, 2024

No promises, but I’ll see what I can do.

Is supporting Windows older than 10 super important to you? Windows 8 has been unsupported (just like .NET 6) for well over a year now. I guess I’m asking why the sunset of .NET 6 is causing problems for you while Windows <10 is not?

@Drake53
Copy link
Author

Drake53 commented Aug 28, 2024

Is supporting Windows older than 10 super important to you?

It is, I'm personally still running windows 8.1.

why the sunset of .NET 6 is causing problems for you

I guess it's mainly because of the UX issue of asking your end users to download a .NET runtime which is no longer supported. Like if you want to download .NET 5 now you get this message:

image

Of course .NET 5 still works and .NET 6 will keep working, but I still think in the long-term it'd be best to move back to .NET framework 4.7.x which has no EOL date (yet).

@benjamin-hodgson
Copy link
Owner

benjamin-hodgson commented Aug 28, 2024

If you don't mind my asking, what kind of software are you shipping? Who are your users?

I'm just trying to understand the constraints here - it seems odd to me that folks are happy to continue using an unsupported OS but installing an old runtime is a bridge too far for them.

If installing the runtime is the sticking point, you could consider shipping your app as a self-contained executable?

@Drake53
Copy link
Author

Drake53 commented Aug 28, 2024

If you don't mind my asking, what kind of software are you shipping? Who are your users?

Mostly modding tools for Warcraft 3

I'm just trying to understand the constraints here - it seems odd to me that folks are happy to continue using an unsupported OS but installing an old runtime is a bridge too far for them.

I don't expect many of the users to actually still use windows 7/8 like myself, the problem is mainly that I develop and build the applications locally so I can't target .NET 7+. I could maybe work around this by setting up CI/CD though.

If installing the runtime is the sticking point, you could consider shipping your app as a self-contained executable?

That's actually a good suggestion, I might do that as well.

Before I forget, there's two other things I'd like to mention as well.

One benefit of targeting .NET standard 2.0 is that you make the library compatible with source generators, because these can only consume .NET standard 2.0 assemblies. I personally have no plans of making a source generator though.

Also, in case you weren't aware of it yet, you could use PolySharp so you don't have to put #ifs everywhere when adding .NET standard support. This library will add some of the 'missing' classes like nullable attributes and index/range, so you can keep using the newer language features even in .NET standard.

@Drake53
Copy link
Author

Drake53 commented Aug 31, 2024

I attempted to do this myself, see: Drake53@86b610d

There are a couple of methods which are not available in .NET standard 2.0 and for which I couldn't find a package, to summarize the changes:

  • Using arrays instead of spans in a couple of places, not sure about the performance implications;
  • Couldn't find an alternative for RuntimeHelpers.IsReferenceOrContainsReferences, so I simply always set _needsClear to true where it's used;
  • For enum methods, using the overload which takes the enum type instead of the generic version;
  • Replaced default interface implementations with implicit interface implementations.

Note that the PolySharp package requires VS 17.3+ so I simply copied the required files into the project.

I won't be making a PR because I also had to make some changes to get the project to build in VS2019, but feel free to use my changes as a reference if you decide to implement this.

@benjamin-hodgson
Copy link
Owner

benjamin-hodgson commented Sep 4, 2024

I looked into this. Supporting NetStandard would mean breaking ITokenStream as it contains default implementations. I don’t want to do the breaking change for Net6 (since I think it’s a disimprovement) and I don’t want to conditionally compile the default impls as that would mean shipping incompatible assemblies in the same package (would cause mayhem with asset selection).

I could conditionally compile the entire interface, and all the public methods that mention it. So ITokenStream would only be (publicly) available on Net6. It’s an 80% fix. What do you think?

@Drake53
Copy link
Author

Drake53 commented Sep 4, 2024

I personally have never used default interface implementations and IMO the feature is kind of counter-intuitive. I find it a bit ironic as well because I read that this feature is intended to allow you to update an interface without causing a breaking change, but in this situation the feature is actually causing a breaking change (unless you go for the conditional compilation solution of course).

IMO the easiest solution (which I also applied in the commit in my previous post) is to remove the default interface implementations. While this would be a breaking change, the two members with a default implementation are trivial, so it won't be much effort for anyone affected by the breaking change to update their code.

On the other hand I think the conditional compilation solution will make the library harder to maintain and use. If anyone uses the library and has multiple target frameworks like .NET standard 2.0 and .NET 6, they will see the interface is only available in .NET 6, so I don't think that would be a good solution.

I might be a bit biased though, because I don't use ITokenStream in my code, so I wouldn't be affected by a breaking change in this interface.

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

No branches or pull requests

2 participants