On Mon, Aug 12, 2024 at 10:47 PM Daniel Sahlberg
<daniel.l.sahlb...@gmail.com> wrote:
>
> Den mån 12 aug. 2024 kl 16:30 skrev Timofei Zhakov <t...@chemodax.net>:
>>
>> I agree the version should not be listed when we don't have this
>> dependency. Based on Daniel's suggestions, I think that the behaviour
>> of the version in the configuration summary should be as explained
>> below:
>>
>> 1. If the ra-serf is enabled:
>>
>> [[[
>> -- Enable RA SERF ................ : ON
>> -- Dependecies:
>> -- SERF .......................... : 1.3.10
>> ]]]
>>
>> 2. If it is disabled:
>>
>> [[[
>> -- Enable RA SERF ................ : OFF ### it could be 'False' as well
>> -- Dependecies:
>> -- SERF .......................... : NONE
>> ]]]
>>
>> 3. Finally, if the version was not found we will have that empty
>> string again. Is it actually okay? See the example below:
>>
>> [[[
>> -- Enable RA SERF ................ : ON
>> -- Dependecies:
>> -- SERF .......................... : ### The module somehow
>> could not recognise the version, so we have an empty string
>> ]]]
>>
>> Basically the suggested code looks nice, but I would like to add an
>> 'else' block to message 'NONE' instead of an empty version if ra-serf
>> is disabled.
>>
>> [[[
>> if(SVN_ENABLE_RA_SERF)
>> message(STATUS " SERF .......................... : ${Serf_VERSION}")
>> else()
>> message(STATUS " SERF .......................... : NONE")
>> endif()
>> ]]]
>
>
> In that case, I'd suggest to set the Serf_VERSION as we are finding Serf. I
> don't know if we are looking at Serf_VERSION anywhere else (when
> SVN_ENABLE_RA_SERF is OFF).
>
> [[[
> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt (revision 1919834)
> +++ CMakeLists.txt (working copy)
> @@ -240,6 +240,8 @@
> if (SVN_ENABLE_RA_SERF)
> find_package(Serf REQUIRED)
> add_library(external-serf ALIAS Serf::Serf)
> +else()
> + set(Serf_VERSION "NONE")
> endif()
>
> ### Python
> ]]]
>
> I still think I prefer to not report SERF at all under dependencies, to make
> the list a bit shorter by not reporting all the dependencies that we don't
> depend on.
>
>>
>> Additionally, the versions of Gettext, Intl, Python, Perl, Ruby (not
>> yet implemented but will be soon) currently are not listed there, but
>> should be. Most of these dependencies could be disabled via options as
>> well.
>
>
> I'll look at these shortly!
What do you think about just removing this section?
There is already a logging of how CMake looks for the dependencies and
of their versions, so I don't see any reason to duplicate it.
Providing an example of these logs below:
[[[
[...]
1> [CMake] -- Found APR:
C:/tima/poshsvn-trunk/vcpkg_installed/x64/x64-svn-windows/lib/libapr-1.lib
(found version "1.7.4")
1> [CMake] -- Found APRUtil:
C:/tima/poshsvn-trunk/vcpkg_installed/x64/x64-svn-windows/lib/libaprutil-1.lib
(found version "1.6.3")
1> [CMake] -- Found ZLIB:
C:/tima/poshsvn-trunk/vcpkg_installed/x64/x64-svn-windows/lib/zlib.lib
(found version "1.3.0")
1> [CMake] -- Found EXPAT:
C:/tima/poshsvn-trunk/vcpkg_installed/x64/x64-svn-windows/lib/libexpatMD.lib
(found version "2.2.6")
1> [CMake] -- Found SQLiteAmalgamation:
C:/tima/svn-cmake/sqlite-amalgamation (found version "3.46.0")
1> [CMake] -- Found Serf:
C:/tima/poshsvn-trunk/vcpkg_installed/x64/x64-svn-windows/lib/serf-1.lib
(found version "3.1.4")
[...]
]]]
Yeah, I added this section myself, but right now it no longer makes
sense for me :)
--
Timofei Zhakov
