ironfede / openmcdf

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

.net standard support #19

Closed michaelp closed 5 years ago

michaelp commented 6 years ago

Based on API compatibility analysis, openmcdf is 100% .net standard capable openmcdf_report.zip

It would be valuable to have openmcdf multi-targeting .net framework and .net standard

michaelp commented 6 years ago

@ironfede, any objections/comments if I try to do the port?

ironfede commented 6 years ago

No objection :-)! Thank you for your interest in openmcdf. If you want, you can open a pull request in order to merge changes in main branch when finished.

michaelp commented 6 years ago

Just an update: I have an issue with two tests that fail with the out of memory exception. The tests try to create multi gigabyte streams. Other than that, no obvious issues. I was busy for the last two weeks But i hope to resolve the issue during the next week.

SimonCropp commented 6 years ago

@michaelp can u share your work by submitting a PR?

michaelp commented 6 years ago

Already shared , what is missing is only tests. @SimonCropp . (actually only two tests are problematic)

michaelp commented 6 years ago

@ironfede @SimonCropp https://github.com/michaelp/openmcdf/commits/netsdt everything pushed. The problem is in the two tests that are trying to create large streams (6gb are accumulated in memory ) In general, there are changes required to reduce internal memory allocations.

michaelp commented 6 years ago

@ironfede @SimonCropp Heads up: I am reviewing the work that @ata6502 did on my fork in order to resolve all the issues. The claim is that everything is resolved. Once, I have the evidence that everything looks good, let's proceed. ETA - today (July 6th)

michaelp commented 6 years ago

@ironfede , @SimonCropp @altso I tend to think that the support for .net standard is ready. I run the tests locally and they are passing without issues. In general, changes to the code were minimal, the change was mainly in the projects formats. Please notice that dotnet pack generates multi-targeting packages as would be expected. The behavior of dotnet test as also standard. A major thing to remember is to pay attention to the package versions since they are deterministic now (i.e. require manual care) @ata6502 run the packages on our private code base and no obvious issues were found. I do observe that performance wise openMCDF is challenged in comparison to the COM driver when it comes to our scenarios. I profiled it and it seems to be a problem of constant resizing of collections and general memory management. But this is a topic for another pull request.

Please review and merge this pull request

altso commented 6 years ago

@ironfede, @michaelp thanks for you work on .net standard support. When do you plan to release this version on nuget?

ironfede commented 6 years ago

@altso I hope to have time to create and publish ngpack during this weekend.

SimonCropp commented 6 years ago

awesome work everyone :) thanks