alanmcgovern / Mono.Nat

UPNP and NAT-PMP port forwarding for .NET
https://github.com/mono/Mono.Nat
MIT License
163 stars 156 forks source link

Nterry fix bitconvert #2

Closed nterry closed 5 years ago

strich commented 10 years ago

Looks like the current Mono.NAT maintainer is AWOL?

nterry commented 10 years ago

Seems so. You can get a vastly improved and regularly maintained version at nuget.

https://www.nuget.org/packages/Mono.Nat/

strich commented 10 years ago

Ah I see. Nice. I might rebase and fork from you then - I presume your GitHub fork is the latest as at NuGet? It would be nice to get all this back into the main repo though. I wonder if we can get on to the Mono team to add maintainers?

nterry commented 10 years ago

It is, however there are some changes coming down the pike to make the lib compatible with Android and IOS and others (In development, PR coming soon). It doesn't currently work except for PC (Mac and linux probably, but I haven't tested them yet)

On Tue, Jan 28, 2014 at 7:09 PM, Scott Richmond notifications@github.comwrote:

Ah I see. Nice. I might rebase and fork from you then - I presume your GitHub fork is the latest as at NuGet? It would be nice to get all this back into the main repo though. I wonder if we can get on to the Mono team to add maintainers?

Reply to this email directly or view it on GitHubhttps://github.com/mono/Mono.Nat/pull/2#issuecomment-33550073 .

strich commented 10 years ago

Ah great. I intend to utilize it in a PC, Mac & Linux commercial product very soon. So I'll definitely be testing those platforms a lot.

nterry commented 10 years ago

Sweet. Ill take all the testing I can.

On Tue, Jan 28, 2014 at 7:16 PM, Scott Richmond notifications@github.comwrote:

Ah great. I intend to utilize it in a PC, Mac & Linux commercial product very soon. So I'll definitely be testing those platforms a lot.

Reply to this email directly or view it on GitHubhttps://github.com/mono/Mono.Nat/pull/2#issuecomment-33550335 .

alanmcgovern commented 10 years ago

Hey,

In general I have no problems with the changes, however porting from bitcoverter is completely unnecessary. Bitcoverter is 100% functional in mono and this library was actually written with mono as the primary target ;)

Unless data2byte has a tangible benefit it'd be great to not rely on nuget for external binaries. This will make life simpler for people on Linux who package and release mono.nat.

Secondly, unless there's a specific reason, we should keep the make files and related cruft. Without those it won't be possible to release Linux packages.

nterry commented 10 years ago

I wasn't aware that bit convert was fixed for mono. The docs haven't been updated to reflect this. I'll go ahead and revert the relevant commits. I was also unaware that the make files were for the packaging. I simply thought they were leftovers as this package hasn't been maintained in a long time. I'll review my pull Rx and get it fixed. Thanks for getting back Alan.

Just as a side note. Mono.nat is currently continuously deployed to nuget. I'm not sure it belongs in the official build of mono.

Also, the docs need updating to reflect that bit convert now works in mono. What version was this added in? I looked at the data convert code and noticed that it uses unsafe code. On Jan 29, 2014 6:57 AM, "Alan McGovern" notifications@github.com wrote:

Hey,

In general I have no problems with the changes, however porting from bitcoverter is completely unnecessary. Bitcoverter is 100% functional in mono and this library was actually written with mono as the primary target ;)

Unless data2byte has a tangible benefit it'd be great to not rely on nuget for external binaries. This will make life simpler for people on Linux who package and release mono.nat.

Secondly, unless there's a specific reason, we should keep the make files and related cruft. Without those it won't be possible to release Linux packages.

Reply to this email directly or view it on GitHubhttps://github.com/mono/Mono.Nat/pull/2#issuecomment-33585979 .

alanmcgovern commented 10 years ago

I'm not sure what documentation you were reading which suggested BitConverter didn't work. It hasn't changed in years besides a few minor things.

Mono.Nat just lives inside the mono organization in github, it doesn't actually ship inside mono itself. Having it on nuget is pretty awesome, so thanks for doing that!

nterry commented 10 years ago

@alanmcgovern Not only is it on nuget, but it is continuously integrated and deployed there.

The big changes that ware in the pike are Android and IOS support. There are a few calls in the WCF stuff that aren't supported on those platforms. I've identified most, if not all of them for Android and I am in process of adding platform-detection and alternate code path to invisibly handle that. IOS will hopefully come after that.

The only issue is that I can only use the business edition of Xamarin for a limited time, and the Business edition is the lowest that supports WCF. Is there any way you know of that will allow for WCF on the free edition. (The IL size limit shouldn't be an issue for testing).

Also, the documentation i am referring to is here:

http://www.mono-project.com/Mono_DataConvert

alanmcgovern commented 10 years ago

I think the comment at the bottom of that is saying only that the bitconverter in the .NET framework doesn't do endian conversions so you have to be careful about bitswapping. This was already taken care of in the library though so it wasn't an issue.

I would have a preference to keep nuget out of the build chain for this as many linux distros have a policy of always building from source and nuget binaries make that harder than it needs to be, especially if the only difference is changing a 'BitConverter.GetBytes' call into a 'NTB.GetBytes' call.

As for WCF - Mono.Nat doesn't use anything from WCF. It uses mscorlib, System.dll and System.Xml.dll. All of those should work fine with the Starter edition of the xamarin products.

nterry commented 10 years ago

Seems like a reasonable use case. I feel that the source tree is a little cluttered. We should work together to find a good solution to fit both use cases. (The make solution, and the CI process to Nuget)