modelica-3rdparty / msgpack-modelica

A MessagePack implementation as a Modelica package
BSD 2-Clause "Simplified" License
6 stars 4 forks source link

Add Windows binaries of msgpack for use with Dymola and SimulationX #6

Closed tbeu closed 10 years ago

tbeu commented 10 years ago

Compiling msgpack on Windows really is a pain. This patch provides binaries in order to make user's life easier.

sjoelund commented 10 years ago

Binaries do not belong in source packages though...

tbeu commented 10 years ago

Sorry, they should go to Resources/Library/winxx.

tbeu commented 10 years ago

Hm, actually the binaries location is correct according to MLS. So where is the problem in saving Dymola / SimulationX users 5 hours of trial'n'error of how to compile the required msgpackc using VS? I also can provide sources if you prefer - this is the way how Modelica_DeviceDrivers does it (by both providing source and libs).

sjoelund commented 10 years ago

It is because source packages should never contain binaries. They are located elsewhere, typically compiled in when making a binary installer. That way binaries are never out of sync and do not bloat the repository. Imagine for every commit changing a C-file you have to provide updated binary files for all platforms. In git you would be unable to treat merging in a good way as well...

tbeu commented 10 years ago

I do appreciate your developer point of view to have clean and tiny repos without any redundancy. But imagine an average Dymola (on Windows) user that wants to try your package. C code is usually hidden to the user since focus is on modeling. Thus I promise that he/she will struggle sooner or later to get it running. VS compilation of msgpackc (from official sources) is not straight forward. It fails if he/she tries to do so out of the box.

tbeu commented 10 years ago

Maybe @dietmarw can fork the package to modelica-3rdparty to make it more known and to allow Win binaries for better ease of use.

dietmarw commented 10 years ago

Sure I can fork it but I agree with @sjoelund on that there is no place for binaries in the repo. Those should be put in either an external location or upon release a download version containing the binaries can be created (See also https://github.com/blog/1547-release-your-software where the binary assets are explicitly mentioned.). So that way you would have to create new binaries for new releases but not for every commit which makes much more sense.

tbeu commented 10 years ago

OK.

sjoelund commented 10 years ago

Awww. The branch was deleted. I had considered trying to make a github release (dietmar's link) with the binaries included to see what would happen.

tbeu commented 10 years ago

Branch is restored now.

sjoelund commented 10 years ago

https://github.com/sjoelund/msgpack-modelica/releases now contains a 0.1 version released with include files and binaries for win/linux 32, and 64-bit. I am a bit unsure why the .lib files are needed. Is it because the dll's do not expose everything needed to link on Windows? The dll and so-files are small in comparison.

@dietmarw I guess Impact will not pull the version with the binaries from github, so maybe I will open an issue over there. I also need to check if modelica-3rdParty will contain the additional assets or only the tag (the git repository does not seem to save the additional files in them). If things get messed up, maybe we move to modelica-3rdParty as the main repository.

dietmarw commented 10 years ago

Yes the binaries added to a release are not yet considered by impact. So filing a ticket is the way to go here.

tbeu commented 10 years ago

As usual, libs for Win users of Dymola and dlls for SimulationX.

sjoelund commented 10 years ago

OK, so the modelica-3rdParty fork does not seem to get the assets from the master. Should we move the repository directly into 3rdParty?

tbeu commented 10 years ago

I would wait until #7 is fixed. Otherwise it is partially useless on Win.

tbeu commented 10 years ago

OK, can be closed now. Header files were still missing,by the way.

sjoelund commented 10 years ago

What do you mean missing? They are in the binaries package in order to not conflict with user-installed libmsgpackc files.

tbeu commented 10 years ago

Yeah, right they are there. Did not check binaries.tar.gz before.