rpgmaker / NetJSON

Faster than Any Binary? Benchmark: http://theburningmonk.com/2014/08/json-serializers-benchmarks-updated-2/
MIT License
231 stars 29 forks source link

Migrated solution to Visual Studio 2017 #168

Closed NimaAra closed 6 years ago

NimaAra commented 7 years ago

This was done to address: #165.

Note there are currently the following tests failing:

The first one has always been failing for me even on the old solution file and the second one I cannot validate as I only have VS2017 installed on my machine and opening the old solution file prompts for a migration.

I have also included the Nuget-Pack.bat to which you can pass the version number to release which generates 2 NuGet packages one with the source and pdb files included. You can run as: NuGet-Pack 1.2.3.4

Kindly have a look and validate.

rpgmaker commented 7 years ago

Thanks I will review them

rpgmaker commented 7 years ago

I will verify the two tests myself. I believe they were working fine

rpgmaker commented 7 years ago

I can't compile it in my 2017 enterprise. The (net35) is not working for me, do I need to download anything to make it work?

rpgmaker commented 7 years ago

image

NimaAra commented 7 years ago

Which version of VS and .NET Core are you using? image

rpgmaker commented 7 years ago

15.2 Enterprise

rpgmaker commented 7 years ago

image

NimaAra commented 7 years ago

Investigating...

NimaAra commented 7 years ago

That's strange, I just did a clone on another machine and it was built just fine. Just to confirm, you are trying to build: https://github.com/NimaAra/NetJSON.git right?

Also can you clone one of my other projects which has a similar structure and see if you can build that? e.g. https://github.com/NimaAra/Easy.MessageHub.git

rpgmaker commented 7 years ago

I cloned https://github.com/NimaAra/NetJSON.git locally. Do you have .net 4.7 installed like I do?

NimaAra commented 7 years ago

No, I don't, let me try it with .NET 4.7 then.

NimaAra commented 7 years ago

Yep, it works fine with .NET 4.7 installed as well. Can you make sure you have the .NET 3.5 SDK installed?

rpgmaker commented 7 years ago

The error I got was for both 1.6 standard and 3.5 framework. I will check if I have the sdk.

NimaAra commented 7 years ago

I believe that's because you are missing: C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v3.5\Profile\Client

Try removing this in the NetJSON.csproj:

  <PropertyGroup>
    <!-- This is a workaround for: https://github.com/Microsoft/msbuild/issues/1333 -->
    <FrameworkPathOverride Condition="'$(TargetFramework)' == 'net35'">C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v3.5\Profile\Client</FrameworkPathOverride>
  </PropertyGroup>

We will need this workaround however let us see it solves the build at least for net35.

rpgmaker commented 7 years ago

I am still getting the same error after removing it.

image

image

NimaAra commented 7 years ago

That's weird, I cannot think of anything else. can you check the output of: dotnet --info?

rpgmaker commented 7 years ago

I will try it out. Thanks

rpgmaker commented 7 years ago

image

NimaAra commented 7 years ago

Hmm.. that's also identical to mine, no idea why it doesn't build. Did you find out whether you have .NET 3.5 SDK?

rpgmaker commented 7 years ago

Did not try it. I will search for it

rpgmaker commented 7 years ago

i have 3.5 installed on my machine. I don't have the profile folder, but i have the rest of the other 3.5 paths.

I will try to test it and figured out what my machine could be missing.

Thanks,

NimaAra commented 7 years ago

Are you sure you have the .NET 3.5 SDK and not the runtime? FYI, HERE's the SDK.

rpgmaker commented 7 years ago

I downloaded it but did not install it. Because I was not sure if it will work for my windows 10

rpgmaker commented 7 years ago

I will install it and see if it helps.

Thanks

rpgmaker commented 7 years ago

I installed it and it made no different :)

NimaAra commented 7 years ago

Not sure what else to suggest :-(

rpgmaker commented 7 years ago

Don't worry I will figure something out.

rpgmaker commented 7 years ago

I do notice something weird about my property page for my solution. The target framework is blank

image

NimaAra commented 7 years ago

Yeah that's fine, it is blank for me as well. this is due to the project targeting multi platforms I believe the VS tooling will be updated to have a more clear support for such configuration.

rpgmaker commented 7 years ago

I am thinking that I will just create a new project and manually copy the files to it and see if that works. I am not able to get it to work with net35 even though the sdk is installed and everything works fine for .net 3.5 project in the old solution.

NimaAra commented 7 years ago

Sure give it a try. The old solution is not the same as the new. In the new solution the projects have been migrated to the new cjsproj format. I would be interested to see if you could get a VM with a fresh install of VS2017 or another machine to rule out any issue with your system.

buildcenter commented 7 years ago

howdy. do you have net35 installed? quick powershell check Get-WindowsOptionalFeature -Online -FeatureName NetFx3 will tell you.

I normally do my builds in a hyper v vm, just to avoid this sort of thing...

NimaAra commented 7 years ago

@buildcenter Just ran it and got:

FeatureName      : NetFx3
DisplayName      : .NET Framework 3.5 (includes .NET 2.0 and 3.0)
Description      : .NET Framework 3.5 (includes .NET 2.0 and 3.0)
RestartRequired  : Possible
State            : Enabled
CustomProperties :
                   \FWLink : http://go.microsoft.com/fwlink/?LinkId=296822
rpgmaker commented 7 years ago

image

I got same thing too :)

buildcenter commented 7 years ago

sorry that wasn't of help. perhaps running on a fresh win10 install (in a vm) would be the best recourse.

NimaAra commented 7 years ago

@buildcenter Have you tried building this pull request yourself? does it work for you?

NimaAra commented 7 years ago

@rpgmaker Any updates? FYI, I just made a similar migration to VaderSharp HERE

rpgmaker commented 7 years ago

I tried different things such as installation of 3.5 sdk for 2017. I wanted to test if I was missing a plug-in for it. I am going to try a new project tomorrow.

Sorry again for struggling with this.

Thanks,

NimaAra commented 7 years ago

no problem, I am interested to find out what is causing this.

rpgmaker commented 7 years ago

i figured it out. it was failing because i had registered a path in package source that did not exists and packages were failing to get restored which ultimately caused the failure. I found this out when i tried to move the project to a new project.

Thanks,

rpgmaker commented 7 years ago

The TestSerializeComplexTuple test is failing. It seems like the test cases does not account for the proper reference to NetJSON.Tuple vs System.Tuple.

NimaAra commented 7 years ago

Okay, that's great! One step closer :-)

rpgmaker commented 7 years ago

Much closer indeed :) . One last request. is it possible to have the Nuget-pack.bat automatically use the version of the assemblies to create the nuget package instead of using 1.0.0.0 by default?

Thanks,

NimaAra commented 7 years ago

With regards to the NetJSON.Tuple, I have set it to compile only when it's targeting net35 so if the tests are being run as netstandard then they will be referencing System.Tuple.

To be fair I did not understand why you had to define the Tuple and I wouldn't be surprised if I messed that part up, couldn't you use the System.Tuple? Or even better how about the newly added System.ValueTuple which is actually a struct.

NimaAra commented 7 years ago

With regards to the packaging, I have set it up the way I release my projects.

Here's basically how it is intended to be used:

I have 2 continuous integration jobs running (AppVeyor/Jenkins/TeamCity etc).

  1. which monitors GitHub and runs the test after every commit.
  2. only monitors the release tags that I create on GitHub e.g. v1.2.3

As soon as it detects the tag, it takes the 1.2.3 and appends the build number e.g. 110 and produces the final version number of 1.2.3.110 which is the number passed in to Nuget-pack.bat.

Overall this approach makes the managing of my OSS projects much simpler with no headache at all.

Alternatively, you can get rid of AssemblyInfo.cs and put the versions inside the csproj file just like I have done for let's say THIS project you can then do a simple xml or regex modification of the csproj in the bat file and repalce it with the version you want which basically achieves what you want.

I honestly prefer the first approach as it's much cleaner and automatically triggered by GitHub. You essentially manage all your releases from GitHub directly.

Hope this helps.

rpgmaker commented 7 years ago

It was done to support tuple in .net 3.5. And to avoid writing code inline to avoid tuple for 3.5. you could change the code to conditionally only support tuple when it is not 3.5.

Thanks,

rpgmaker commented 7 years ago

The process make sense I guess. I always wanted to specific the version to indicate major and minor release. Let me think through it.

Thanks,

NimaAra commented 7 years ago

You can still have full control over major.minor. The tag name that you create, dictates the semantic versioning. You can create one tag named v1.2.0 and if you later want to patch it you can create v1.2.1 or if you want to do a minor release you then create v1.3.0. etc.

NimaAra commented 7 years ago

So you want me to disable Tuple support for net35?

rpgmaker commented 7 years ago

Yep. Go ahead and make the changes to only use tuple for version higher than 3.5.

Thanks,