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

Fix CMake unit test compilation (no-pie) #2137

Merged
merged 3 commits into from
Jun 25, 2018

Conversation

MrAnno
Copy link
Collaborator

@MrAnno MrAnno commented Jun 22, 2018

This PR contains a workaround for a bug in the implementation of the POSITION_INDEPENDENT_CODE property (CMake).

https://gitlab.kitware.com/cmake/cmake/issues/16561

MrAnno added 3 commits June 22, 2018 18:04
Signed-off-by: László Várady <laszlo.varady@balabit.com>
Signed-off-by: László Várady <laszlo.varady@balabit.com>
Signed-off-by: László Várady <laszlo.varady@balabit.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS

include(CheckCCompilerFlag)
check_c_compiler_flag(-no-pie NO_PIE_AVAILABLE)
if (NO_PIE_AVAILABLE)
set_property(TARGET ${ADD_UNIT_TEST_TARGET} APPEND_STRING PROPERTY LINK_FLAGS " -no-pie")
Copy link
Collaborator

Choose a reason for hiding this comment

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

So in summary set_property(TARGET ${ADD_UNIT_TEST_TARGET} PROPERTY POSITION_INDEPENDENT_CODE FALSE) does not work with mac and gcc v>=6, hence the workaround. Does the POSITION_INDEPENDENT_CODE give anything after this workaround, or we could just delete that line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could remove that line, it does not give anything to the tests.
However, the code below is just a workaround that we'll remove after a "few" years (:smiley:), so I would leave POSITION_INDEPENDENT_CODE here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok fine for me.

@furiel furiel merged commit 808d78e into syslog-ng:master Jun 25, 2018
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.

4 participants