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

use FORCE_COMPILE_HRPSYSUTIL if you want to know whethere hrpsysUtils is build or not #1321

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

Conversation

k-okada
Copy link
Contributor

@k-okada k-okada commented Jul 25, 2022

@gergondet we had a little problem on 89d8ac4, when we compile hrpsys on real-hardware , where we do not have OpenCV and others.
Because before #1310, we could compile hrpsys without any compile options, but after #1310, we need to add -DUSE_HRPSYSUTIL=OFF explictly.

So as a compromise, how about introducing FORCE_BUILD_HRPSYSUTILS ? This will not break existing behavior as well as if you add that option, adn that option wil make it clear whether if hrpsysUtils is compiled or not.

CMakeLists.txt Outdated
set(USE_HRPSYSUTIL OFF)
endif()
find_package(OpenGL REQUIRED)
if (NOT OpenGL_FOUND)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (NOT OpenGL_FOUND)
if (NOT OPENGL_FOUND)

OpenGL_FOUNDではなくOPENGL_FOUNDに値がセットされるようです。

https://github.com/fkanehiro/hrpsys-base/pull/1310/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aL166

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 think default behavior of find_packaeg(<package is to set <package>_FOUND
https://cmake.org/cmake/help/v3.0/command/find_package.html

but

$ cat CMakeLists.txt 
cmake_minimum_required(VERSION 2.4)
project(hrpsys)
find_package(OpenGL REQUIRED)
message("=> OpenGL_FOUND: ${OpenGL_FOUND}")
message("=> OPENGL_FOUND: ${OPENGL_FOUND}")

outputs

-- Found OpenGL: /usr/lib/x86_64-linux-gnu/libOpenGL.so   
=> OpenGL_FOUND: TRUE
=> OPENGL_FOUND: TRUE

???

Copy link
Contributor

Choose a reason for hiding this comment

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

find_package(OpenGL)をするとFindOpenGL.cmakeが実行されるのですが、

FindOpenGL.cmakeの中でFIND_PACKAGE_HANDLE_STANDARD_ARGS関数が呼ばれると、FIND_PACKAGE_HANDLE_STANDARD_ARGS関数の中でOpenGL_FOUNDとOPENGL_FOUND両方がセットされるようです。

ところが、cmake 2.4.7を使っているような古いPCでは、FindOpenGL.cmakeの中でFIND_PACKAGE_HANDLE_STANDARD_ARGSが呼ばれず、OPENGL_FOUNDだけを直接セットしているようです。

そのため、古いcmakeと今のcmake両方で動くようにするためには、OpenGL_FOUNDではなくOPENGL_FOUNDを使う必要があるのではないかと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you. please create PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

お手数ですが上の僕のコメントの中でSuggested Changeの枠で表示されている内容についても、ブラウザ上でマウス操作でmergeできますので、取り入れていただけますでしょうか

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

CMakeLists.txt Outdated Show resolved Hide resolved
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