Le 22/04/15 08:01, Domen Vrankar a écrit :
2015-04-21 23:38 GMT+02:00 Raffi Enficiaud <[email protected]>:
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
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers