-
-
Notifications
You must be signed in to change notification settings - Fork 368
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
Add basic support for netcore #190
base: master
Are you sure you want to change the base?
Conversation
…(new MemoryStream(array))` in netcore
Not sure what's the right way to create an PR with submodule? Or should I work on 2.0 branch which use dnlib from NuGet?
LIMITED:
<packer id="compressor">
<argument name="compat" value="true" />
</packer>
|
❌ Build ConfuserEx 408 failed (commit 3534876d74 by @yyjdelete) |
Seems CI use VS2017, and not support |
✅ Build ConfuserEx 412 completed (commit 490e257f31 by @yyjdelete) |
@@ -94,11 +94,12 @@ public static class ConfuserEngine { | |||
var asmResolver = new AssemblyResolver(); | |||
asmResolver.EnableTypeDefCache = true; | |||
asmResolver.DefaultModuleContext = new ModuleContext(asmResolver); | |||
asmResolver.EnableFrameworkRedirect = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this line without any conditions will break resolving the assemblies when mixing .NET 2.0, .NET 3.5 and .NET 4 assemblies in the same project. Means if you have a .NET 3.5 assembly as the dependency of a .NET 4 assembly, the referenced assemblies will not resolve properly, because the mscorlib
version changes. This works during runtime, because the .NET rumtime redirects mscorlib 3.5
to mscorlib 4.0
. The line you added, disables this functionality for dnlib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will test this and reference netfx dll in netcore
later, still not sure what will happen in this case. Is there any actual issue for the case can be tested?
The option is needed for netcore, otherwise something like System.Runtime 4.2.2
will be changed to 4.0.0 by FrameworkRedirect.ApplyFrameworkRedirect()
, and then it will perfer the one in GAC as extra match instead of netcore.
Since FindExactMatch
is still false, it can match mscorlib 4.0 if 3.5 is not found.And what will happen if resolve 3.5 for 3.5 and 4.0 for 4.0 side by side, if reference assembly don't needed to be confused, since they should be api compatable to executed.
@@ -384,6 +385,8 @@ public static class ConfuserEngine { | |||
if (!Path.IsPathRooted(output)) | |||
output = Path.Combine(Environment.CurrentDirectory, output); | |||
output = Utils.GetRelativePath(output, context.BaseDirectory); | |||
if (Path.IsPathRooted(output)) | |||
output = Path.GetFileName(output); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this is added? Why is the oath completely stripped in case the path is rooted? If the path is still rooted at this point, it only means that the output path has no relative relation to the location of the project. That is perfectly valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkaring
All usesite of context.OutputPaths[i]
look like Path.Combine(context.OutputDirectory, context.OutputPaths[i])
or Path.Combine(tmpDir, context.OutputPaths[i])
, if context.OutputPaths[i]
is an fullpath, it will get the same path as input, and makes the input file will be overwrite by output data.
We can just switch the CI to VS2019. That is not an issue. |
✅ Build ConfuserEx 413 completed (commit 567605b9c4 by @yyjdelete) |
Maybe it makes sense to update dnlib via the separated pull request? Also, I've created an issue for that: #208 |
Any updates? |
@NotoriousRebel Fell free to close this PR. |
7c93b95
to
74a624f
Compare
Fix #58, fix #106