search

ironfede / openmcdf

Microsoft Compound File .net component - pure C# - netstandard 2.0
Mozilla Public License 2.0
308 stars 76 forks source link

[RFC] Add a .NET 8.0 target to OpenMcdf.Extensions #123

Open Numpsy opened 7 months ago

Numpsy commented 7 months ago

I see from the code history that there was once a change to .NET 6.0, that was then reverted to .NET Standard 2.0. However, I think it would be useful to add an additional .NET 6.0 target on top of the existing ones (rather than removing them).

The main immediate arguments are:

  1. A .NET 6,0 build doesn't need to use the System.Text.Encoding.CodePages NuGet package, as System.Text.Encoding.CodePages is built in to it.
  2. In order to enable new features like IsTrimmable and get analysis warnings about any issues. (I've been trying to use it in a self contained, trimmed build recently, and more early analysis is always useful there)

But also it should be useful for #57 to make span based functions in System.IO.Stream and related available, as there are places that should be able to benefit from things like spans, stackalloced buffers etc (e.g. writing null terminators and padding with spans, instead of writing bytes in loops)

This is just doing the Extensions lib now due to the System.Text.Encoding.CodePages dependency, but the same question exists for the core library.

Thoughts?

ironfede commented 7 months ago

You're right in your analysis but compatibility was (and I think that still is) one of the major goals for OpenMcdf. Now, this doesn't mean that we can't have platform support upgrade but if you introduce support for net 6 in this branch, all things you're mentioning should be included and developed with conditional version-moniker in the code, I suppose. Do you think that this could be supported without risking a lot of regression errors and potential pitfalls in development? In my opinion, pheraps, a new major release targeting .net 6 would be more mantaineable. This would also possibly imply freezing new developments and evolutions on 2.x branch. Do you think it could be a feasible solution?

Numpsy commented 7 months ago

Do you think that this could be supported without risking a lot of regression errors

I think the main thing would be that if the library targeted .NET 4.x / 6.0 / Standard 2.0 and the unit tests target 4.x/6.0 then the Standard build wouldn't get covered (you could add another earlier version to the tests easily enough, with the caveat that .NET Core 3.1 and .NET 5.0 are both out of support)

Another thing is that you can potentially use a subset of the Span features in .NET 4.x via the System.Memory NuGet package, but I think not all the extra functions that have been added to Streams etc (as an example, the metadata extractor library has recently been having some work to use spans done, whilst still supporting .NET 4.6.2)

jeremy-visionaid commented 1 month ago

@Numpsy Thanks for looking at this!

@ironfede I'd love to use OpenMcdf in one of our projects. I think we're going to want to add .net 6, spans etc. to it, but also some other likely breaking changes to improve the API and increase performance, reduce memory footrpint, possibly trimmable support, etc. My company and others I work with are happy to make contributions to that end. We're going to need to ship in a few months though - Is there anything that would be useful to assist in moving towards a v3 release?

Numpsy commented 1 month ago

I started a bit of span stuff in #125 though just the suggestions in #57 and to get comments on bumping the minimum supported .NET version vs. conditionally compiling the span related functions for different .NET versions.

possibly trimmable support

fwiw I'm already using it in trimmed builds (the only warning I've seen in that area was fixed in #126)