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

[Domen Vrankar]

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 "")...

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

>> 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.

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.

Thanks,
Domen