Clean cmake opengl warning ubuntu22 by Tamir91 · Pull Request #11442 · realsenseai/librealsense

Tamir91 · 2023-02-15T12:23:18Z

Tracking on [LRS-673]

Nir-Az · 2023-02-19T16:38:36Z

 cmake_minimum_required(VERSION 3.1.0)

+# This policy provides compatibility with projects that expect the legacy GL library to be used.
+if (POLICY CMP0072 AND UNIX AND NOT APPLE)


Can you explain why NOT APPLE?

I understood from the ticket name that I need to fix the issue on U22.
Have we Apple machines to purpose seen new behavior?

Looks like you are right,
You can verify on GHA, we have a mac machine
https://github.com/IntelRealSense/librealsense/actions/runs/4183814020/jobs/7248655076

Ups, I saw it before)
May I run viewer on Mac and see live that all work correctly?

Since you don't have a mac, you can only run MAC with a VM if you want

Nir-Az · 2023-02-19T16:38:54Z

+# This policy provides compatibility with projects that expect the legacy GL library to be used.
+if (POLICY CMP0072 AND UNIX AND NOT APPLE)
+	set(CMAKE_POLICY_DEFAULT_CMP0072 NEW)
+	message(STATUS "Policy CMP0072 setted: NEW")


No need to see this message..

The comment was removed.

Nir-Az · 2023-02-19T16:40:16Z

 #  minimum required cmake version: 3.1.0
 cmake_minimum_required(VERSION 3.1.0)

+# This policy provides compatibility with projects that expect the legacy GL library to be used.


Please update this generic comment,
if you think NEW should be used..

Nir-Az · 2023-02-20T14:37:07Z

I just saw you place it on the main CMakelist, why is that?
You should place stuff like that as narrow and close to the relevant line as possible.
We don't builf GLFW project on all cases, so we don't need it for all cases.

The best place to place it is here:
https://github.com/IntelRealSense/librealsense/blob/master/third-party/glfw/CMakeLists.txt
This file is configures only if we include GLFW

Tamir91 · 2023-02-20T15:29:28Z

I just saw you place it on the main CMakelist, why is that? You should place stuff like that as narrow and close to the relevant line as possible. We don't builf GLFW project on all cases, so we don't need it for all cases.

The best place to place it is here: https://github.com/IntelRealSense/librealsense/blob/master/third-party/glfw/CMakeLists.txt This file is configures only if we include GLFW

You are right.
In my first commit and now I tried to add code to glfw/CMakeList.txt and it not solving the warning.
I have now checked if adding the code to third-party/CMakeList.txt solves the warning...
Please advise.

Nir-Az · 2023-02-20T15:58:16Z

I just saw you place it on the main CMakelist, why is that? You should place stuff like that as narrow and close to the relevant line as possible. We don't builf GLFW project on all cases, so we don't need it for all cases.
The best place to place it is here: https://github.com/IntelRealSense/librealsense/blob/master/third-party/glfw/CMakeLists.txt This file is configures only if we include GLFW

You are right. In my first commit and now I tried to add code to glfw/CMakeList.txt and it not solving the warning. I have now checked if adding the code to third-party/CMakeList.txt solves the warning... Please advise.

OK, thats interesting,
It means someone else is calling
find_package(OpenGL REQUIRED)

From the log we can see:

Call Stack (most recent call first):
  CMake/opengl_config.cmake:1 (find_package)
  tools/CMakeLists.txt:25 (include)

Please try adding this (instead of other lines)
to the top of CMake/opengl_config.cmake file

# Comment why we are doing this
if (POLICY CMP0072)
    cmake_policy(SET CMP0072 NEW)
endif()

BTW, why did you use another method than cmake_policy(SET..?

Tamir91 · 2023-02-21T07:49:33Z

Tamir91 · 2023-02-21T07:55:01Z

I just saw you place it on the main CMakelist, why is that? You should place stuff like that as narrow and close to the relevant line as possible. We don't builf GLFW project on all cases, so we don't need it for all cases.
The best place to place it is here: https://github.com/IntelRealSense/librealsense/blob/master/third-party/glfw/CMakeLists.txt This file is configures only if we include GLFW

You are right. In my first commit and now I tried to add code to glfw/CMakeList.txt and it not solving the warning. I have now checked if adding the code to third-party/CMakeList.txt solves the warning... Please advise.

OK, thats interesting, It means someone else is calling find_package(OpenGL REQUIRED)

From the log we can see:
Call Stack (most recent call first):
  CMake/opengl_config.cmake:1 (find_package)
  tools/CMakeLists.txt:25 (include)
Please try adding this (instead of other lines) to the top of CMake/opengl_config.cmake file
# Comment why we are doing this
if (POLICY CMP0072)
    cmake_policy(SET CMP0072 NEW)
endif()
BTW, why did you use another method than cmake_policy(SET..?

I used cmake_policy(SET. function because only it affects the policy warning (I found it Stackoverflow).
I put the policy in CMake/opengl_config.cmake and found it works too.
Are you think set policy there?

Nir-Az

LGTM

Tamir91 added 2 commits February 14, 2023 16:26

added cmake policy cmp0072 for unix

449eb88

added cmake policy for Unix

e353e37

Nir-Az reviewed Feb 19, 2023

View reviewed changes

the comment updated

09b6c73

Tamir91 added 2 commits February 20, 2023 17:05

moving policy check to other file

0c7a4d4

trying other function

4823d3a

the policy moved to opengl_config.cmake

9259548

Tamir91 closed this Feb 21, 2023

Tamir91 reopened this Feb 21, 2023

Tamir91 added 2 commits February 21, 2023 09:58

commented code removed

88061bc

using new policy for all supported OS

3fdadfe

Nir-Az approved these changes Feb 21, 2023

View reviewed changes

Nir-Az merged commit 43cc7c2 into realsenseai:development Feb 21, 2023

Tamir91 deleted the clean_cmake_opengl_warning_ubuntu22 branch February 26, 2023 09:29

Uh oh!

Conversation

Tamir91 commented Feb 15, 2023

Uh oh!

Nir-Az Feb 19, 2023

Choose a reason for hiding this comment

Uh oh!

Tamir91 Feb 19, 2023

Choose a reason for hiding this comment

Uh oh!

Nir-Az Feb 19, 2023

Choose a reason for hiding this comment

Uh oh!

Tamir91 Feb 20, 2023

Choose a reason for hiding this comment

Uh oh!

Nir-Az Feb 20, 2023

Choose a reason for hiding this comment

Uh oh!

Nir-Az Feb 19, 2023

Choose a reason for hiding this comment

Uh oh!

Tamir91 Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nir-Az Feb 19, 2023

Choose a reason for hiding this comment

Uh oh!

Nir-Az commented Feb 20, 2023

Uh oh!

Tamir91 commented Feb 20, 2023

Uh oh!

Nir-Az commented Feb 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tamir91 commented Feb 21, 2023

Uh oh!

Tamir91 commented Feb 21, 2023

Uh oh!

Nir-Az left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Tamir91 Feb 21, 2023 •

edited

Loading

Nir-Az commented Feb 20, 2023 •

edited

Loading