(0016504)
Denis Scherbakov (reporter)
2009-05-18 04:06

??? If the argument is a list, each member in the list will be
tested.

I have a list variable

SET (MYLIST "library1.so;library2.so")

I want to make sure that MYLIST is present, how your function works here?

(0016505)
Marcel Loose (developer)
2009-05-18 05:30

Try the following CMakeLists.txt file, first using the current version of find_package_handle_standard_args(), next using my patched version and compare the output of cmake. Which of the two outputs, do you think, is more intuitive?

project(MyProject)
cmake_minimum_required(VERSION 2.6)
set(MYLIST "library1-NOTFOUND; library2.so")
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(MYPREFIX DEFAULT_MSG MYLIST)

(0016506)
Denis Scherbakov (reporter)
2009-05-18 06:01

Found MYPREFIX: library1-NOTFOUND; library2.so

So what should have been checked? Which two outputs do you mean?

(0016507)
Denis Scherbakov (reporter)
2009-05-18 06:04

Aha, I understand

-- Could NOT find MYPREFIX (missing: library1-NOTFOUND)

My intention was to check that MYLIST is present, I didn't want FPHSA to go into MYLIST contents and check any of them.

"I think this change won't break any existing code." So it does break code.

(0016508)
Denis Scherbakov (reporter)
2009-05-18 06:11
edited on: 2009-05-18 06:12

If I want FPHSA to check a list of variables, I would do:

IF ( CHECK_1_NEEDED )
LIST(APPEND CHLIST "CHECK_1_VARIABLE")
ENDIF ()

IF ( CHECK_2_NEEDED )
LIST(APPEND CHLIST "CHECK_2_VARIABLE")
ENDIF ()

FPHSA(... ${CHLIST})

This will ask FPHSA to check two variables: CHECK_1_VARIABLE CHECK_2_VARIABLE

But case no. 2

SET (CHLIST "library1.so;library2.so")

is different. What you implemented breaks this case.

(0016509)
Marcel Loose (developer)
2009-05-18 07:27
edited on: 2009-05-18 07:30

I looked through all cmake module files (in /usr/share/cmake/Modules) and NONE of them pass the value of a list to FPHSA; i.e. all these modules use CHLIST, instead of ${CHLIST} in your example above. That's why I said that I think it won't break any existing code.

I agree that my change breaks the code above. I'm not a CMake-guru, but do you think it could be possible to change the code so that it supports both uses?

(0016511)
Marcel Loose (developer)
2009-05-18 08:28

On second inspection: the solution suggested by sche_de doesn't work. At least not with the following CMakeLists.txt file. I think that exactly this weird behavior got me started to write this patch.

project(MyProject)
cmake_minimum_required(VERSION 2.6)
SET(CHLIST "library1.so;library2.so")
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(MYPREFIX DEFAULT_MSG ${CHLIST})

produces the following output:
-- Could NOT find MYPREFIX (missing: library1.so library2.so)
-- Configuring done
-- Generating done

(0016512)
Denis Scherbakov (reporter)
2009-05-18 08:38

As I see the current version of the code - it supports both cases. if-endif outside FPHSA as in the example above will do the trick.

What the patch does - it checks inside every list to see if it has NOTFOUND suffix in some of the elements, but what if I want to have MYOPTION-NOTFOUND inside a list? And it is ok for me and it means success for me?

So I think:
1. The patch will break existing code. And it already does.
2. Functionality of the patch can be implemented without patching anything.

(0016513)
Marcel Loose (developer)
2009-05-18 08:43

The reason why this fails is that, in line 36, FPHSA does IF(NOT ${_VAR1}), where it should do IF(NOT _VAR1), if you'd want to pass a list by value.

Check the output of cmake with the CMakeLists.txt below to see what I mean.

project(MyProject NONE)
cmake_minimum_required(VERSION 2.6)
set(var "library1.so")
if(NOT var)
  message("var is FALSE")
else()
  message("var is TRUE")
endif(NOT var)
if(NOT ${var})
  message("\${var} is FALSE")
else()
  message("\${var} is TRUE")
endif(NOT ${var})

FPHSA will even fail if you pass a one-element list *by value*; and the reason is line 36 in FPHSA.

(0016514)
Marcel Loose (developer)
2009-05-18 08:51

What would you expect to be the result of running cmake on this file?

project(MyProject)
cmake_minimum_required(VERSION 2.6)
SET(CHLIST "library1.so")
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(MYPREFIX DEFAULT_MSG ${CHLIST})

According to your suggestion in note 0016508, I guess you assume it will print
-- Found MYPREFIX: library1.so

But it doesn't. It prints:
-- Could NOT find MYPREFIX (missing: library1.so)

Only when I use CHLIST instead of ${CHLIST} will I get the expected behavior.

So, IMHO FPHSA's interface is counter-intuitive, because you cannot pass a list of values to it, neither by reference, nor by value, and get the result you'd expect. It is simply broken!

(0016515)
Denis Scherbakov (reporter)
2009-05-18 08:59

Here is an example that you patch breaks:

PROJECT(MyProject)

CMAKE_MINIMUM_REQUIRED(VERSION 2.6.4 FATAL_ERROR)

LIST(APPEND MYOPTIONS "OPT1-NOTFOUND")
LIST(APPEND MYOPTIONS "OPT2")

# MyPackage is found, when MYOPTIONS variable is set
INCLUDE(FindPackageHandleStandardArgs)
FIND_PACKAGE_HANDLE_STANDARD_ARGS(MyPackage DEFAULT_MSG MYOPTIONS)

=================================
My solution DOES work:

PROJECT(MyProject)

CMAKE_MINIMUM_REQUIRED(VERSION 2.6.4 FATAL_ERROR)

SET(CHECK_1_NEEDED "TRUE")
SET(CHECK_2_NEEDED "TRUE")

IF ( CHECK_1_NEEDED )
  FIND_LIBRARY(VAR_LIBNF NAMES libNotFOuNd NotFOuNd)
  LIST(APPEND CHECKLIST "VAR_LIBNF")
ENDIF ()

IF ( CHECK_2_NEEDED )
  FIND_LIBRARY(VAR_LIBC NAMES libc c)
  LIST(APPEND CHECKLIST "VAR_LIBC")
ENDIF ()

# MyPackage is found, when all libraries in a checklist are found
INCLUDE(FindPackageHandleStandardArgs)
FIND_PACKAGE_HANDLE_STANDARD_ARGS(MyPackage DEFAULT_MSG ${CHECKLIST})

(0016516)
Denis Scherbakov (reporter)
2009-05-18 09:09

>> What would you expect to be the result of running cmake on this file?

See two examples above and comments in them.

>> So, IMHO FPHSA's interface is counter-intuitive, because you cannot pass a list of values to it, neither by reference, nor by value, and get the result you'd expect. It is simply broken!

You can pass a list of variables, and my example is a proof. FPHSA works as expected.

(0016517)
Marcel Loose (developer)
2009-05-18 09:28

OK, that brings us back to square one. I always assumed I could use FPHSA as follows.

set(MYLIBS)
find_library(MYLIB_A NAMES liba)
list(APPEND MYLIBS ${MYLIB_A})
find_library(MYLIB_B NAMES libb)
list(APPEND MYLIBS ${MYLIB_B})
find_package_handle_standard_args(MYPKG DEFAULT_MSG MYLIBS)

But from your example, I see that that's an incorrect assumption. I still think my interpretation is more intuitive, because, with the current implementation, you'll have to maintain a separate list of variable names (containing MYLIB_A and MYLIB_B for the above example) and pass that list by value.
Just my 2 cents.

(0016524)
Marcel Loose (developer)
2009-05-19 03:35

So, what's next? Will you stick with the current design of FPHSA, or do you consider to change its behavior in the way I suggested? AFAIK it doesn't break existing FindXXX macros in the CMake distribution. I don't know for third party stuff, though. But since FPHSA is rather new, there's a reasonable chance it won't break much code.

(0016525)
Denis Scherbakov (reporter)
2009-05-19 05:02
edited on: 2009-05-19 05:03

I am not a CMake developer, but my opinion is completely opposite.

1. Functionality of the patch can be implemented using existing version at ease.
2. The patch breaks existing code.
3. The patch checks variable contents, when I did not ask it to.
4. For every recently introduced function, there are already people, who use it.
5. Many modules are hosted outside CMake.

(0016526)
Marcel Loose (developer)
2009-05-19 05:37

OK, that's clear. I'll just wait and see.

(0017544)
Alex Neundorf (developer)
2009-09-15 15:24

Maybe the issue is actually that
"NOTFOUND" -> false
"blub-NOTFOUND" -> false
"bar;blub-NOTFOUND" -> false
"blub-NOTFOUND;bar" -> true

Should the last one maybe also evaluate to false ?

Alex

(0017551)
Marcel Loose (developer)
2009-09-16 03:46

I just re-read the thread on the mailing list. A couple of months have since passed, so the details are not fresh in my head anymore, but I think you hit the nail on the head.

Quoting from the first mail in that thread:
<quote>
However, XXX_LIBRARIES evaluates to TRUE, even if one or more of its
elements evaluate to FALSE (i.e. one of the libraries was not found and
YYY-NOTFOUND was returned).
</quote>

(0017552)
Denis Scherbakov (reporter)
2009-09-16 03:59

I personally think that LIST contents should not be analyzed at all. I posted a solution, which works without modifying anything. So I would leave everything as is.

(0017553)
Marcel Loose (developer)
2009-09-16 07:45

We had a lengthy discussion about your solution, both through the mail and the bug tracker. The main problem I have with your solution is that you have to keep a separate list of variables that you want to have checked by FPHSA. You only need that list to work around, what I consider, a bug in FPHSA.

I think Alex really found the essence of the problem. The asymmetry in logic -- true;false -> false vs. false;true -> true -- is, rather counter-intuitive, to put it mildly.

The question is not, whether FPHSA should analyze the contents of LIST. The question is more basic: What does it mean to say 'if(LIST)'? In other words, when does LIST evaluate to true and when to false?

(0017765)
Alex Neundorf (developer)
2009-09-26 03:35

Nothing will happen here before 2.8.0 is out.

Alex

(0030575)
Alex Neundorf (developer)
2012-08-13 14:37

Closing this one since the actual issue is now in 0011410 .

Notes
(0016504) Denis Scherbakov (reporter) 2009-05-18 04:06	??? If the argument is a list, each member in the list will be tested. I have a list variable SET (MYLIST "library1.so;library2.so") I want to make sure that MYLIST is present, how your function works here?

(0016505) Marcel Loose (developer) 2009-05-18 05:30	Try the following CMakeLists.txt file, first using the current version of find_package_handle_standard_args(), next using my patched version and compare the output of cmake. Which of the two outputs, do you think, is more intuitive? project(MyProject) cmake_minimum_required(VERSION 2.6) set(MYLIST "library1-NOTFOUND; library2.so") include(FindPackageHandleStandardArgs) find_package_handle_standard_args(MYPREFIX DEFAULT_MSG MYLIST)

(0016506) Denis Scherbakov (reporter) 2009-05-18 06:01	Found MYPREFIX: library1-NOTFOUND; library2.so So what should have been checked? Which two outputs do you mean?

(0016507) Denis Scherbakov (reporter) 2009-05-18 06:04	Aha, I understand -- Could NOT find MYPREFIX (missing: library1-NOTFOUND) My intention was to check that MYLIST is present, I didn't want FPHSA to go into MYLIST contents and check any of them. "I think this change won't break any existing code." So it does break code.

