Skip to content
This repository was archived by the owner on Mar 19, 2021. It is now read-only.

Conversation

@vocero
Copy link
Contributor

@vocero vocero commented Jan 10, 2020

Resource type RT_GROUP_ICON weren't written properly when calling the AddWin32Icon methods on the Writer class, the ID of each GRPICONDIRENTRY were written as an INT instead of a USHORT.

…r class

Resource type RT_GROUP_ICON weren't written properly when calling the AddWin32Icon methods on the Writer class, the ID of each GRPICONDIRENTRY were written as an INT instead of a USHORT.
@mike-barnett mike-barnett merged commit d16c5af into dotnet:master Jan 10, 2020
@mike-barnett
Copy link
Contributor

Wow! How did you ever find this? Thanks so much!

@vocero
Copy link
Contributor Author

vocero commented Jan 10, 2020

That's exactly what I told myself when I found it hehe. I was trying to find a method to make ILMerge use a different icon for the output file that the source or the attribute assemblies. So after experimenting with the AddWin32Icon method of the Writer class, I couldn't get it to work (the generated exe only contained the first icon size, and the rest were missing). I then tried loading the that generated exe in a win32 resource viewer, and I had the same problem (only the first icon size was showing).

I ended up writing a test .net app that would parse all the icons from the Win32 resources, and only after that I noticed that all GRPICONDIRENTRY icon structures were there, but only the first one had correct values, all subsequent structures values were invalid. I then came to the conclusion that ILMerge must not be writing them properly, debugged it and found it.

I assume that it was never reported as this method is not really exposed as an option in the ILMerge.exe, I just stumbled on it because I ended up calling it.

Glad I could help.

@mike-barnett
Copy link
Contributor

Very impressive! Congrats!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants