-
Notifications
You must be signed in to change notification settings - Fork 304
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
Add Visual C++ support #63
Add Visual C++ support #63
Conversation
Visual C++ requires the declaration of methods to be exactly the same in the .h and .c files; otherwise it throws C2375. This is fixed by: - Defining PLIST_API_MSC in the public headers (via plist.h) which will add __declspec( dllexport ) to all method definitions in the public headers when compiling with Visual C++ - Ensuring PLIST_API is always set to __declspec( dllexport ) in the private source files when compiling with Visual C++. Using PLIST_API in the public headers does not work, becuase its value depends on HAVE_FVISIBILITY, which may be defined in config.h, which is not a public header.
Add msc_config.h file which contains definitions for inline, __func__ and various definitions that make the Visual C++ compiler be compatible with the libplist codebase
@FunkyM , this PR was updated to work with the latest changes in master. There are a couple of other PRs open here that do pretty much the same thing - e.g. #69, #49, #4 and #5 . Is there anything that is preventing you from merging any of these PRs? It's a pitty to see new "add VS compilation support" PRs pop up every so and then - essentially duplicating the same effort over and over again. |
Hi, I'm more than happy to help, just let me know :) Peter On 2016. 03. 26. 16:15, Frederik Carlier wrote:
|
@backinside a little late, I know, but just wanted to know - I currently publish win32/msvc versions of libimobiledevice in source form (https://github.com/libimobiledevice-win32), and for each of the individual projects, you can also get the binaries through AppVeyor and NuGet (as a CoApp package). I was wondering - if you are looking for a Win32 build of libimobiledevice, would that be enough for you or are you looking for something else? |
@qmfrederik actually I have my own version, that I'm happy with now. But I'm happy to help to add proper support so I don't have to maintain my own version. |
@backinside , I guess that's the point - a lot of us seem to have their own versions, which we are happy with, but not necessarily see maintaining a VC++ build as something we actually enjoy doing :-). Yes, I have 64-bit builds. So I guess my question is - what is it that would make you a VC++ distribution of libimobiledevice consider as "thrustworthy" - i.e. that you would use that one instead of maintaining your own? I'm really trying to understand what it is we can do so instead of duplicating each other's VC++-porting effort, we could try to combine our efforts into coming up with one community-maintained VC++-port. (The ideal way would be to have one of the PRs merged in the actual libimobiledevice repo but that doesn't seem to be happening soon). |
@qmfrederik I don't get ti, why is it so hard to merge the win32 pull request? What do you mean by "doesn't seem to be happening soon" ? |
@backinside well, take for libimobiledevice: libimobiledevice/libimobiledevice#34 (yours), libimobiledevice/libimobiledevice#42, libimobiledevice/libimobiledevice#141, libimobiledevice/libimobiledevice#195, libimobiledevice/libimobiledevice#196, libimobiledevice/libimobiledevice#264, which are all PR's related to VC++ support; the oldest one is about 3 years old yet hasn't been merged. |
Anybody got any feedback on the PRs, on why they are not merged? |
@@ -0,0 +1,628 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file looks like a merge file (.orig) :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks! This PR has been superseded by #76, which shouldn't have this file :)
@qmfrederik , idevicebackup2.exe failed on your last build from appveyor - libimobiledevice-1.2.1.81-windows-x64
But it's ofc imobiledevice related one, not your VS port. to reproduce launch "idevicebackup2 backup DIR" |
Hi,
This pull request adds support for Microsoft Visual C++ in libplist.
It only includes changes in the .h and .c files; the actual project files are not included to keep the libplist repository clean.
A lot of the changes are minor; for example, VC++ is more picky about implicit casts so a lot of cast statements have been added. Similarly, if the declaration in the .h and .c file differ, VC++ complains. Hence a lot of PLIST_API_MSC statements have been added in the .h files.
Care has been taken to make sure that VC++ -specific statements are wrapped in an #ifdef _MVC_VER statement, so that they won't interfere with other compilers on the Windows platform.
I used Travis as a CI server in the upstream repository, to verify compilation still works on the Linux platform. It did, and you can see the result here: https://travis-ci.org/libimobiledevice-win32/libplist/builds/59121180
The upstream repository also contains a sample .travis.yml file that you could use if you wanted CI; it is not included in this pull request.
Similarly, I also tested on a Windows machine using AppVeyor and you can see the results here: https://ci.appveyor.com/project/qmfrederik/libplist/
All feedback is welcome,
Frederik.