search

gluck / il-repack

Open-source alternative to ILMerge
Apache License 2.0
1.14k stars 213 forks source link

Bug: ILRepack creates corrupted resources by merging NET Core3.1 WindowsForms Applications #277

Open ymalich opened 3 years ago

ymalich commented 3 years ago

Hello, I've just found the bug by merging class libraries with a WindowsForms Application targeted to NET Core3.1. The integrated image resources can't be loaded anymore. The exception is being thrown "Unhandled exception. System.Runtime.Serialization.SerializationException: The input stream is not a valid binary format. The starting contents (in bytes) are: 04-AE-01-89-50-4E-47-0D-0A-1A-0A-00-00-00-0D-49-48 ...". I've created the small test project with all the binaries in order you can reproduce the problem easiely. Please find attached. WindowsFormsTestCoreApp.zip

ymalich commented 3 years ago

I've found where the problem is. The System.Resources.ResourceWriter is used by ILRepack to re-create resources. It always generates .NET Framework resource reader type ("System.Resources.ResourceReader, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089#System.Resources.RuntimeResourceSet") in the packed resource byte stream. The Core .NET applications use another resource reader type to read resources - "System.Resources.Extensions.DeserializingResourceReader, System.Resources.Extensions, Version=4.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51". I've just fixed the problem locally by passing source resource byte block to output stream without re-creation. I'll make tests for this case and create the pull-request.

BTW. I just merged the latest version Mono.Cecil into the version, used by ILRepack (it supports netstandard2.0), converted ILRepack to SDK-project format with net472 and NetCore3.1 output and made several code adaptations to the new Mono.Cecil version. Should I create the pull request for this upgrade?

iskiselev commented 3 years ago

This project does not look alive. Migrating to cecil in #236 is sitting in review for 2.5 years. I'd say PR improving current state is good - even if it won't be merged, it gives ideas what can be done in different situations to others. And project desperately needs some maintainer / well maintained fork.

ymalich commented 3 years ago

I see. Would be great to read the opinion of @gluck first, if he would like to give away the ownership to community. The most difficult question after that is - who would like to be the maintainer? I'm currently missing some knowledge in this area, so I'm not ready to be the maintainer, who could accept pull-request from others.

timotei commented 3 years ago

@iskiselev I am handling some of the maintainer tasks, though I didn't manage to review all the remaining stuff 😓

That PR was particularly tricky because it was actually breaking some of our internal apps with that upgrade. And as you posted there seem to be some other issues. However, we will need that update, so I will try to expedite the merging. Maybe we will release 3.0 as an alpha version with support for .NET Core

KirillOsenkov commented 7 months ago

Actually I'm keeping the issue open to investigate whether a better solution would be to use the same serializer as the original assembly. If the original resource is Core, use the Core serializer.

ymalich commented 6 months ago

Hello, the new version 2.0.21 merges my WinForms application assemblies correctly, without exceptions. I've testet with .NET 6.0, 7.0 and 8.0. Best regards,YuryÂ