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.

Sign up for GitHub

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

Jump to bottom

Install headers to include/${PROJECT_NAME} #46

Merged
merged 2 commits into from
Mar 8, 2022
Merged

Install headers to include/${PROJECT_NAME} #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4db27d0
Install headers to include/${PROJECT_NAME}
sloretz Mar 2, 2022
34c72e9
Fix include directory used by tests
sloretz Mar 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions 0001-Install-yaml.h-to-INSTALL_INCLUDE_DIR.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
From bfcbb19c954fa20a68708e707e76c725058e001f Mon Sep 17 00:00:00 2001
From: Shane Loretz <[email protected]>
Date: Tue, 1 Mar 2022 16:54:10 -0800
Subject: [PATCH] Install yaml.h to INSTALL_INCLUDE_DIR

Signed-off-by: Shane Loretz <[email protected]>
---
CMakeLists.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4f81148..80b12cb 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -84,7 +84,7 @@ target_include_directories(yaml PUBLIC
install(
FILES
include/yaml.h
- DESTINATION include COMPONENT Development
+ DESTINATION ${INSTALL_INCLUDE_DIR} COMPONENT Development
)

install(
--
2.25.1

16 changes: 12 additions & 4 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,20 @@ macro(build_libyaml)
endif()
include(ExternalProject)
externalproject_add(libyaml-0.2.5
URL https://github.com/yaml/libyaml/archive/refs/tags/0.2.5.zip
URL_MD5 2464078985a73f9d298689d36061143f
TIMEOUT 60
GIT_REPOSITORY https://github.com/yaml/libyaml.git
GIT_TAG 2c891fc7a770e8ba2fec34fc6b545c672beb37e6 # 0.2.5
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll point out that you can use an actual tag here (like 0.2.5), which I think would be preferable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure that's preferable. Currently a URL to a zip file and an MD5 sum are used. The MD5 sum makes sure the archive contains what we expect it to (limited by the ease of which it is to make an MD5 collision these days). If GIT_TAG only says the human readable tag name, then a change to the tag upstream could break the build, or worse be a security issue. Using the sha hash solves that.

Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed this elsewhere, and we are going to switch to git hashes everywhere. So with that, I'll go ahead and approve.

GIT_CONFIG advice.detachedHead=false
# Suppress git update due to https://gitlab.kitware.com/cmake/cmake/-/issues/16419
# See https://github.com/ament/uncrustify_vendor/pull/22 for details
UPDATE_COMMAND ""
TIMEOUT 600
CMAKE_ARGS
-DCMAKE_INSTALL_PREFIX=${CMAKE_CURRENT_BINARY_DIR}/libyaml_install
-DINSTALL_INCLUDE_DIR:STRING=include/${PROJECT_NAME}
${extra_cmake_args}
PATCH_COMMAND
${CMAKE_COMMAND} -E chdir <SOURCE_DIR> git apply -p1 --ignore-space-change --whitespace=nowarn
${CMAKE_CURRENT_SOURCE_DIR}/0001-Install-yaml.h-to-INSTALL_INCLUDE_DIR.patch
)

# The external project will install to the build folder, but we'll install that on make install.
Expand All @@ -79,7 +87,7 @@ macro(build_libyaml)
PATTERN config.h EXCLUDE
)

set(yaml_INCLUDE_DIRS ${CMAKE_CURRENT_BINARY_DIR}/libyaml_install/include)
set(yaml_INCLUDE_DIRS ${CMAKE_CURRENT_BINARY_DIR}/libyaml_install/include/${PROJECT_NAME})
set(yaml_LIBRARY_DIRS ${CMAKE_CURRENT_BINARY_DIR}/libyaml_install/lib)
set(yaml_LIBRARIES yaml)
endmacro()
Expand Down