Set OGRE minimal version to 1.8. Warn on versions not supported (ign-rendering3) #376

j-rivero · 2021-08-10T17:51:14Z

Might require gazebosim/gz-cmake#175 to work

Flag USE_UNOFFICIAL_OGRE_VERSIONS disabled the warning for the experienced users. Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>

codecov · 2021-08-10T18:11:32Z

Codecov Report

Merging #376 (e56de72) into ign-rendering3 (9341eb2) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head e56de72 differs from pull request most recent head 90ebd1e. Consider uploading reports for the commit 90ebd1e to get more accurate results

@@               Coverage Diff               @@
##           ign-rendering3     #376   +/-   ##
===============================================
  Coverage           53.34%   53.35%           
===============================================
  Files                 131      131           
  Lines               12036    12036           
===============================================
+ Hits                 6421     6422    +1     
+ Misses               5615     5614    -1

Impacted Files	Coverage Δ
...e/ignition/rendering/base/BaseGaussianNoisePass.hh	`100.00% <0.00%> (+3.33%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9341eb2...90ebd1e. Read the comment docs.

iche033 · 2021-08-10T18:26:53Z

CMakeLists.txt

+
+  if (OGRE_FOUND)
+    IGN_BUILD_WARNING("Ogre 1.x versions greater than 1.9 are not officially supported."
+                      "The software might compile and event work but support from upstream"


event - > even

support from upstream could be reduced to accept patches for newer versions

To confirm, I think this means we will provided limited support in the form of accepting patches for newer versions but not actively develop against them right?

To confirm, I think this means we will provided limited support in the form of accepting patches for newer versions but not actively develop against them right?

Eh yes, that is my idea but you Ian or @chapulina probably have it clearer in your mind. Please feel free to reformulate to somehow explain that we won't fix bug on other versions of Ogre but we accept all kind of patches to make it happen.

Got it. A couple of minor suggestions are: event -> even, and accept -> accepting

Done c57a8e5

Changelog.md

iche033 · 2021-08-11T17:14:54Z

CMakeLists.txt

+  endif()
+endif()
+
+ign_find_package(IgnOGRE VERSION 1.9.0


I think we can skip looking for 1.9.0 if 1.10 is found?

how about this 90ebd1e ?

iche033 · 2021-08-11T17:18:45Z

CMakeLists.txt

+
+  if (OGRE_FOUND)
+    IGN_BUILD_WARNING("Ogre 1.x versions greater than 1.9 are not officially supported."
+                      "The software might compile and event work but support from upstream"


Got it. A couple of minor suggestions are: event -> even, and accept -> accepting

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>

chapulina · 2021-08-26T02:06:42Z

Since this was merged, there are confusing status messages about IgnOgre, i.e.

https://build.osrfoundation.org/job/ignition_rendering-ci-ign-rendering3-bionic-amd64/49/consoleFull

-- Looking for IgnOGRE - not found

...

-- Looking for IgnOGRE - found
...

   -- Missing dependency [IgnOGRE] (Components: RTShaderSystem, Terrain, Overlay)
Call Stack (most recent call first):

Full snippet

-- Looking for IgnOGRE - not found

-- Checking for module 'OGRE-RTShaderSystem >= 1.9'
--   Found OGRE-RTShaderSystem , version 1.9.0
-- Checking for module 'OGRE-Terrain >= 1.9'
--   Found OGRE-Terrain , version 1.9.0
-- Checking for module 'OGRE-Overlay >= 1.9'
--   Found OGRE-Overlay , version 1.9.0
-- Looking for IgnOGRE - found

-- Boost version: 1.65.1
-- -- Finding OGRE 2.1
-- Looking for IgnOGRE2 - found

CUDA_TOOLKIT_ROOT_DIR not found or specified
-- Could NOT find CUDA (missing: CUDA_TOOLKIT_ROOT_DIR CUDA_NVCC_EXECUTABLE CUDA_INCLUDE_DIRS CUDA_CUDART_LIBRARY) 
-- optix library not found.  Please locate before proceeding.
-- OptiX headers (optix.h and friends) not found.  Please locate before proceeding.
-- optix Prime library not found.  Please locate before proceeding.
-- Could NOT find OptiX (missing: OptiX_FOUND) (Required is at least version "3.8.0")
-- Looking for OptiX - not found

CMake Warning at /usr/share/cmake/ignition-cmake2/cmake2/IgnConfigureBuild.cmake:53 (message):
   CONFIGURATION WARNINGS:
   -- Missing dependency [IgnOGRE] (Components: RTShaderSystem, Terrain, Overlay)
Call Stack (most recent call first):
  CMakeLists.txt:154 (ign_configure_build)

But then it looks like the ogre component is correctly built and installed.

Might require gazebosim/gz-cmake#175 to work

@j-rivero , is that going to improve the messages so that they're less misleading?

https://github.com/osrf/buildfarmer/issues/224

scpeters · 2021-08-26T04:32:41Z

Since this was merged, there are confusing status messages about IgnOgre, i.e.

https://build.osrfoundation.org/job/ignition_rendering-ci-ign-rendering3-bionic-amd64/49/consoleFull
-- Looking for IgnOGRE - not found

...

-- Looking for IgnOGRE - found
...

   -- Missing dependency [IgnOGRE] (Components: RTShaderSystem, Terrain, Overlay)
Call Stack (most recent call first):
Full snippet
But then it looks like the ogre component is correctly built and installed.

Might require ignitionrobotics/ign-cmake#175 to work

@j-rivero , is that going to improve the messages so that they're less misleading?

osrf/buildfarmer#224

in addition to the confusion of the messages, it's generating a cmake warning that makes the build unstable:

https://build.osrfoundation.org/job/ignition_rendering-ci-ign-rendering3-bionic-amd64/49/cmake/

chapulina · 2021-08-26T16:53:36Z

I think this also broke downstream Windows builds, which are installing Ogre 1.12.9:

https://build.osrfoundation.org/job/ign_gui-pr-win/996/consoleFull#console-section-10

 BEGIN SECTION: install external dependency ogre
"VCPKG_HEAD is 2021.05.12"
The vpckg directory is not using the expected snapshot 2020.11
Failed in windows_library with error #0.
Computing installation plan...
The following packages are already installed:
    ogre[assimp,core,freeimage,overlay,zziplib]:x64-windows -> 1.12.9#5
Package ogre:x64-windows is already installed

Total elapsed time: 14.3 us

The package ogre:x64-windows provides CMake targets:

    find_package(OGRE CONFIG REQUIRED)
    # Note: 6 target(s) were omitted.
    target_link_libraries(main PRIVATE OgreMain OgreBites OgrePaging OgreVolume)

# END SECTION
Applied user-wide integration for this vcpkg root.

All MSBuild C++ projects can now #include any installed libraries.
Linking will be handled automatically.
Installing new libraries will make them instantly available.

CMake projects should use: "-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake"

...

-- Looking for IgnOGRE - not found

-- Looking for IgnOGRE - not found

j-rivero · 2021-08-26T17:11:14Z

@j-rivero , is that going to improve the messages so that they're less misleading?

osrf/buildfarmer#224

I'm not sure. I've merged gazebosim/gz-cmake#175 to see if things are working as expected on Windows. Not that if we are using an unsupported versions of Ogre in our CI then:

Seems weird that our own CI uses an unsupported version, we might need to rethink about it
As a workaround, we can enable -DUSE_UNOFFICAL_OGRE_VERSIONS:ON to get rid of the warning

Failure was not the intended result in any case of this PR, we need to consider it as a bug and resolved as soon as we can.

scpeters · 2021-09-16T18:15:59Z

is this new USE_UNOFFICAL_OGRE_VERSIONS in use anywhere yet? can we fix the spelling to USE_UNOFFICIAL_OGRE_VERSIONS?

j-rivero · 2021-09-16T18:17:24Z

is this new USE_UNOFFICAL_OGRE_VERSIONS in use anywhere yet? can we fix the spelling to USE_UNOFFICIAL_OGRE_VERSIONS?

I did not use it anywhere. It should be fixed, I'll open a PR.

traversaro · 2021-10-04T15:12:57Z

Sorry for being a bit late to the party. : ) However, I was trying to debug some probably related regression in conda-forge/libignition-rendering4-feedstock#19 (comment) , and I was trying to understand how to use the new USE_UNOFFICIAL_OGRE_VERSIONS option, and I may be wrong, but it seems that if USE_UNOFFICIAL_OGRE_VERSIONS is ON, then find_package(IgnOgre <..>) is never invoked? Is that the case, or I am missing something obvious?

j-rivero · 2021-10-04T16:49:08Z

Sorry for being a bit late to the party. : ) However, I was trying to debug some probably related regression in conda-forge/libignition-rendering4-feedstock#19 (comment) , and I was trying to understand how to use the new USE_UNOFFICIAL_OGRE_VERSIONS option, and I may be wrong, but it seems that if USE_UNOFFICIAL_OGRE_VERSIONS is ON, then find_package(IgnOgre <..>) is never invoked? Is that the case, or I am missing something obvious?

ouch #453

j-rivero added 2 commits August 10, 2021 19:48

Set OGRE minimal version to 1.8. Warn on versions not supported.

b1aee2f

Flag USE_UNOFFICIAL_OGRE_VERSIONS disabled the warning for the experienced users. Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>

Add Changelog entry

d68761a

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>

j-rivero requested a review from iche033 as a code owner August 10, 2021 17:51

set pr number

a78519d

github-actions bot added the 🏰 citadel Ignition Citadel label Aug 10, 2021

Merge branch 'ign-rendering3' into jrivero/ogre_warn_versions

8e0657a

chapulina added the needs upstream release Blocked by a release of an upstream library label Aug 10, 2021

iche033 reviewed Aug 10, 2021

View reviewed changes

fix grammar

2cdccf2

chapulina mentioned this pull request Aug 11, 2021

Provide information about the Ogre version used at configure time #375

Closed

iche033 reviewed Aug 11, 2021

View reviewed changes

j-rivero added 2 commits August 11, 2021 19:52

Fix grammar

c57a8e5

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>

If ogre-1.10 is found, skip the 1.9 check

90ebd1e

iche033 mentioned this pull request Aug 11, 2021

Fix problems on IgnOGRE when version is not found gazebosim/gz-cmake#175

Merged

6 tasks

j-rivero requested a review from iche033 August 23, 2021 11:07

iche033 approved these changes Aug 24, 2021

View reviewed changes

j-rivero enabled auto-merge (squash) August 24, 2021 18:13

j-rivero disabled auto-merge August 24, 2021 18:13

j-rivero merged commit 99c08c5 into ign-rendering3 Aug 24, 2021

j-rivero deleted the jrivero/ogre_warn_versions branch August 24, 2021 18:14

chapulina mentioned this pull request Sep 9, 2021

Ubuntu CI not finding Ogre 1.9 #400

Closed

j-rivero mentioned this pull request Sep 15, 2021

Avoid configure warning when checking ogre-1.10 #411

Merged

2 tasks

j-rivero mentioned this pull request Sep 16, 2021

Fix the spelling of USE_UNOFFICIAL_OGRE_VERSIONS flag #413

Merged

1 task

traversaro mentioned this pull request Oct 4, 2021

libignition-rendering4 v6_6.0.0 conda-forge/libignition-rendering4-feedstock#19

Merged

3 tasks

j-rivero mentioned this pull request Oct 4, 2021

Fix check for ogre when using USE_UNOFFICIAL_OGRE_VERSIONS #453

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set OGRE minimal version to 1.8. Warn on versions not supported (ign-rendering3) #376

Set OGRE minimal version to 1.8. Warn on versions not supported (ign-rendering3) #376

j-rivero commented Aug 10, 2021 •

edited

Loading

codecov bot commented Aug 10, 2021 •

edited

Loading

iche033 Aug 10, 2021

j-rivero Aug 10, 2021

iche033 Aug 11, 2021

j-rivero Aug 11, 2021

iche033 Aug 11, 2021

j-rivero Aug 11, 2021

iche033 Aug 11, 2021

chapulina commented Aug 26, 2021

scpeters commented Aug 26, 2021

chapulina commented Aug 26, 2021

j-rivero commented Aug 26, 2021

scpeters commented Sep 16, 2021

j-rivero commented Sep 16, 2021

traversaro commented Oct 4, 2021

j-rivero commented Oct 4, 2021

Set OGRE minimal version to 1.8. Warn on versions not supported (ign-rendering3) #376

Set OGRE minimal version to 1.8. Warn on versions not supported (ign-rendering3) #376

Conversation

j-rivero commented Aug 10, 2021 • edited Loading

codecov bot commented Aug 10, 2021 • edited Loading

Codecov Report

iche033 Aug 10, 2021

Choose a reason for hiding this comment

j-rivero Aug 10, 2021

Choose a reason for hiding this comment

iche033 Aug 11, 2021

Choose a reason for hiding this comment

j-rivero Aug 11, 2021

Choose a reason for hiding this comment

iche033 Aug 11, 2021

Choose a reason for hiding this comment

j-rivero Aug 11, 2021

Choose a reason for hiding this comment

iche033 Aug 11, 2021

Choose a reason for hiding this comment

chapulina commented Aug 26, 2021

scpeters commented Aug 26, 2021

chapulina commented Aug 26, 2021

j-rivero commented Aug 26, 2021

scpeters commented Sep 16, 2021

j-rivero commented Sep 16, 2021

traversaro commented Oct 4, 2021

j-rivero commented Oct 4, 2021

j-rivero commented Aug 10, 2021 •

edited

Loading

codecov bot commented Aug 10, 2021 •

edited

Loading