-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ILMerge fails to properly merge WpfMath #202
Comments
The code you mentioned most likely fails because of resource usage, see this line: https://github.com/ForNeVeR/wpf-math/blob/a09779dfbec9a4be1b8ed4df312a60255ac697d0/src/WpfMath/DefaultTexFontParser.cs#L224 Are you aware how ILMerge behaves with regarding to resources? Should we make any adjustments to the code to be ILMerge-compatible? |
I'm not aware, and I'm also personally unfamiliar with Pack URIs. Here is what and my look like in ILSpy. It's not obvious to me that the resources that are being looked for are changing relative paths, but it does seems obvious that this is near the root of the problem. Any thoughts? |
@ForNeVeR, I'm looking at this again and found someone else dealing with this kind of issue in another merger. However, it seems like their issue was with the path to the resource being based on the assembly name, which it doesn't look like is the cases here. Do you see any issues above with the paths to the resources and why or how the path might change depending on the application? If not, then I think there's an issue on the merging side, but if so, then maybe there is a way to make the URI in this repo more robust to merging. Thoughts? Thanks! |
Okay, it's totally possible that the assembly name should be right in our code (i.e. we should take the current assembly name instead of the hard-coded |
Change based on conversation in ForNeVeR#202, basic attempt failed
I get the kind of change you're interested in making, but I lack the pack URI experience and time at this very moment to dive more deeply into this other than the iteration in davidagross@42afb17. When that gets built and referenced in my test project I actually get the following error before deleting the collocated DLL, meaning I've regressed the code and would appreciate any tips on what I'm looking for in terms of variations on the gotten assembly name. I have my test project all set up to test updates to this library and I'm happy to continue the work with a bit more of your guidance and support, but I'm also happy to hand off the assignment of this issue to you if you want to take it from here. Thanks, and thanks for resolving to work on this given the state of affairs explained in dotnet/ILMerge#70. |
Ok, now it is the moment when I'll need to take a closer look at the project. Seems like you've done everything right. |
Happy New Year, @ForNeVeR. Any progress on this, or anything I can help test on my end? |
Unfortunately, not yet; sorry for the pause. I'll try to make it till 2020-01-09. |
I've taken a look at the issue. The following patch doesn't solve the problem, even if it was supposed to: Index: src/WpfMath/DefaultTexFontParser.cs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/WpfMath/DefaultTexFontParser.cs (revision 7ae7168a86dcd425a238ee25a9c9e2e72d21d1ec)
+++ src/WpfMath/DefaultTexFontParser.cs (date 1584174774150)
@@ -221,7 +221,8 @@
private GlyphTypeface CreateFont(string name)
{
// Load font from embedded resource.
- var fontUri = new Uri(string.Format("pack://application:,,,/WpfMath;component/{0}{1}", fontsDirectory, name));
+ var assemblyShortName = Assembly.GetExecutingAssembly().GetName().Name;
+ var fontUri = new Uri($"pack://application:,,,/{assemblyShortName};component/{fontsDirectory}{name}");
return new GlyphTypeface(fontUri);
} Even when I'm trying to use a correct assembly name (according to the pack URIs documentation), the following exception gets thrown (note it's not "file not found" anymore, but something about "I/O Error occurred"):
Here's the resource section of the binary after ILMerge: It looks like From this investigation, it's clear that we cannot do anything about the issue in the WPF-Math code currently. If you'll ever found a way to rename the resource using ILMerge, then please inform me, and I'll try if the patch helps the resulting application to work. If it is, then we'll merge it. I'll not merge that patch to the main code base for now, because we've no proof that it'll work even after some resource-related workarounds with ILMerge. For now, I'll close the issue as (supposedly) non-actional from our side. Feel free to comment though, if it becomes actionable then I'll gladly reopen and fix it. |
Repository https://github.com/davidagross/ILMerge-vs-WpfMath provides a MWE of this error.
Basically, when
WpfMath.dll
isn't next to the merged, standalone executable, the render fails:indicating that the merge was not successfull. This is also tracked in dotnet/ILMerge#70.
The text was updated successfully, but these errors were encountered: