Fix several issues with config merger#408
Conversation
- possible multiple startup elements - all the assembly binding redirects from child assemblies go into the first <dependentAssembly> element - ilrepack crashes if child assembly config has comments - (cosmetic) if the assembly is merged in, the redirect entry for it is no longer needed
|
to validate binding redirects, look into https://nuget.org/packages/checkbinarycompat |
|
In an ideal world everything should be fully processed and then semantically merged (i.e. if one redirect is -1.1.0.0 and the other is -1.2.0.0 the second is the right one to use). |
| { | ||
| var startups = root.Elements("startup"); | ||
|
|
||
| foreach (var s in startups.Skip(1)) |
There was a problem hiding this comment.
do you need a .ToArray() here? or does it work fine?
There was a problem hiding this comment.
Yup, you might be right.
My test case had 2 startup elements :)
There was a problem hiding this comment.
wanna send a follow up or file a bug? or verify that it works fine
| } | ||
| } | ||
|
|
||
| if (binding.Elements().Count() == 0) |
|
Thank you! You fork was very helpful when reconciling multiple forks and PRs back into the main repo. |
|
Published 2.0.44 |
Hi,
You might remember me as owner of one of "more alive" forks back in the day.
I see that config merging got some attention in the past years which is great. But, fundamentally, I don't think that any non-schema-aware merging can be 100% successful.
This adds a little bit of awareness where it hurts me the most.
Trying the current implementation, I've encountered following issues:
For us it's caused by different attributes
If we run unmerged .exe only it's startup config is used -> the rest can be dropped when merging
In theory, merging startup sections isn't so great either: e.g. primary .exe requires 4.8.1, merged-in 4.6.2, you end up with both 4.8.1 AND 4.6.2 which will cause problems
So far, the fix is to at least drop multiple startups because that's bad merge output
Critical issue as it totally breaks redirections and causes merged .exe to crash
Not merging is the answer
But having stuff that cannot be merged based on name causes problems with the rest of the algorithm, so HashSet added to compensate
Trivial fix, also fixes An error occurred while merging DLLs #401
Doesn't cause any real problems, so is cosmetic
But annoying if you have large projects and trying to figure out why your merged exe has problem with redirects