Ankit Meel (ankitm) retitled this revision from CMake: issue warnings while changing options to CMake: issue warnings when changing options.Feb 6 2021, 8:56 AM

Ankit Meel (ankitm) added a reviewer: Sergey Sharybin (sergey).Feb 9 2021, 6:27 AM

Found some more instances to improve.

Sergey Sharybin (sergey) edited reviewers, added: Bastien Montagne (mont29), Ray Molenkamp (LazyDodo), Campbell Barton (campbellbarton); removed: Sergey Sharybin (sergey).Feb 9 2021, 11:26 AM

Campbell Barton (campbellbarton) requested changes to this revision.Feb 11 2021, 5:29 AM

Campbell Barton (campbellbarton) added inline comments.

CMakeLists.txt
169–170	This can be removed as the minimum version is now 3.5.
789–790	Would leave as-is, `WITH_CPU_SSE` was not requested, so disabling shouldn't be a warning. It would make more sense to warn if `TEST_SSE_SUPPORT` fails.

Evan Wilson (EAW) awarded a token.Feb 11 2021, 9:02 AM

feedback
some in platform_apple file

Harbormaster completed remote builds in B13819: Diff 35767.Mar 30 2021, 1:07 PM

Ankit Meel (ankitm) removed a reviewer: Bastien Montagne (mont29).Mar 30 2021, 1:08 PM

Campbell Barton (campbellbarton) accepted this revision.Mar 30 2021, 4:47 PM

This revision is now accepted and ready to land.Mar 30 2021, 4:47 PM

Closed by commit rBe5f0d176d4cf: CMake: issue warnings when changing options (authored by Ankit Meel (ankitm)). · Explain WhyMar 30 2021, 4:59 PM

This revision was automatically updated to reflect the committed changes.

Ankit Meel (ankitm) added a commit: rBe5f0d176d4cf: CMake: issue warnings when changing options.

While I have nothing against this patch (I quite like it actually) however the patch description is highly unsatisfactory, please try at least a little , see The ingredients of a patch for the bare minimum we expect.

in said patch description you could have make the argument why WARNING

CMake Warning at CMakeLists.txt:728 (message):
  No dependencies need 'WITH_BOOST' forcing WITH_BOOST=OFF


-- 'WITH_OPENVDB' is disabled: forcing 'set(WITH_NANOVDB OFF)'
-- 64 bit compiler detected.
-- Visual Studio 2019 detected.
-- SSE2 SIMD instructions enabled
-- Tests: Using Python executable: K:/BlenderGit/2019_full_tests/bin//2.93/python/bin/python$<$<CONFIG:Debug>:_d>
-- Disabling sequencer render tests because OIIO idiff does not exist
-- Configuring done
-- Generating done
-- Build files have been written to: K:/BlenderGit/2019_full_tests

would be preferred over STATUS

-- No dependencies need 'WITH_BOOST' forcing WITH_BOOST=OFF
-- 'WITH_OPENVDB' is disabled: forcing 'set(WITH_NANOVDB OFF)'
-- 64 bit compiler detected.
-- Visual Studio 2019 detected.
-- SSE2 SIMD instructions enabled
-- Tests: Using Python executable: K:/BlenderGit/2019_full_tests/bin//2.93/python/bin/python$<$<CONFIG:Debug>:_d>
-- Disabling sequencer render tests because OIIO idiff does not exist
-- Configuring done
-- Generating done
-- Build files have been written to: K:/BlenderGit/2019_full_tests

I bet you had a good reason for making this change, but i'm not seeing it.

I really think we should change WARNING back to STATUS. The console output from WARNING is messy and this was not done consistently (platform_apple.cmake was changed but not other platforms).

I don't see what that change achieves.

rBf061de9b3e6b: Cleanup/CMake: warning to status to reduce noise