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

Kill Newtonsoft.Json dependency #237

Closed
Pliner opened this issue Jul 14, 2014 · 22 comments
Closed

Kill Newtonsoft.Json dependency #237

Pliner opened this issue Jul 14, 2014 · 22 comments

Comments

@Pliner
Copy link
Member

Pliner commented Jul 14, 2014

Hi!

I have a small trouble after last update of newtonsoft.json to 6 version.

A lot of libraries use 4.5 version of this library.

What do you think about removing serialization dependency from easynetq and give this responsibility to user as it's done with IEasyNetQLogger?

Also there is an option to do an built in implementation without any dependencies.

@micdenny
Copy link
Member

IMHO it is not a bad idea, but we must consider that instead of logging, serializing is a mandatory part, so we could think about using by defaul the DataContractSerializer (or XmlSerializer), and let the user change it like for IEasyNetQLogger, but we could also consider to simply add a separated nuget package, like "EasyNetQ.Newtonsoft.Json", to maintain the friction-less of EasyNetQ installation, for whom do not care about external dependencies, then make some kind of dynamic dependency resolution with MEF or manually using Assembly namespace, that by default tries to load the dll that contains implementation of ISerializer, if there's not dll it fallbacks to XmlSerializer or DataContractSerializer (already included in .net) or at the end loads the overloaded one if the developer as force one while creating the bus.

What do you think?

@mikehadlow
Copy link
Contributor

Hi Yuri,

I'd be against removing serialization. It would put a big boundary for
users first coming to EasyNetQ, "first write a serializer", and I don't
like the idea of having to install multiple NuGet packages (which wouldn't
really solve the problem). IMHO, the best way forward would be to ILMerge
Newtonsoft.Json into EasyNetQ so it's no longer a separate dependency. It's
not something I've much experience with doing though. Someone already has
an EasyNetQ (Bundle) NuGet available with ILRepack. Maybe we just merge
their changes?

Mike

On Mon, Jul 14, 2014 at 2:52 PM, Yury Pliner notifications@github.com
wrote:

Hi!

I have a small trouble after last update of newtonsoft.json to 6 version.

A lot of libraries use 4.5 version of this library.

What do you think about removing serialization dependency from easynetq
and give this responsibility to user as it's done with IEasyNetQLogger?

Also there is an option to do an built in implementation without any
dependencies.


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237.

@micdenny
Copy link
Member

I like the idea of ILMerge. I just have experience with red-gate smart assembly, that also can merge dependencies in one binary.

I don't know how ILMerge solve the problem of having two version of the same assembly in the same application domain, maybe is not a problem for JIT.

Edit: Yes, it is possibile to internalize assemblies to avoid conflicts:

ILMerge /t:library /internalize /out:Deploy/NHibernate.dll NHibernate.dll Castle.DynamicProxy2.dll

some other readings:

@mikehadlow
Copy link
Contributor

Effectively ILRepack creates a single assembly out of several separate
ones. You can specify that certain assemblies be renamed as internal so
they have no public interface and so no namespace conflicts with the
consuming application.

On Mon, Jul 14, 2014 at 3:13 PM, Michael Denny notifications@github.com
wrote:

I like the idea of ILMerge. I just have experience with red-gate smart
assembly, that also can merge dependencies in one binary.

I don't know how ILMerge solve the problem of having two version of the
same assembly in the same application domain, maybe is not a problem for
JIT.


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237#issuecomment-48904309.

@mikehadlow
Copy link
Contributor

I just tried out ILRepack. It seems to work. I've been able to create a
bundled version of EasyNetQ with internalized Newtonsoft.Json and
RabbitMQ.Client.

I'd quite like to go with this and see if it works for people. What do you
guys think?

Mike

On Mon, Jul 14, 2014 at 3:28 PM, Mike Hadlow mike@suteki.co.uk wrote:

Effectively ILRepack creates a single assembly out of several separate
ones. You can specify that certain assemblies be renamed as internal so
they have no public interface and so no namespace conflicts with the
consuming application.

