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

Weaver Proposal: Fixing duplicate mscorlib references #43

Closed
SimonCropp opened this issue Feb 17, 2013 · 18 comments
Closed

Weaver Proposal: Fixing duplicate mscorlib references #43

SimonCropp opened this issue Feb 17, 2013 · 18 comments

Comments

@SimonCropp
Copy link
Member

No description provided.

@davkean
Copy link

davkean commented Feb 18, 2013

The first issue as raised here, http://visualstudiogallery.msdn.microsoft.com/b0e0b5e9-e138-410b-ad10-00cb3caf4981, is caused by the C# compiler outputting a reference to the wrong mscorlib ("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089") when outputting (what we call) an old-style portable assembly. There should only be one reference in this particular case, and that's to "mscorlib, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e, Retargetable=Yes".

The second issue, as raised here, https://oxyplot.codeplex.com/discussions/431821, is a completely different issue. It is completely normal that there will be two mscorlib's (or even System.Core, System, etc) when a .NET Framework project consumes a portable library. This is normally a no-op as the runtime will automatically resolve them to the same assembly. What is likely happening, is that the WPF designer is not handling portable libraries in this particular context. Usually most applications and libraries don't need to handle portable libraries any differently as the runtime will automatically handle it - but in this case, the designer is calling LoadFile - in which case it is telling the runtime that its signing up to handle them. We fixed a bunch of these issues before we shipped, but it looks like one slipped through the cracks. I've filed a bug internally to track this.

@SimonCropp
Copy link
Member Author

So do you want me to fix the 1st or 2nd or both?

Would it be possible to get some binaries that have each of these problems? attached to this issue is fine

And what do i name the weaver? "DavToldMeToBuildIt"?

@distantcam
Copy link
Member

What about "IThoughtDotNetWouldSolveDllHellButInsteadItJustMadeItWorseSoHereIsAFixForTheStupidity"?

@SimonCropp
Copy link
Member Author

Yeah I know. But if I can help them out without too much effort it seems
worth it

On 18/02/2013, at 6:18 PM, Cameron MacFarland notifications@github.com
wrote:

What about
"IThoughtDotNetWouldSolveDllHellButInsteadItJustMadeItWorseSoHereIsAFixForTheStupidity"?


Reply to this email directly or view it on
GitHubhttps://github.com//issues/43#issuecomment-13709133.

@davkean
Copy link

davkean commented Feb 18, 2013

You can only fix just the first one. Removing references in the second case won't help it.

If you changed to remove unused references (which I suggested on Twitter), then you call it just that. RemoveUnusedReferences. If you are going to make it just remove the mscorlib, 4.0.0.0 reference, then you need to make sure that you only do it when the TargetFrameworkAttribute has an identifier of ".NETPortable" and (exactly) Version=v4.0. Otherwise, you will break a lot more.

@SimonCropp
Copy link
Member Author

so can i get an assembly that need to be fixed? the simpler the assembly the better.

@tibel
Copy link

tibel commented Feb 18, 2013

I only have an assembly for the second issue, not the first one.
You can download OxyPlot.Core NuGet package or if you want a really simple one see the comment from objo.

@davkean
Copy link

davkean commented Feb 18, 2013

Can't work out how to attach files here. To create a module that exhibits the problem, do the following:

  1. Create a blank portable class library (be sure to accept the default platforms)
  2. Unload project, edit it, and change OutputType to module.
  3. Reload the project and build

Looking at the output, this is going to be harder than I thought:

  • You can't look for unused references as it appears that both references are actually used
  • You can't look at TargetFrameworkAttribute as it only applies to an assembly, and the result is only a module

Therefore any such rule, must be very careful that it is only turned on in the specific case above. Let me repeat what I said, the second issue will not be fixed by removing one of the duplicated mscorlibs. These duplicated mscorlibs are to be expected in this case, and the second issue is not related to them.

@SimonCropp
Copy link
Member Author

@davkean ok I am going to have a try at replicating/fixing this tonight (8 hours time) I will let you know how I go

@SimonCropp
Copy link
Member Author

@davkean how do i look inside a netmodule with a decompiler?

@SimonCropp
Copy link
Member Author

no response so closing

@icnocop
Copy link

icnocop commented Aug 14, 2013

I would also like to have this feature.

Unfortunately, this issue has since been closed without a resolution, so I am just trying to continue the discussion.

You should be able to reproduce the issue without having to change the OutputType to module, just leave it as Library.

You can open the assembly in Telerik's JustDecompile and then expand the references node and you will see:

mscorlib, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e
mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089

System.Core, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e
System.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089

Thank you.

@SimonCropp SimonCropp reopened this Aug 14, 2013
@SimonCropp
Copy link
Member Author

@icnocop sounds great. do you want to start a fody addin that performs this clean up?

@icnocop
Copy link

icnocop commented Aug 14, 2013

@SimonCropp, I will try.

I've used NotifyPropertyWeaver in the past, but am new to Fody, so this may be a good starting point. :)

Thank you.

@SimonCropp
Copy link
Member Author

@icnocop well the hardest part is picking a cool name and icon. the rest is easy.

@distantcam
Copy link
Member

What about RedundantRedundantizR.Fody?

@SimonCropp
Copy link
Member Author

deduplicator :)

@icnocop
Copy link

icnocop commented Aug 15, 2013

I created RemoveReference.Fody here: https://github.com/icnocop/RemoveReference.Fody

There are still several more things to do:

  1. Make RemoveReferenceAttribute available to the assembly being processed.
  2. Test with the actual NuGet package.
  3. Update README.md with help documentation
  4. Integration with a continuous build server
  5. Deploy to NuGet Repository
  6. Update Fody to reference new addin

Thank you.

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

No branches or pull requests

5 participants