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

DLL Imports/Exports and fixes for Visual Studio Builds #958

Merged
merged 5 commits into from Aug 14, 2016

Conversation

abeham
Copy link
Contributor

@abeham abeham commented Aug 12, 2016

Two changes are critical, I don't know if you want to make them:

  • src/cliquer/graph.c was renamed to src/cliquer/graph2.c
  • src/heap.c was renamed to src/heap2.c

Otherwise in Visual Studio (2013) both heap.c and heap.cc build to the same .o file and overwrite each other.

Other than that:

  • I included PRPACK in makefile's echosources target so that these are part of the visual studio project
  • I added MSDOS to the compiler's preprocessor definitions (otherwise there is a compile error)
  • I removed the __BEGIN_DECL and __END_DECL directives from each header file and put them in igraph_decl.h which is included in all .h files that had these directives
  • I added DECLDIR directive to all "public" functions which is either empty, or when Visual Studio is used as compiler is __declspec(dllexport) when building with IGRAPH_EXPORTS flag or __declspec(dllimport) when building without the flag

@ntamas
Copy link
Member

ntamas commented Aug 12, 2016

Thanks for your work on this, it looks great!

I would prefer renaming src/cliquer/graph.c to src/cliquer/cliquer_graph.c and src/bliss/heap.cc to src/bliss/bliss_heap.cc instead - feels more readable than graph2.c and heap2.c.

Also, the AppVeyor build has failed for one of the build configurations because MSVC does not seem to include stdint.h:

https://ci.appveyor.com/project/ntamas/igraph/build/1.0.55/job/6x86orpkpp7d0xyf#L3494

To be honest, I don't quite understand why it fails - more precisely, I don't understand how the other three build configurations on AppVeyor could succeed if stdint.h is missing. It can even happen that prpack_solver.cpp does not need stdint.h at all. Can you try recompiling this without stdint.h on your machine?

@abeham
Copy link
Contributor Author

abeham commented Aug 13, 2016

Yes, very strange that only one failed. prpack_solver.cpp builds fine in VS2013 without stdint.h so I removed it. I also made the requested filename changes.

@abeham
Copy link
Contributor Author

abeham commented Aug 13, 2016

Okay, I think that did it. I had to avoid including stdint.h also for MINGW builds. But reusing pstdint.h was not the way to go...

@ntamas
Copy link
Member

ntamas commented Aug 14, 2016

Thanks, now I think this one is ready for merging.

@ntamas ntamas merged commit ac88d12 into igraph:master Aug 14, 2016
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

Successfully merging this pull request may close these issues.

None yet

2 participants