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

[Cpp] CMake: use variables for specifying install paths consistently #4109

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

jmairboeck
Copy link

This allows overriding them if needed when the system uses different paths than usual (e.g. Haiku, which uses "develop/headers" for includes instead of "include").

Signed-off-by: Joachim Mairböck j.mairboeck@gmail.com

@@ -222,14 +223,14 @@ endif(ANTLR4_INSTALL)

if(EXISTS LICENSE.txt)
install(FILES LICENSE.txt
DESTINATION "share/doc/libantlr4")
DESTINATION ${CMAKE_INSTALL_DOCDIR})
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this use an ANTLR-specific sub-directory? 🤔

Copy link
Author

@jmairboeck jmairboeck Feb 13, 2023

Choose a reason for hiding this comment

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

CMAKE_INSTALL_DOCDIR contains a project specific sub-directory by default, see https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html#result-variables. It defaults to DATAROOTDIR/doc/PROJECT_NAME.

Edit: Maybe the project name should be changed to lower-case so that this matches the old value?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, interesting. Didn't know that.

I wonder whether changing the casing of the project name has other side effects though 🤔

This allows overriding them if needed when the system uses different paths than
usual (e.g. Haiku, which uses "develop/headers" for includes instead of
"include").

Use the standard variable ${CMAKE_INSTALL_INCLUDEDIR} as the base path for the
include directory.

Signed-off-by: Joachim Mairböck <j.mairboeck@gmail.com>
…TALL_DOCDIR available

Signed-off-by: Joachim Mairböck <j.mairboeck@gmail.com>
@jmairboeck
Copy link
Author

I rebased this on top of the changes from #4661.

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