[cmake-developers] [CMake 0011944]: CPackDeb: Support dependencies between components/Debian packages

[Raffi Enficiaud]

Le 22/04/15 08:01, Domen Vrankar a écrit :
> 2015-04-21 23:38 GMT+02:00 Raffi Enficiaud <raffi.enficiaud at mines-paris.org>:
>> Le 21/04/15 23:01, Domen Vrankar a écrit :
>>>
>>> Hi,
>>>
>>> I pushed your first patch to next (I've split it into two separate
>>> commits and made some minor cleanup changes):
>>> http://www.cmake.org/gitweb?p=cmake.git;a=commit;h=8e0ecf9
>>>
>>>> Please find attached my last patch that allows the settings of the
>>>> dependencies per component.
>>>
>>>
>>> I haven't finished reviewing the rest of the patches however I've
>>> noticed that you omit quotes when setting or comparing variables.
>>
>>
>> Ok. I might help in the future if you point me on a specific line and
>> explain me the mistake.
>
> Almost every line in your cmake scripts that uses set, if, string or
> other commands (see the explanation below).
> I wouldn't be bothering you with that and fix it myself if the test
> wouldn't be failing and some other changes needed to be done.
>
>>>
>>> I've also noticed that the last test in last commit is succeeding on
>>> Ubuntu 15.04 but failing on Debian 7.8.0.
>>> It first fails with a cryptic error (string FIND requires X variables
>>> as input message...) on this line:
>>> string(FIND ${dpkg_depends} "lib" index_libwhatever)
>>> and after I put quotes arround ${dpkg_depends} it returns an error
>>> that the value is an empty string.
>>
>>
>> It might help if you send me the test log file of the run.
>
> I'll send you the data in the afternoon.
>
>> On the other
>> hand, I do not understand why it would be an error if "${dpkg_depends}" is
>> an empty string. (I should still be able to find from an empty string,
>> shouldn't I?)
>
> If you take a look at the code it searches for "lib" inside
> ${dpkg_depends} and then the next line checks if it found the content
> and prints out an error instead (so the test fails) - that is the
> expected way of your test failing.
>
> On the other hand since the value of ${dpkg_depends} is an empty
> string that value doesn't contain a value... So without quotes it is
> the same as if that variable would not even exist in the above string
> command (there is no empty string - the variable gets substituted by
> the content of that variable so it's the same as writing nothing) -
> and test failing like that is probably not what you intended.
>
> The other reason is that set(some_var some_value other_value) creates
> a list and set(some_var some_value) creates a string (list with one
> value) and set(var "foo bar") set(other_var ${var}) creates an array
> from string... Also using set(some_var) is cryptic - it's hard to know
> if you meant unset(some_var) or set(some_var "")...

Thanks for the explanations. Which raises new questions:

How would I create empty lists then (to which I can subsequently 
append)? Or check that a variable is defined? Or tell the calling 
function that the output variable is set to an empty string?
Those questions should sound very naive, but this is less than obvious 
to me.


For instance

string(TOUPPER ${CPACK_DEB_PACKAGE_COMPONENT} _local_component_name)
set(_component_shlibdeps_var 
CPACK_DEBIAN_${_local_component_name}_PACKAGE_SHLIBDEPS)

if(DEFINED ${_component_shlibdeps_var})
   ...

The variable _component_shlibdeps_var clearly is defined and contains a 
string that names/points a variable. In the if statement, I am checking 
if the variable pointed by _component_shlibdeps_var is defined by the 
user. It is not clear to me why I would put quotes in this case, there 
is no risk for _component_shlibdeps_var being an empty variable.

Would it be better with/equivalent to:
if("${_component_shlibdeps_var}")

?


Another example is
   if(CPACK_DEBIAN_PACKAGE_AUTO_DEPENDS AND NOT 
CPACK_DEBIAN_PACKAGE_AUTO_DEPENDS STREQUAL "")

The intention is to check if CPACK_DEBIAN_PACKAGE_AUTO_DEPENDS is 
defined and is not an empty string. Here I added the 'NOT 
CPACK_DEBIAN_PACKAGE_AUTO_DEPENDS STREQUAL ""' to an existing statement. 
Is it equivalent to

if(NOT "${CPACK_DEBIAN_PACKAGE_AUTO_DEPENDS}" STREQUAL "")

?

Again

set(CPackRun_CPackDEBConfiguration_ALL_CONFIGS)
is not the same to me as
unset(CPackRun_CPackDEBConfiguration_ALL_CONFIGS)

and actually comes from the line
set(CPackComponentsForAll_BUILD_OPTIONS)

>
> That is why I mentioned that you aren't using any quotes most of the
> time while using variables.

When I look at the changes you made,

     set(options "")
     set(oneValueArgs "FILENAME")

and from here

http://www.cmake.org/cmake/help/v3.2/module/CMakeParseArguments.html?highlight=cmake_parse_argument

I do not see the value of putting the quotes in fact, especially around 
FILENAME, since there are already quotes around ${option}, etc in the 
call to cmake_parse_arguments.

>
>>> I haven't researched it further so if you have an option to test it on
>>> Debian that would be great, otherwise I'll provide a fix in the
>>> following days.
>>
>>
>> I won't be able to install a Debian box any time soon.
>
> OK I'll send you the generated data and do the rest of the
> testing/fixing of the test myself.

Please do (refer to my previous message).

>
> There are a few other things to change though.
> Take a look at CPackRPM man page:
> http://www.cmake.org/cmake/help/v3.2/module/CPackRPM.html?highlight=cpack%20rpm
>
> There is an explanation at the top that component variables override
> non component variables and then each non component and component
> variable is explained at the same time - without this the man page
> will get even longer as it already is without a good reason. Please
> change the way you wrote the documentation for your component
> variables.
>
> Also put the text:
> # The value of `CPACK_DEBIAN_<COMP>_PACKAGE_DEPENDS` can be set to an
> empty string
> #  to enable the automatic discovery of dependencies without inheriting from
> #  the default value of :variable:`CPACK_DEBIAN_PACKAGE_DEPENDS`.
>
> inside a Note so that it will be more visible.

Can it be in a subsequent patch or I should rework those? (reworking the 
patches is more work for me.)

Best,
Raffi