On Mon, Jul 14, 2014 at 3:13 PM, Michael Denny notifications@github.com
wrote:

I like the idea of ILMerge. I just have experience with red-gate smart
assembly, that also can merge dependencies in one binary.

I don't know how ILMerge solve the problem of having two version of the
same assembly in the same application domain, maybe is not a problem for
JIT.


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237#issuecomment-48904309
.

@micdenny
Copy link
Member

I think people will need to manually remove the other dependencies that were added by easynetq before. You could write this note inside the nuget package:

<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd">
  <metadata>
  ...
     <releaseNotes>adasdda asda asdasd</releaseNotes>
  ...
  </metadata>
</package>

For me is 👍

@StevenBonePgh
Copy link
Contributor

I like this approach as well. Thanks, guys!

@jonnii
Copy link
Contributor

jonnii commented Jul 14, 2014

Another option would be to use simplejson and let people opt in to the newtonsoft.json dependency. simplejson is surprisingly good and it just installs as a .cs file in your project.

SimpleJson: https://github.com/facebook-csharp-sdk/simple-json

@micdenny
Copy link
Member

Nice idea @jonnii but does SimpleJson has also all the attributes that newtonsoft has? Because otherwise could become a big breaking changes for who is using those attributes (not me 😄), yes, of course they can set newtonsoft serializer again while creating the bus, but the ones that already has code in production, well, dunno if they're will so happy.

Actually yes, this preserve the easy-peasy idea of mike, for who is approaching to EasyNetQ for the first time, and will remove 489 KB of dll (952 KB with xml doc).

Is there any performance comparison out there?

What do you think @mikehadlow ?

@mikehadlow
Copy link
Contributor

Hmm, changing the serializer has the potential to break people's work in
unexpected ways. I'm not sure it I want to do that initially.

On Mon, Jul 14, 2014 at 4:33 PM, Michael Denny notifications@github.com
wrote:

Nice ide @jonnii https://github.com/jonnii but does SimpleJson has also
all the attributes that newtonsoft has? Because otherwise could become a
big breaking changes for who is using those attributes (not me [image:
😄]), yes, of course they can set newtonsoft serializer again while
creating the bus.

Actually yes, this preserve the easy-peasy idea of mike, for who is
approaching to EasyNetQ for the first time, and will remove 489 KB of dll
(952 KB with xml doc).

Is there any performance comparison out there?

What do you think @mikehadlow https://github.com/mikehadlow ?


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237#issuecomment-48915098.

@jonnii
Copy link
Contributor

jonnii commented Jul 14, 2014

If you're worried about performance then you shouldn't be using newtonsoft.json or any json serializer but using something like msgpack or protobufs etc...

I'm 👎 on interning the rabbitmq client, I think that's too opaque.

@mikehadlow
Copy link
Contributor

OK, how about we initially go for interning (is that the word?)
Newtonsoft.Json, see how that goes, then think again about RabbitMQ.Client?
I get your reluctance to intern RabbitMQ.Client, having a separate
dependency is much less likely to be a problem for most people.

On Mon, Jul 14, 2014 at 4:47 PM, Jonathan Goldman notifications@github.com
wrote:

If you're worried about performance then you shouldn't be using
newtonsoft.json or any json serializer but using something like msgpack or
protobufs etc...

I'm [image: 👎] on interning the rabbitmq client, I think that's too
opaque.


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237#issuecomment-48916964.

@micdenny
Copy link
Member

I agree to avoid interning RabbitMQ.Client, because if a bug-fix is released by RabbitMQ team you need to wait a new EasyNetQ package, instead now you can update the rabbitmq.client assembly and often that it sufficient.

👍 to initially internalize just Newtonsoft.Json

@Pliner
Copy link
Member Author

Pliner commented Jul 14, 2014

Oh... Very hot discussion :) Thanks guys for answers!
I must confess I have never used ILRepack before but it seems very interesting. And of course it's a better way thаn have a clone of EasyNetQ with downgraded Newtonsoft.Json:)

