nissl-lab / npoi

a .NET library that can read/write Office formats without Microsoft Office installed. No COM+, no interop.
Apache License 2.0
5.67k stars 1.43k forks source link

System.Drawing.Common is deprecated on non-Windows platforms #656

Closed safern closed 2 years ago

safern commented 2 years ago

Hello, I work on the .NET Libraries team, and I saw that this library has a lot of usage in the community and depends on System.Drawing.Common for xplat scenarios.

We made the decision to make System.Drawing.Common a windows only library starting .NET6+ due to a lot of reasons after a long discussion. You can see more details here: https://github.com/dotnet/designs/blob/a15a8c7a324c482c5e38b73e46c2afe56f6bd504/accepted/2021/system-drawing-win-only/system-drawing-win-only.md

Also: https://github.com/dotnet/docs/issues/25257

We wanted to reach out so that you could react to this changes when .NET 6 is released. Also if you have any questions/concerns please do let us know at https://github.com/dotnet/runtime

What is the recommended action to still support winforms/wpf scenarios for example?

You can mark the APIs that need to use System.Drawing.Common to support winforms/wpf as windows only using the SupportedOSPlatform("windows") annotation, and then add new APIs that use another library like ImageSharp or `SkiaSharp. System.Drawing.Common is deprecated on non-Windows platforms

tonyqus commented 2 years ago

Thank you for reaching me. I'll evaluate the breaking change in .NET 6.

ligg commented 2 years ago

expect for fixed it

tonyqus commented 2 years ago

@ligg Can you help on this? We need an alternative library to make it work.

tonyqus commented 2 years ago

@safern Do you have any suggestion about alternative to System.Drawing.Common on Linux?

ligg commented 2 years ago

I tried to fix it, but I haven't succeeded yet now,i set the environment variable "DOTNET_System_Drawing_EnableUnixSupport=true" in docker,it's working well

tonyqus commented 2 years ago

https://docs.microsoft.com/en-us/dotnet/core/compatibility/core-libraries/6.0/system-drawing-common-windows-only

tonyqus commented 2 years ago

760

tonyqus commented 2 years ago

I comment on dotnet runtime repo to complain about the retiring of System.Drawing. If you have the same option, please comment in this issue.

jaklithn commented 2 years ago

I am also very eager to get rid of this unnecessary dependency causing trouble in CI/CD pipelines. According to the above link on Microsoft they recommend you switch to alternative libs.

"To use these APIs for cross-platform apps, migrate to one of the following libraries:

Is that something that can be done in the near future? In order to keep using this lib I need to confirm to my tea that there is a plan to have this fixed.

tonyqus commented 2 years ago

@jaklithn Yep, I'm actually planning this because I don't think I have the power to change decision from .NET team. But migration to one of these library may take at least 2-5 manday in order to finish coding and QA.

760 is a potential fix. @ligg is help on this. I'll review it in 2 weeks.

emisand commented 2 years ago

@tonyqus I have been migrating our company solution from System.Drawing to ImageSharp. I think it's one of the most straightforward options to choose, it's fully made with .NET code, and it does not have any external dependencies to binaries in the system like SkiaShap or Maui have,

You will start by adding these two package references:

<PackageReference Include="SixLabors.ImageSharp" Version="2.0.0" />
<PackageReference Include="SixLabors.ImageSharp.Drawing" Version="1.0.0-beta14" />

Documentation on the image handling: https://docs.sixlabors.com/articles/imagesharp/gettingstarted.html Documentation on the graphics drawing: https://docs.sixlabors.com/articles/imagesharp.drawing/gettingstarted.html

Let me know if you encounter any issues while migrating.

Xenoage commented 2 years ago

I created a quick experimental version, where all System.Drawing code is removed, either replaced by SixLabors.ImageSharp (image and color functionality) or commented out (text measuring functionality). Thus, calling methods like ISheet.AutoSizeColumn have no effect. Apart from this, everything seems to work fine.

https://github.com/Xenoage/npoi/

Xenoage commented 2 years ago

Also added SixLabors.Fonts library and reactivated the text measuring functions. Was quite simple; but maybe fallbacks for missing fonts should be added (see "TODO-Fonts" comments).

The two relevant commits:

tonyqus commented 2 years ago

@Xenoage Can you create a PR? So that it's easy for me to compare and review the code. Thank you!

beaver316 commented 2 years ago

@tonyqus is there any progress with this? I am using Blazor WASM, debugging on Windows using Edge browser, and I receive the following error in my code for saving a workbook to a filestream.

"Unhandled Exception: System.PlatformNotSupportedException: System.Drawing is not supported on this platform."

Interestingly only when I autosize a column using ISheet.AutoSizeColumn is the error thrown. If I comment out these lines then my code works fine. For now I don't autosize in order to avoid the problem. I suppose the autosize method is making use of the deprecated library.

tonyqus commented 2 years ago

@beaver316 The new fix will not release very soon. What I suggest is to enable System.Drawing.EnableUnixSupport according to MS Doc.

flq commented 2 years ago

Hi @Xenoage any progress on this, or sth to be picked up? I'm currently lobbying in our company to get some days in order to migrate to ImageSharp and get rid of System.Drawing, so I'm wondering what happened? 😅

flq commented 2 years ago

@beaver316 The new fix will not release very soon. What I suggest is to enable System.Drawing.EnableUnixSupport according to MS Doc.

What is meant by "not releasing very soon"? Provided we could port to ImageSharp, this would be a good thing to push forward, right?

Xenoage commented 2 years ago

Our fork, using ImageSharp instead of System.Drawing, is working fine so far and is already deployed in production. Sorry for not having the time to clean up a few things and creating a pull request yet. I'll do that as soon as possible.

flq commented 2 years ago

Cool - if you want any help...I could cherry-pick your commits relevant to the code onto the latest commits from here and then it could become a PR. Leaving System.Drawing behind certainly seems to be the future.

If the maintainers would rather stick to the MS dependency, I suggest we could create an abstraction layer. As far as I can see, the only reason System.Drawing/ImageSharp is used is to use some colors and fonts - maybe this could be pushed behind a facade such that exchanging the dependency is a straightforward thing to do?

tonyqus commented 2 years ago

@beaver316 The new fix will not release very soon. What I suggest is to enable System.Drawing.EnableUnixSupport according to MS Doc.

What is meant by "not releasing very soon"? Provided we could port to ImageSharp, this would be a good thing to push forward, right?

The planned next release is Oct, 2022. Since NPOI is a weekend project, I don't wanna make the schedule too tight. I still need some nice outdoor weekends especially after the Shanghai lockdown.

Anyway, I'm considering reschedule the release. maybe to Aug. (I cannot confirm yet) Or I release a sub version like 2.5.7 and cherrypick some changes.

flq commented 2 years ago

Totally get you there, your outdoor weekends are well deserved! The .NET Core releases have usually been towards the end of the year, so that should work out nicely.

Do you agree that ImageSharp dependency is the way forward or do you want to seek out other solutions?

tonyqus commented 2 years ago

@flq ImageSharp looks to be a good candidate. The only problem I'm thinking is that if I should totally get rid of System.Drawing for any .NET version (including versions before .NET 6). Or I should enable ImageSharp for just .NET 6 only. This replys on the stableness of ImageSharp in each .NET standard version.

flq commented 2 years ago

That I don’t know unfortunately, in our product we use .NET Core 6 and previously 3, where it was/is being used without issues. If you want to stay open for all options I would propose putting the divergent calls behind an interface and switch out the implementation based on compile target. Unfortunately I can really only test the ImageSharp branch, someone else would need to check the System.Drawing branch

ligg commented 2 years ago

这个问题还没修复啊,我们用imagesharp改造的版本已经用了很久了,目前为止一直没有什么问题,我建议直接再发布一个.net 6.0的版本来维护 foreign friends, if you want to know what I'm talking about, please use google translate

tonyqus commented 2 years ago

@ligg 其实我感觉社区里用.NET 6的人不太多,只是某些MVP在强推而已,NPOI的用量可以间接反映.NET版本的使用情况,如果.NET 6用的人多的话,这个帖子早炸了,我目前也就接到10个以内的complaint,也就是说全球范围内.NET 6的装机量并不高,大家都在观望。实际上我之前做自由职业的时候,涉及的项目包含多家公司,有相当一部分还在使用.NET core 2.2/3.1,.NET 5的都很少。

It looks .NET 6 is not so popular in the community so far. Only a few Microsoft MVPs are pushing this to happen. The usage stats of NPOI can indirectly reflect the usage of .NET versions. If there are many .NET 6 developers, this post has been very long. But I only got at most 10 complaints from the community about System.Drawing is not usable in .NET 6. In other words, the installation volume of .NET 6 is not high. A lot of developers are pending instead of migrating. While working as a freelancer these years, the projects I'm involved in only uses .NET core 2.2/3.1. Even .NET 5 is very rare. There are still huge projects using old .NET stack.

flq commented 2 years ago

So I guess I fell through the cracks, after all this legacy stuff still works on windows. Let me assure you we aren’t a bunch of MVPs just some product team that gets a lot out of their rented hardware since every time we upgrade to the next LTS version (which NET6 is) we get performance gains just for updating the framework.

If you think an adapter approach is too much of a maintenance burden, then that’s OK, I won’t bother 👍🏼.

tonyqus commented 2 years ago

@flq Don't worry. Support for .NET 6 is on NPOI's roadmap. It will not be later than Oct, 2020.

The .NET market in China is a bit different from global market. We lost major market of .NET in the past 5-7 years. A few big .NET companies have migrated to Java. Although I totally understand that the performance gains of migrating to .NET 6, it's still dangerous if anything goes wrong (for example, breaking changes or any logic re-writing). .NET is tagged as a not-so-good and not-secure language in China.

I don't think the major reason is .NET language itself. A lot of mistakes are made by human (.NET developers in China). But .NET language as a tool takes this responsiblity instead of these developers. These developers finally got a Java/Go job and they even got salary increasement after changing the company. The consequence of this is that most business guys in China believe that .NET cannot meet most business needs but Java is much better. Big Internet companies like Alibaba, JD has huge impact on programming language choices. JD used to use .NET but finally they migrate to Java.

tonyqus commented 2 years ago

According to SixLabor nuget, this library only support .net framework 4.7.2. That means NPOI have to give up support of .NET framework 4.5. Any concern on this?

tonyqus commented 2 years ago

894 PR created

flq commented 2 years ago

According to SixLabor nuget, this library only support .net framework 4.7.2. That means NPOI have to give up support of .NET framework 4.5. Any concern on this?

I guess they also dropped support because that version has reached its end of support this year in April . I think it’s the right thing to do to leave unsupported versions behind.

Synthose commented 2 years ago

Since #894 merged , is there a guide to getting the column autoSize method up and running, or is that still non-functional?

tonyqus commented 2 years ago

NPOI 2.6.0 rc is released. It's a prerelease version. Please help test if it works on Linux with .NET 6

flq commented 2 years ago

Will have a look once I’m back from holidays

flq commented 1 year ago

I can confirm that this release would work really great on our Mac machines - fantastic cross-platform story. Also a very nice touch to allow to keep the stream open on write (as every library should do, really ;)

So, big thumbs up from me, looking forward to the release. We won't be running any prerelease nuget in production, but as soon as this hits the road, we'll take it.

s4lvo commented 1 year ago

After the upgrade to the v2.6.0 I have a different error on linux:

Sequence contains no elements at System.Linq.ThrowHelper.ThrowNoElementsException() at System.Linq.Enumerable.First[TSource](IEnumerable`1 source) at NPOI.SS.Util.SheetUtil.IFont2Font(IFont font1) at NPOI.SS.Util.SheetUtil.GetDefaultCharWidth(IWorkbook wb) at NPOI.SS.Util.SheetUtil.GetColumnWidth(ISheet sheet, Int32 column, Boolean useMergedCells, Int32 firstRow, Int32 lastRow, Int32 maxRows) at NPOI.SS.Util.SheetUtil.GetColumnWidth(ISheet sheet, Int32 column, Boolean useMergedCells) at NPOI.XSSF.UserModel.XSSFSheet.AutoSizeColumn(Int32 column, Boolean useMergedCells) at NPOI.XSSF.UserModel.XSSFSheet.AutoSizeColumn(Int32 column)

tonyqus commented 1 year ago

@s4lvo This looks to be a new bug. Can you create a new issue for that?

Xenoage commented 1 year ago

Quick workaround could be on Linux to install the Microsoft Fonts, e.g. package ttf-mscorefonts-installer on Ubuntu. Would be great if the library can work without those fonts.