TommyKaneko / Sketchup-API-C-Wrapper

A complete set of C++ Wrapper classes for SketchUp C API objects
MIT License
29 stars 8 forks source link

CMake configuration for Visual Studio #42

Closed thomthom closed 6 years ago

thomthom commented 6 years ago

This PR sets up a CMake project to generate a Visual Studio solution. It require at least CMake 3.8.

To generate a solution:

mkdir build
cd build
CMake .. -G "Visual Studio 15 2017 Win64"

If making adjustments to the CMake configuration, update the VS solution by right-clicking the ALL_BUILD project and build only that.

It creates a solution with the following projects:

I also nuked the old manual VS project I created along with the Xcode project.

The main CMake configuration is in root of the project CMakeLists.txt. This refer to various other .cmake files under a cmake directory. I created a .cmake per VS project.

It also adds a third-party directory for GoogleTest and SLAPI. I did not move the existing SLAPI .framework files. Leaving this for the XCode work of the CMake configuration.

After this PR we need to tweak it to allow for generation of Xcode projects.

jimfoltz commented 6 years ago

We are using release 1.8.0 of GoogleTest which is a year and a half old compared to the latest master branch, which builds with no warnings and no need for special config. Just checkout the master branch in git if you'd like to use a more recent although presumably less stable version.

thomthom commented 6 years ago

I stuck with last known release to ensure stability. Wasn't sure what state master was in. But I've bern burned before with unstable masters in various projects. Wasted a lot of time with half baked pre-releases.

  1. feb. 2018 19:56 skrev "Jim Foltz" notifications@github.com:

We are using release 1.8.0 of GoogleTest which is a year and a half old compared to the latest master branch, which builds with no warnings and no need for special config. Just checkout the master branch in git if you'd like to use a more recent although presumably less stable version.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/TommyKaneko/Sketchup-API-C-Wrapper/pull/42#issuecomment-368250922, or mute the thread https://github.com/notifications/unsubscribe-auth/AALvoggJdsTQGLv4p2Z89pTKPzYRtE0Nks5tYFtwgaJpZM4SR3k0 .

thomthom commented 6 years ago

My thinking was that I preferred a known stable release over unknown bleeding edge.

I see no issue with 1.8 being more than a year. It's a mature project.

  1. feb. 2018 19:56 skrev "Jim Foltz" notifications@github.com:

We are using release 1.8.0 of GoogleTest which is a year and a half old compared to the latest master branch, which builds with no warnings and no need for special config. Just checkout the master branch in git if you'd like to use a more recent although presumably less stable version.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/TommyKaneko/Sketchup-API-C-Wrapper/pull/42#issuecomment-368250922, or mute the thread https://github.com/notifications/unsubscribe-auth/AALvoggJdsTQGLv4p2Z89pTKPzYRtE0Nks5tYFtwgaJpZM4SR3k0 .

thomthom commented 6 years ago

Looks like I deleted some files right before making the pull request.

I'll fix that tomorrow.

thomthom commented 6 years ago

Fix the branch - the files wasn't missing, but .gitignore excluded them so I didn't notice while I was building on my machine.

thomthom commented 6 years ago

Tommy, any specific reasons for these files to be ignored? Usually easier to just ignore the build directories.

# Prerequisites
*.d

# Compiled Object files
*.slo
*.lo
*.o
*.obj

# Precompiled Headers
*.gch
*.pch

# Compiled Dynamic libraries
*.so
*.dylib

# Fortran module files
*.mod
*.smod

# Compiled Static libraries
*.lai
*.la
*.a

# Executables
*.exe
*.out
*.app
jimfoltz commented 6 years ago

There should be an option to skip building googletest and the tests in the CMakeLists.txt file

Then the tests and googletest should only be built when this option is set. This way consumers of the wrapper library only get the library without also getting the tests and google tests in their projects.

Proposed:

option(DEVELOPER "Set this when developing the wrapper library itself" OFF)
# ...
include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/SketchUpAPICpp.cmake)
if(DEVELOPER)
    include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/GoogleTest.cmake)
    include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/SketchUpAPITests.cmake)
endif(DEVELOPER)

Here is how I am adding the wrapper library from my program in a separate folder:

include_directories("../Wrapper/include" "${SKETCHUP_HEADERS}")
# CppWrapper
add_subdirectory(../Wrapper ${CMAKE_CURRENT_BINARY_DIR}/Wrapper)
add_executable(skpinfo
    src/main.cpp
    src/output.hpp
    src/print_dictionary.hpp
)
target_link_libraries(skpinfo SketchUpAPICpp ${SKETCHUP_LIBRARY})
thomthom commented 6 years ago

There should be an option to skip building googletest and the tests in the CMakeLists.txt file

Sounds good to me.

thomthom commented 6 years ago

There should be an option to skip building googletest and the tests in the CMakeLists.txt file On second thought, is it really needed? The Visual Studio solution will generate separate projects for GoogleTest and the test project. One can easily build only the Wrapper lib.

Similary with generated Xcode projects from CMake. How much use is it really to make CMake not generate build targets for the tests and GoogleTest?

thomthom commented 6 years ago

(Btw, we can probably sort that out in a separate PR. Better to just get this large PR commited before it gets stale, IMO.)

TommyKaneko commented 6 years ago

There you go, merged. Hope nothing breaks.

jimfoltz commented 6 years ago

Similary with generated Xcode projects from CMake. How much use is it really to make CMake not generate build targets for the tests and GoogleTest?

It’s useful when someone just wants to use the library and not hack on it.

jimfoltz commented 6 years ago

The way the cmake files are written, any project using the library also gets the tests and google test as sub projects.

thomthom commented 6 years ago

I'm not sure I fully understand what you mean by using the library in this context. If you pull the repo as-is, you use cmake to generate the VS solution - and if you only care for the Wrapper lib (the static library) then you compile only that project. And you can link that directly.

Is this related to using add_subdirectory from another project's CMake files? (I'm completely green to CMake workflows.)

TommyKaneko commented 6 years ago

I have to admit I'm a little lost on CMake, so not sure how much I can contribute. I have got as far as figuring out that this: https://www.johnlamp.net/cmake-tutorial-2-ide-integration.html#section-MacOsX

... does not generate an Xcode project. Am slightly missing my old Xcode file at the moment. Would like a steer on how to get Xcode working with this.

jimfoltz commented 6 years ago

It's OK as is. Using include() instead of add_subdirectory() means everything is global and applies to all targets instead of being able to localize includes and definitions, and options for each target. We can tweak it as we go.

I didn't mean to suggest cmake replace the current Xcode project or VS solution, I only meant it would be a nice option to have. I was looking at it from the point of view of a consumer of the library. The main purpose of having a cmake file from this perspective was not to make it easy to develop the library itself, but to make it easy to use the library.