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

Modern cmake & target for use with add_submodule #16

Merged
merged 21 commits into from
Aug 7, 2020

Conversation

Dawars
Copy link
Contributor

@Dawars Dawars commented Aug 5, 2020

Based on #15

The main consideration was to be able to be added as a submodule to other projects and link by target:

add_subdirectory(libs/libraw)
target_link_libraries(ProjectName PRIVATE libraw::libraw)

The minimum supported cmake version is 3.12 because of FIND_PACKAGE(JPEG) and because it's easy to install.

A small change is that the repo now contains libraw as a submodule and it doesn't have to be manually downloaded. This is in order to ease the usage by tools that expect the repo to work out of the box.

@Dawars Dawars changed the title Modern cmake Modern cmake & target for use with add_submodule Aug 5, 2020
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
.gitmodules Outdated Show resolved Hide resolved
@Dawars Dawars requested a review from letmaik August 5, 2020 20:26
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@Dawars Dawars requested a review from letmaik August 6, 2020 10:09
Copy link
Collaborator

@letmaik letmaik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now. Thanks for working through the changes! I'll hold off merging the PR until I validated this in rawpy's CI.

letmaik added a commit to letmaik/rawpy that referenced this pull request Aug 7, 2020
@letmaik
Copy link
Collaborator

letmaik commented Aug 7, 2020

I checked rawpy's CI and had to do the following changes:

  • Update CMake to >= 3.12
    • for Windows I still had 3.7
    • for CentOS 6 (manylinux docker image), the package manager version is 2.8, I now install a newer version from cmake.org
  • Add -DENABLE_OPENMP=OFF for macOS builds
    • This is because this PR makes it a failure if OpenMP is not found, and the default is ENABLE_OPENMP=ON

The CMake update is fine. The OpenMP update is a minor semantic break but I think this is good as otherwise there is no way to verify that you're actually getting OpenMP if you request it.

I'm going to merge this now. Thanks again for working on this, very much appreciated and long overdue :)

@letmaik letmaik merged commit 573aece into LibRaw:master Aug 7, 2020
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.

2 participants