Skip to content

switch to using CMake install procedure for Eigen 3.3.4 & newer#1482

Merged
damianam merged 2 commits intoeasybuilders:developfrom
boegel:eigen_cmake
Aug 24, 2018
Merged

switch to using CMake install procedure for Eigen 3.3.4 & newer#1482
damianam merged 2 commits intoeasybuilders:developfrom
boegel:eigen_cmake

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Aug 23, 2018

If the CMake-based installation procedure is not using, the eigen3.pc file for pkg-config does not get generated & installed...

Copy link
Copy Markdown
Member

@migueldiascosta migueldiascosta left a comment

Choose a reason for hiding this comment

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

lgtm

@damianam
Copy link
Copy Markdown
Member

@boegel it lgtm, but I wonder where the version check comes from. I haven't seen anything in Eigen's changelog pointing to any change in cmake files or pkg-config in 3.3.4.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Aug 24, 2018

@damianam I don't want to make this change for old Eigen versions/easyconfigs because i) the problem hasn't popped up for older versions (I suspect because not many software packages require eigen3.pc, and ii) mainly because the change requires using CMake as a build dependency for something that is installed with dummy toolchain (since nothing is actually being compiled).
That implies that CMake itself should also be built with dummy, which leaves us at the mercy of the OS compiler for building CMake 😱

@damianam
Copy link
Copy Markdown
Member

I understand........ But I would add a comment making clear where the version restriction comes from. This is an EB issue, not an eigen requirement. Normally version checks are due to changes from one version onwards, which is not the case here. Looks to me like sort of "abusing" version checks for something they are not intended for (not exactly that, but you know what I mean)

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Aug 24, 2018

@damianam I understand the potential confusion, clarifying comment added in 175326c

Copy link
Copy Markdown
Member

@damianam damianam left a comment

Choose a reason for hiding this comment

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

lgtm

@damianam damianam merged commit 64da183 into easybuilders:develop Aug 24, 2018
@boegel boegel deleted the eigen_cmake branch August 24, 2018 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants