Le 22/04/15 08:01, Domen Vrankar a écrit :
2015-04-21 23:38 GMT+02:00 Raffi Enficiaud <raffi.enfici...@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.

Hi,

I am having a look now at the changes you made on the first patch (say 75b0e1679c39ca824a4c49d9e1a2ae2b5f04ae06), file RunCPackComponentsDEB/RunCPackVerifyResult-components-lintian-dpkgdeb-checks.cmake

Instead of finding lintian and dpkg-deb in the check file
find_program(LINTIAN_EXECUTABLE lintian)
find_program(DPKGDEB_EXECUTABLE dpkg-deb)

why not returning the string "NOTFOUND" in eg. the functions run_lintian lintian_output, like this:

set(${lintian_output} "NOTFOUND" PARENT_SCOPE)


If I understand correctly this should evaluate to false in an if() and we should be able to make the distinction with an empty string. On the other hand, we can add a variable that may return "ok", "notfound" or "error".


What are the best practices there?

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

Reply via email to