(0016508) Denis Scherbakov (reporter) 2009-05-18 06:11 edited on: 2009-05-18 06:12	If I want FPHSA to check a list of variables, I would do: IF ( CHECK_1_NEEDED ) LIST(APPEND CHLIST "CHECK_1_VARIABLE") ENDIF () IF ( CHECK_2_NEEDED ) LIST(APPEND CHLIST "CHECK_2_VARIABLE") ENDIF () FPHSA(... ${CHLIST}) This will ask FPHSA to check two variables: CHECK_1_VARIABLE CHECK_2_VARIABLE But case no. 2 SET (CHLIST "library1.so;library2.so") is different. What you implemented breaks this case.

(0016509) Marcel Loose (developer) 2009-05-18 07:27 edited on: 2009-05-18 07:30	I looked through all cmake module files (in /usr/share/cmake/Modules) and NONE of them pass the value of a list to FPHSA; i.e. all these modules use CHLIST, instead of ${CHLIST} in your example above. That's why I said that I think it won't break any existing code. I agree that my change breaks the code above. I'm not a CMake-guru, but do you think it could be possible to change the code so that it supports both uses?

(0016511) Marcel Loose (developer) 2009-05-18 08:28	On second inspection: the solution suggested by sche_de doesn't work. At least not with the following CMakeLists.txt file. I think that exactly this weird behavior got me started to write this patch. project(MyProject) cmake_minimum_required(VERSION 2.6) SET(CHLIST "library1.so;library2.so") include(FindPackageHandleStandardArgs) find_package_handle_standard_args(MYPREFIX DEFAULT_MSG ${CHLIST}) produces the following output: -- Could NOT find MYPREFIX (missing: library1.so library2.so) -- Configuring done -- Generating done

(0016512) Denis Scherbakov (reporter) 2009-05-18 08:38	As I see the current version of the code - it supports both cases. if-endif outside FPHSA as in the example above will do the trick. What the patch does - it checks inside every list to see if it has NOTFOUND suffix in some of the elements, but what if I want to have MYOPTION-NOTFOUND inside a list? And it is ok for me and it means success for me? So I think: 1. The patch will break existing code. And it already does. 2. Functionality of the patch can be implemented without patching anything.

(0016513) Marcel Loose (developer) 2009-05-18 08:43	The reason why this fails is that, in line 36, FPHSA does IF(NOT ${_VAR1}), where it should do IF(NOT _VAR1), if you'd want to pass a list by value. Check the output of cmake with the CMakeLists.txt below to see what I mean. project(MyProject NONE) cmake_minimum_required(VERSION 2.6) set(var "library1.so") if(NOT var) message("var is FALSE") else() message("var is TRUE") endif(NOT var) if(NOT ${var}) message("\${var} is FALSE") else() message("\${var} is TRUE") endif(NOT ${var}) FPHSA will even fail if you pass a one-element list by value; and the reason is line 36 in FPHSA.

(0016514) Marcel Loose (developer) 2009-05-18 08:51	What would you expect to be the result of running cmake on this file? project(MyProject) cmake_minimum_required(VERSION 2.6) SET(CHLIST "library1.so") include(FindPackageHandleStandardArgs) find_package_handle_standard_args(MYPREFIX DEFAULT_MSG ${CHLIST}) According to your suggestion in note 0016508, I guess you assume it will print -- Found MYPREFIX: library1.so But it doesn't. It prints: -- Could NOT find MYPREFIX (missing: library1.so) Only when I use CHLIST instead of ${CHLIST} will I get the expected behavior. So, IMHO FPHSA's interface is counter-intuitive, because you cannot pass a list of values to it, neither by reference, nor by value, and get the result you'd expect. It is simply broken!

(0016515) Denis Scherbakov (reporter) 2009-05-18 08:59	Here is an example that you patch breaks: PROJECT(MyProject) CMAKE_MINIMUM_REQUIRED(VERSION 2.6.4 FATAL_ERROR) LIST(APPEND MYOPTIONS "OPT1-NOTFOUND") LIST(APPEND MYOPTIONS "OPT2") # MyPackage is found, when MYOPTIONS variable is set INCLUDE(FindPackageHandleStandardArgs) FIND_PACKAGE_HANDLE_STANDARD_ARGS(MyPackage DEFAULT_MSG MYOPTIONS) ================================= My solution DOES work: PROJECT(MyProject) CMAKE_MINIMUM_REQUIRED(VERSION 2.6.4 FATAL_ERROR) SET(CHECK_1_NEEDED "TRUE") SET(CHECK_2_NEEDED "TRUE") IF ( CHECK_1_NEEDED ) FIND_LIBRARY(VAR_LIBNF NAMES libNotFOuNd NotFOuNd) LIST(APPEND CHECKLIST "VAR_LIBNF") ENDIF () IF ( CHECK_2_NEEDED ) FIND_LIBRARY(VAR_LIBC NAMES libc c) LIST(APPEND CHECKLIST "VAR_LIBC") ENDIF () # MyPackage is found, when all libraries in a checklist are found INCLUDE(FindPackageHandleStandardArgs) FIND_PACKAGE_HANDLE_STANDARD_ARGS(MyPackage DEFAULT_MSG ${CHECKLIST})

(0016516) Denis Scherbakov (reporter) 2009-05-18 09:09	>> What would you expect to be the result of running cmake on this file? See two examples above and comments in them. >> So, IMHO FPHSA's interface is counter-intuitive, because you cannot pass a list of values to it, neither by reference, nor by value, and get the result you'd expect. It is simply broken! You can pass a list of variables, and my example is a proof. FPHSA works as expected.

(0016517) Marcel Loose (developer) 2009-05-18 09:28	OK, that brings us back to square one. I always assumed I could use FPHSA as follows. set(MYLIBS) find_library(MYLIB_A NAMES liba) list(APPEND MYLIBS ${MYLIB_A}) find_library(MYLIB_B NAMES libb) list(APPEND MYLIBS ${MYLIB_B}) find_package_handle_standard_args(MYPKG DEFAULT_MSG MYLIBS) But from your example, I see that that's an incorrect assumption. I still think my interpretation is more intuitive, because, with the current implementation, you'll have to maintain a separate list of variable names (containing MYLIB_A and MYLIB_B for the above example) and pass that list by value. Just my 2 cents.

(0016524) Marcel Loose (developer) 2009-05-19 03:35	So, what's next? Will you stick with the current design of FPHSA, or do you consider to change its behavior in the way I suggested? AFAIK it doesn't break existing FindXXX macros in the CMake distribution. I don't know for third party stuff, though. But since FPHSA is rather new, there's a reasonable chance it won't break much code.

(0016525) Denis Scherbakov (reporter) 2009-05-19 05:02 edited on: 2009-05-19 05:03	I am not a CMake developer, but my opinion is completely opposite. 1. Functionality of the patch can be implemented using existing version at ease. 2. The patch breaks existing code. 3. The patch checks variable contents, when I did not ask it to. 4. For every recently introduced function, there are already people, who use it. 5. Many modules are hosted outside CMake.

(0016526) Marcel Loose (developer) 2009-05-19 05:37	OK, that's clear. I'll just wait and see.

(0017544) Alex Neundorf (developer) 2009-09-15 15:24	Maybe the issue is actually that "NOTFOUND" -> false "blub-NOTFOUND" -> false "bar;blub-NOTFOUND" -> false "blub-NOTFOUND;bar" -> true Should the last one maybe also evaluate to false ? Alex

(0017551) Marcel Loose (developer) 2009-09-16 03:46	I just re-read the thread on the mailing list. A couple of months have since passed, so the details are not fresh in my head anymore, but I think you hit the nail on the head. Quoting from the first mail in that thread: <quote> However, XXX_LIBRARIES evaluates to TRUE, even if one or more of its elements evaluate to FALSE (i.e. one of the libraries was not found and YYY-NOTFOUND was returned). </quote>

(0017552) Denis Scherbakov (reporter) 2009-09-16 03:59	I personally think that LIST contents should not be analyzed at all. I posted a solution, which works without modifying anything. So I would leave everything as is.

(0017553) Marcel Loose (developer) 2009-09-16 07:45	We had a lengthy discussion about your solution, both through the mail and the bug tracker. The main problem I have with your solution is that you have to keep a separate list of variables that you want to have checked by FPHSA. You only need that list to work around, what I consider, a bug in FPHSA. I think Alex really found the essence of the problem. The asymmetry in logic -- true;false -> false vs. false;true -> true -- is, rather counter-intuitive, to put it mildly. The question is not, whether FPHSA should analyze the contents of LIST. The question is more basic: What does it mean to say 'if(LIST)'? In other words, when does LIST evaluate to true and when to false?

(0017765) Alex Neundorf (developer) 2009-09-26 03:35	Nothing will happen here before 2.8.0 is out. Alex

(0030575) Alex Neundorf (developer) 2012-08-13 14:37	Closing this one since the actual issue is now in 0011410 .

Issue History
Date Modified	Username	Field	Change
2009-05-18 03:46	Marcel Loose	New Issue
2009-05-18 03:46	Marcel Loose	File Added: FindPackageHandleStandardArgs.cmake.patch
2009-05-18 04:06	Denis Scherbakov	Note Added: 0016504
2009-05-18 05:30	Marcel Loose	Note Added: 0016505
2009-05-18 06:01	Denis Scherbakov	Note Added: 0016506
2009-05-18 06:04	Denis Scherbakov	Note Added: 0016507
2009-05-18 06:11	Denis Scherbakov	Note Added: 0016508
2009-05-18 06:12	Denis Scherbakov	Note Edited: 0016508
2009-05-18 07:27	Marcel Loose	Note Added: 0016509
2009-05-18 07:30	Marcel Loose	Note Edited: 0016509
2009-05-18 08:28	Marcel Loose	Note Added: 0016511
2009-05-18 08:38	Denis Scherbakov	Note Added: 0016512
2009-05-18 08:43	Marcel Loose	Note Added: 0016513
2009-05-18 08:51	Marcel Loose	Note Added: 0016514
2009-05-18 08:59	Denis Scherbakov	Note Added: 0016515
2009-05-18 09:09	Denis Scherbakov	Note Added: 0016516
2009-05-18 09:28	Marcel Loose	Note Added: 0016517
2009-05-19 03:35	Marcel Loose	Note Added: 0016524
2009-05-19 05:02	Denis Scherbakov	Note Added: 0016525
2009-05-19 05:03	Denis Scherbakov	Note Edited: 0016525
2009-05-19 05:37	Marcel Loose	Note Added: 0016526
2009-07-18 10:42	Philip Lowman	Category	CMake => Modules
2009-09-14 13:14	Bill Hoffman	Status	new => assigned
2009-09-14 13:14	Bill Hoffman	Assigned To	=> Alex Neundorf
2009-09-15 15:24	Alex Neundorf	Note Added: 0017544
2009-09-16 03:46	Marcel Loose	Note Added: 0017551
2009-09-16 03:59	Denis Scherbakov	Note Added: 0017552
2009-09-16 07:45	Marcel Loose	Note Added: 0017553
2009-09-26 03:35	Alex Neundorf	Note Added: 0017765
2010-11-10 12:03	David Cole	Relationship added	related to 0011410
2012-08-13 14:37	Alex Neundorf	Note Added: 0030575
2012-08-13 14:37	Alex Neundorf	Status	assigned => closed
2012-08-13 14:37	Alex Neundorf	Resolution	open => no change required