-
Notifications
You must be signed in to change notification settings - Fork 471
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 Target Frameworks for Castle Core v5.0 #597
Comments
This comment has been minimized.
This comment has been minimized.
Sorry, please ignore my above post. I misread some relevant parts of the top post. TL;DR: I think the targets we have now are still fine. I understand the desire to only target supported versions, but OTOH when it's easy for us to support older versions (no code changes needed), why shouldn't we? We'll otherwise force downstream libraries to update their targets, and I'm not sure we need to do so. What are the actual downsides to targeting unsupported older versions? This isn't the same situation now as when we dropped support for .NET 3.5 and .NET Standard 1.x; dropping those allowed us to simplify our code and conditional compilation. This time, by dropping Re: exchanging .NET Standard, citing from here:
I'm not sure we need any new APIs just now. The major one we've been waiting for is probably |
Do we know any dates when Castle Core v5.0 will be released? We are waiting for .netstandard 2.0 support as a step forward for migration to .net 6 from .net 4.8. Seems it is the latest item for this release |
@talenor Does |
@jonorossi Well... we haven't dived deep into netstandard1.5 because netstandard 2.0 exists quite a long time and we checked only 2.0 so I can't provide a reasonable answer. |
@talenor Are you just using Castle.Core (maybe via a mocking library), or other Castle libraries too? |
@jonorossi only Castle.Core (directly, via some ninject extensions, and via Moq) |
We are also waiting for this because we want to get rid off these old assemblies that come from old version of netstandard libraries #602 When there is newer target framework defined then those assemblies dont get installed. This is why .net6 target framework would be good |
Also since .net5 that is the recommended way forward from microsoft. https://docs.microsoft.com/en-us/dotnet/standard/frameworks .net 5 and .net 6 TFMs should be included in next major IMO |
Is there any reason why castle core cannot just add netstandard 2+ / net5 / net6 target frameworks and reduce the big number of unnecessary dependencies for those target frameworks https://www.nuget.org/packages/Castle.Core/ This way modern applications can get rid off possible security vulnerabilities #602 and reduce installation size? |
I know that with .NET Framework the target framework affects things at runtime (e.g. default SSL/TLS protocols enabled is a big one until you upgrade to 4.6.2), but that could only apply to the executable and not a library. |
@stakx's quote from Microsoft Docs is still there regarding whether we need Any one with a concrete reason for wanting the target frameworks to change for v5.0? Otherwise I'll close this by the end of the week. |
Hi @jonorossi, what would you define as a concrete reason? We bounced a couple of messages in #602 relating to transitive references, and that's probably one reason I'd push for including Reviewing the Microsoft Docs page from above, there is this in the same section under Reusable libraries:
It doesn't seem entirely unreasonable to assume that having a There's also less chance of subtle bugs, such as breaking changes causing issues, where older versions are used because of Castle pulling them down. I had an issue recently with Kestrel, where my project was pulling in an older version because of a dependency, but other code was using newer shared DLLs that built fine, but then broke at runtime because of changed functionality. |
A Or does the lack of a |
I had a quick look at some of the top 100 by downloads on nuget.org (https://www.nuget.org/stats/packages) and found the most downloaded (e.g. Json.NET, Serilog, nlog, log4net) that run across heaps of frameworks didn't have a .NET 5/6 target. As I went down the list I found a bunch that do (e.g. StackExchange.Redis, FluentValidation, Google.Protobuf, Npgsql, RestSharp, Autofac), but each of them use at least one new API or they replaced older netcoreapp with net5/6. Happy to add a new target if it solves a problem, but similar to @stakx I too am not seeing what we gain with another target to build, run unit tests against and ship around in the nuget package. |
I think that point about targetting .NET 5 or .NET 6 is that netstandard requires package references that are not required if targetting the actual framework because those are part of the runtime. Thus a smaller dependency tree, which is much easier to manage for those who have requirements to run package license and security scans. It most likely means that some of the dependencies are more up to date, as netstandard likely references the lowest supported version of a dependency. I know that we have had to reference up to date versions of System.Net.Http and System.Text.RegularExpressions in projects which contain a reference that targets .NETStandard 1.0 as the implicit versions referenced were from 2017 and had known high severity security vulnerabilities. As these packages were not needed by the actual project, which had access to those classes and namespaces as part of the framework, it was necessary to install additional packages simply to update implicitly added references and satisfy a security scanning tool. I believe this is what @Jevonius is referring to. |
@Jevonius is this true, I'm no expert on this stuff. If so, that's been enough. |
@Jevonius @pwdst If we added Is there any recommendation or feeling on how long we'd want to even keep a .NET Standard target for anymore? If we kept a net4.x target for .NET Framework and added net5/6 for .NET, at some point all the old .NET Core versions are well and truly obsolete and unnecessary to support. We know that DynamicProxy doesn't work on all .NET Standard targets and that we just let the runtime throw a platform not supported exception. I assume with the .NET linker project in development, things probably work much better without .NET Standard too which is sort of obsolete now. I think having a single .net5/6 wouldn't be too much of a problem, just don't see the need to have both. .NET 5 is end of life May 8 so no one should be doing new development using it, I'd have no problem just doing net6.0 if it solves the dependency messy problems with .NET Standard. |
Correction for my previous comment. One reason to kept a .NET Standard target is for Windsor, if it wants to target .NET Standard Castle Core also needs to target .NET Standard. |
It is worth noting that Newtonsoft.JSON has not had a release for over a year, is heavily constrained by previous design decisions that the author has publically said would not be made today, and has no public roadmap for future releases (at least when I last checked a couple of weeks ago). Microsoft has largely replaced it with System.Text.Json by default as of .NET Core 3.0 - authors are required to opt back in to Newtonsoft.JSON after this point. The latest stable version release two months ago targets .NET Framework 4.6.1, .NET Core 3.1, .NET 6 and .NETStandard 2.0 - the most recent preview version drops explicit .NET Core 3.1 support, bumps .NET Framework to 4.6.2 and adds .NET 7. This is clearly how Microsoft believe that libraries should handle target frameworks. That said, with a Long Term Support version of .NET Core and now .NET offered periodically, I would probably offer the following-
I would probably ignore the non LTS releases unless they added a new API that was useful to the library. |
Agreed that if adding Let's just not confuse today's situation with how it was back then with I can't judge whether that one |
@jonorossi I don't believe that anyone is asking that .NETStandard support is dropped or taken away as a target framework, simply that a recent .NET framework is added as additional target. This would allow the library to be used both in projects that target .NET Standard themselves, and also have potential benefits for projects that are targetting the full framework. I would imagine a supported LTS version would be more than sufficient for any authors. For a planned release of Castle.Core that would probably be .NET 6, as .NET Core 3.1 is ending LTS support this year and authors should be upgrading anyway. |
I'm not an expert either! @pwdst's answer (#597 (comment)) aligns with what we've observed but it's all anecdotal. Your follow on question around when to drop support for framework/standard/older cores is worth some thought - the jump to 5.0.0 would seem like a reasonable time to drop support for older versions, and @pwdst's suggestion around supporting As an aside, as I understand it the guy that wrote Newtonsoft.JSON now works for Microsoft and is part of the System.Text.Json team, which probably explains why the original library is in decline. |
NETStandard.Library is that package, but when using .NET Standard 2.0 it looks like it only has
Sorry, I didn't mean for it to come across like that. Just trying to understand the landscape better. I've really lost track of the "best practice" guidance for target frameworks in the last couple of years.
Sounds good. In master right now we've got
@Jevonius @pwdst Do either of you have the time to contribute part or all of this work? |
To be clear, you mean retargeting the projects and putting a PR together? I should have some time for that, it's a fairly minor tweak and hopefully won't break any tests! |
Clarification for those reading at home: dropping it from master. As those here know, we've never had a release with a .NET Standard 2.x target.
Yes, Probably best to do them as 2 separate commits (i.e. netfx and net6), or even 2 separate pull requests. Our build has Mono building and testing too. Our set up is a little unique, make sure to check out the files in the |
As suggested, I've created a PR (#614) moving the minimum version up to 4.6.2. First attempt at contributing, but I've tried to be guided by previous PRs/commit structures so hopefully all in order. If happy with this, I'll take a look at the changes for |
Just for reference, the Quoting from @jonorossi's decision from #407 (comment):
Since it now looks like we're going to drop support for ancient framework versions, perhaps we should drop |
Ah yes, I had completely forgotten about that. Cheers.
"commented on 28 Apr 2020"... that's right now 😆
That'll drop support .NET Core 2.1 which we currently support through .NET Standard 1.x, but I'm good with that since it was end of life August 21, 2021. .NET Standard 2.1 supports .NET Core 3.1 which is still in support. And we support .NET Framework via the netfx target rather than .NET Standard anyway. Sounds good to me. @Jevonius do you agree? |
I'm a little wary of suggesting removing Given the only difference I can see between Castle builds for 2.0 and 2.1 is the inclusion of the |
Despite my above post, and despite having argued against For me, the more important thing would be to not drop One argument against having too many TFMs was that we'd need to cover all of them in the CI test runs (thus making the CI setup more complex and slower). How about keeping
Also agree with that one. I think it's fine either way, with or without |
Good reasoning @Jevonius. I think to keep things moving we just take the path of least resistance and go with both (since we've already got them) and leave it running tests on both targets as the CI build is pretty quick (5 minutes) anyway. We might find by the time we get to v6 no one is using .NET Standard targets anymore 😉 . So we do: |
For the unit tests we've currently got: <TargetFrameworks>net462;netcoreapp2.1;netcoreapp3.1</TargetFrameworks> There is no point testing on end of life .NET runtimes, do we change this to: <TargetFrameworks>net462;net6.0</TargetFrameworks> |
3.1 is in support until December 2022, so I'd be tempted to say:
This then covers the |
Ah yer, 3.1 is an LTS release too. Yep go with your proposed list. |
Have just opened a PR (#616) for adding |
Coming back to dropping |
That would, IMHO, be the wrong move, as I commented elsewhere recently. If we decided to drop any of the If it's important to you guys to set the test coverage issue aright, then by all means go ahead and remove |
netstandard2.1 is not compatible with older xamarin and .NET framework, but as people are including .net45 for example I don't this think this really matters. https://dotnet.microsoft.com/en-us/platform/dotnet-standard#versions Adding new target frameworks is not breaking change... so if anybody ever comes with an issue about those they can be added in 5.0.1 and so on... Just hurry up with the release of v5 please Not having a modern target framework is bigger issue than having some old target framework as they can always be added later without breaking changes... |
Thanks to @Jevonius we've now got a .NET 6 target in master (#616). Here are the final TFMs for Castle Core v5.0.0: <!-- v4.4.1 -->
<TargetFrameworks>net35;net40;net45;netstandard1.3;netstandard1.5</TargetFrameworks>
<!-- master / v5.0.0 -->
<TargetFrameworks>net462;netstandard2.0;netstandard2.1;net6.0</TargetFrameworks> |
We've made changes to our target frameworks for our next major version but none of this has been released yet. With so much time passed and the .NET Core landscape changing I think we need to re-evaluate our target frameworks before releasing.
.NET Core notes:
.NET Framework notes:
Our set up:
What do we support for v5.0? I don't think we should be supporting any .NET Core versions that are now end of life, there is no reason for people to expect we support new versions of Castle Core on end of life runtimes. Do we continue with any .NET Standard TFMs? Even though DynamicProxy might not work on some .NET Standard-based runtimes, Windsor depends on Castle Core and so we shouldn't restrict Windsor unnecessarily.
/cc @stakx
The text was updated successfully, but these errors were encountered: