Skip to content

[CMake] fixes and improvements#129

Merged
fabiencastan merged 34 commits intodevelopfrom
cmake/win/fixesVcpkg
May 31, 2020
Merged

[CMake] fixes and improvements#129
fabiencastan merged 34 commits intodevelopfrom
cmake/win/fixesVcpkg

Conversation

@simogasp
Copy link
Member

@simogasp simogasp commented May 24, 2020

This PR fixes and improve cmake build system (no code is touched) and it should make easier the creation of the port file for vcpkg (https://github.com/alicevision/vcpkg/tree/ports/cctag)

A brief summary of the changes:

  • use GNUinstalldir

  • export the targets for in-tree build (it was missing)

  • print a build configuration summary at the end

  • all the build outputs are now found in directory named [Linux|Windows|Darwin] as standard in AV

  • Switch to modern cmake for Eigen, so now imported targets are used

  • add an option CCTAG_EIGEN_NO_ALIGN to disable eigen alignment (@fabiencastan )

  • Cuda support is not possible on windows unless Eigen 3.3.9 is used (as of now, not yet released, so using their develop branch): the minimum required version is in general 3.3.4, so a check is made when MSVC and Cuda are enabled to ensure the version is right otherwise cmake fails.

  • a fix in FindTBB to set the proper order of linking, apparently in static (under windows) tbb must be first

  • added properties VERSION and POSTFIX_DEBUG for all the targets

  • config.hpp and version.hpp are now placed in generated/cctag to allow `#include <cctag/version.hpp>

  • version.hpp has now a string for the full version

  • install the headers with an explicit list of files, otherwise empty directories may occur (e.g. cuda if cuda is disabled)

(NB there are obvious analogies with alicevision/popsift#92)

simogasp added 30 commits May 23, 2020 20:31
On windows static (vcpkg) the order of
linking seems to be important and the main
tbb lib must be first
@simogasp simogasp added scope:build build:windows anything related to the windows build linux labels May 24, 2020
@simogasp simogasp added this to the v1.0.0 milestone May 24, 2020
@simogasp simogasp requested review from fabiencastan and griwodz May 24, 2020 21:49
@simogasp simogasp force-pushed the cmake/win/fixesVcpkg branch from bddcfdf to 272efed Compare May 30, 2020 20:09


if(CCTAG_WITH_CUDA)
install(DIRECTORY "cctag"
Copy link
Member

Choose a reason for hiding this comment

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

What is the problem with this code?

Copy link
Member

Choose a reason for hiding this comment

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

I see in the PR comment: "install the headers with an explicit list of files, otherwise empty directories may occur (e.g. cuda if cuda is disabled)".
If it's only for the empty folder, it increase the complexity too much for this small gain.

Copy link
Member Author

@simogasp simogasp May 30, 2020

Choose a reason for hiding this comment

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

also all the test directories were added.
This is not something that can be easily avoided with cmake
https://gitlab.kitware.com/cmake/cmake/-/issues/17122
other than using GLOB with all the problems it creates
https://stackoverflow.com/questions/55451084/cmake-files-matching-pattern-copies-empty-directories/55722518#55722518

And for the record I added this because vcpkg complains if you install empty directories

Copy link
Member

Choose a reason for hiding this comment

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

ok

@fabiencastan fabiencastan merged commit 16a3fd8 into develop May 31, 2020
@fabiencastan fabiencastan deleted the cmake/win/fixesVcpkg branch May 31, 2020 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build:windows anything related to the windows build linux scope:build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants