Skip to content
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

Update modules mvids before rewriting an assembly #16

Merged
merged 2 commits into from
Mar 14, 2016

Conversation

ESolovova
Copy link
Contributor

When we modify an assembly we should also update its mvid. Otherwise some coverage/profiling tools can be confused by having different assemblies with similar mvids. Please see, for instance: https://dotnettools-support.jetbrains.com/hc/en-us/community/posts/205988489-Unit-Testing-works-but-dotCover-failes-to-run-test-Unrecoverable-Error

@@ -122,6 +122,9 @@ public IAssemblyLoader Rewrite(string path)
WriteSymbols = hasSymbols
};

foreach (var module in assembly.Modules)
module.Mvid = Guid.NewGuid();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, please fix the indenting to match the current code base convention of 4 spaces. Also, I prefer to use curly braces (each on a new line) for every for each body, including the single line ones.

@vanderkleij
Copy link
Owner

Thanks for the pull request!

The mvid (module version id) is something that I wasn't aware of and seems quite interesting. It might also be related to #1. I will do some testing shortly.

vanderkleij added a commit that referenced this pull request Mar 14, 2016
Update modules mvids before rewriting an assembly
@vanderkleij vanderkleij merged commit 638c86c into vanderkleij:master Mar 14, 2016
@harigutta
Copy link

@ESolovova Thank you so much for the looking into this.

@vanderkleij Your library is awesome.

This will help our company getting coverage for lots of legacy code.

@vanderkleij
Copy link
Owner

Thanks for the kind words! Glad you like it 👍

Version 0.5.25 that includes this fix is available on nuget now.

@harigutta
Copy link

@vanderkleij Thank you.

I reported an issue we ran into when using with NUnit.
#17

Currently, I'm building locally to get around it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants