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 install path #2202

Merged
merged 7 commits into from
Apr 5, 2024
Merged

Fix CMake install path #2202

merged 7 commits into from
Apr 5, 2024

Conversation

MegMll
Copy link
Collaborator

@MegMll MegMll commented Apr 4, 2024

Fix issue in #2187.
Probably does not work in case boost and pinocchio are not installed in the directory.

CMakeLists.txt Outdated
Comment on lines 357 to 360
FOREACH(boostlib ${BOOST_REQUIRED_COMPONENTS})
PKG_CONFIG_APPEND_LIBS("boost_${boostlib}")
ENDFOREACH(boostlib ${BOOST_REQUIRED_COMPONENTS})
# PKG_CONFIG_APPEND_BOOST_LIBS(${BOOST_REQUIRED_COMPONENTS})
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change the jrl cmakemodule directly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why this change?

Because the aboslute path is only present, after the other change, because boost::filesystem is not header only, and so to still link on all components, without having -L/path it's the only solution i saw.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you remove the commented line?

Copy link
Contributor

Choose a reason for hiding this comment

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

for cmeel packaging, which also avoid full paths, those are simply removed for now.
A better solution would be to have a side project properly writing .pc files for boost, in a clean way. I started something in this direction 2y ago, but didn't went very far.

Comment on lines 25 to 26
SET_TARGET_PROPERTIES(${PROJECT_NAME} PROPERTIES LINKER_LANGUAGE CXX INSTALL_RPATH "\$ORIGIN")

Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not commit this change as it is not related to the topic of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please undo this change ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry will do it again

CMakeLists.txt Outdated
Comment on lines 357 to 360
FOREACH(boostlib ${BOOST_REQUIRED_COMPONENTS})
PKG_CONFIG_APPEND_LIBS("boost_${boostlib}")
ENDFOREACH(boostlib ${BOOST_REQUIRED_COMPONENTS})
# PKG_CONFIG_APPEND_BOOST_LIBS(${BOOST_REQUIRED_COMPONENTS})
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change the jrl cmakemodule directly?

@jcarpent jcarpent enabled auto-merge (squash) April 4, 2024 14:15
jcarpent
jcarpent previously approved these changes Apr 4, 2024
@jcarpent
Copy link
Contributor

jcarpent commented Apr 4, 2024

@MegMll You are still missing the important change that you removed by force-pushing.

@jcarpent jcarpent changed the title Topic/cmake Fix CMake install path Apr 4, 2024
@jcarpent jcarpent enabled auto-merge (squash) April 5, 2024 07:29
@jcarpent jcarpent merged commit b71da0e into stack-of-tasks:devel Apr 5, 2024
17 of 18 checks passed
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.

None yet

3 participants