On Sun, Aug 11, 2024 at 5:39 PM Daniel Sahlberg <dsahlb...@apache.org> wrote:
>
> On 2024/07/11 15:38:21 rin...@apache.org wrote:
> > Author: rinrab
> > Date: Thu Jul 11 15:38:21 2024
> > New Revision: 1919150
> >
> > URL: http://svn.apache.org/viewvc?rev=1919150&view=rev
> > Log:
> > On the 'cmake' branch: Support RA-Serf and add Serf dependency.
>
> [...]
>
> > +### Serf
> > +if (SVN_BUILD_RA_SERF)
> > + find_package(Serf REQUIRED)
> > + add_library(external-serf ALIAS Serf::Serf)
> > +endif()
> > +
>
> [...]
>
> > +message(STATUS " Build RA SERF ................. :
> > ${SVN_BUILD_RA_SERF}")
>
> [...]
>
> > +message(STATUS " SERF .......................... : ${Serf_VERSION}")
>
> How about enclosing the last message (${Serf_VERSION}) inside an if
> (SVN_ENABLE_RA_SERF), like this:
> [[[
> if (SVN_ENABLE_RA_SERF)
> message(STATUS " SERF .......................... : ${Serf_VERSION}")
> endif()
> ]]]
>
> Currently the configuration summary output is
> [[[
> -- Enable RA SERF ................ : OFF
> -- Dependecies:
> -- SERF .......................... :
> ]]]
>
> With the above change it will only be:
> [[[
> -- Enable RA SERF ................ : OFF
> -- Dependecies:
> ]]]
>
> With SVN_ENABLE_RA_SERF the output is (in any case):
> [[[
> -- Enable RA SERF ................ : ON
> -- Dependecies:
> -- SERF .......................... : 1.3.10
> ]]]
>
> I don't see a reason for listing Serf (with an empty version string) under
> Dependencies if it is not used.
>
> Another option would be to set the Serf_VERSION string to something
> reasonable if SVN_ENABLE_RA_SERF is OFF, output could be something like
> below, but think it is less clear.
> [[[
> -- Enable RA SERF ................ : OFF
> -- Dependecies:
> -- SERF .......................... : None
> ]]]
Hi,
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()
]]]
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.
--
Timofei Zhakov