👍 to internalize just Newtonsoft.Json

@jeffdoolittle
Copy link
Contributor

I agree with Michael. Prefer to IL Merge just the json serializer, not the
rabbit mq client.

On Mon, Jul 14, 2014 at 9:01 AM, Michael Denny notifications@github.com
wrote:

I agree to avoid interning RabbitMQ.Client, because if a bug-fix is
released by RabbitMQ team you need to wait a new EasyNetQ package, instead
now you can update the rabbitmq.client assembly and often that it
sufficient.

[image: 👍] to initially internalize just Newtonsoft.Json


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237#issuecomment-48919012.

@mikehadlow
Copy link
Contributor

OK, I'll look at getting a new version of EasyNetQ with interned
Newtonsoft.Json out today.

On Mon, Jul 14, 2014 at 5:06 PM, Jeff Doolittle notifications@github.com
wrote:

I agree with Michael. Prefer to IL Merge just the json serializer, not the
rabbit mq client.

On Mon, Jul 14, 2014 at 9:01 AM, Michael Denny notifications@github.com
wrote:

I agree to avoid interning RabbitMQ.Client, because if a bug-fix is
released by RabbitMQ team you need to wait a new EasyNetQ package,
instead
now you can update the rabbitmq.client assembly and often that it
sufficient.

[image: 👍] to initially internalize just Newtonsoft.Json


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237#issuecomment-48919012.


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237#issuecomment-48919596.

@Pliner
Copy link
Member Author

Pliner commented Jul 15, 2014

Many thanks, Mike!

@mikehadlow
Copy link
Contributor

OK, it's all done. Verison 0.35.0 onwards has Newtonsoft.Json merged.

On Tue, Jul 15, 2014 at 10:47 AM, Yury Pliner notifications@github.com
wrote:

Many thanks, Mike!


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237#issuecomment-49010949.

@micdenny
Copy link
Member

great job @mikehadlow !! Many thanks!

@Fydon
Copy link
Contributor

Fydon commented Jul 24, 2014

@mikehadlow you said that with ILRepack

"you can specify that certain assemblies be renamed as internal so they have no public interface and so no namespace conflicts with the consuming application."

When I switched the the new ILRepack version of EasyNetQ, it doesn't pick up my JsonConstructor attributes and so deserialisation breaks, which I assume is because of the namespace differences between the two Newtonsoft.Json libraries. Is it possible to have a separate NuGet package that doesn't use ILRepack or is there a simple way to have JsonConstructor work?

@mikehadlow
Copy link
Contributor

It's because EasyNetQ now references its internal Newtonsoft.Json assembly
rather than the assembly you are using. Namespace and version are probably
identical, but it won't pick up your attributes.

The easy way to make it work is to recreate the JsonSerializer:

https://github.com/mikehadlow/EasyNetQ/blob/master/Source/EasyNetQ/JsonSerializer.cs

... and register it with EasyNetQ as a new component:

https://github.com/mikehadlow/EasyNetQ/wiki/Replacing-EasyNetQ-Components

I hope this helps,
Mike

On Thu, Jul 24, 2014 at 12:03 PM, Fydon notifications@github.com wrote:

@mikehadlow https://github.com/mikehadlow you said that with ILRepack

"you can specify that certain assemblies be renamed as internal so they
have no public interface and so no namespace conflicts with the consuming
application."

When I switched the the new ILRepack version of EasyNetQ, it doesn't pick
up my JsonConstructor attributes and so deserialisation breaks, which I
assume is because of the namespace differences between the two
Newtonsoft.Json libraries. Is it possible to have a separate NuGet package
that doesn't use ILRepack or is there a simple way to have JsonConstructor
work?


Reply to this email directly or view it on GitHub
https://github.com/mikehadlow/EasyNetQ/issues/237#issuecomment-49994283.

@Fydon
Copy link
Contributor

Fydon commented Jul 24, 2014

Thank you for the quick response. Yes that solved 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

No branches or pull requests

7 participants