[cmake-developers] find_package without REQUIRED can cause fatal error now

[David Cole]

On Tue, Jul 31, 2012 at 2:30 PM, Alexander Neundorf <neundorf at kde.org> wrote:
> On Tuesday 31 July 2012, Bill Hoffman wrote:
>
>> On 7/30/2012 3:38 PM, Alexander Neundorf wrote:
>
>> > The idea was that this is a sign of a broken install.
>
>> >
>
>> > Can you uninstall the vtk-devel package (does this exist ?), because it
>
>> > is broken, i.e. does not contain everything is claims to ?
>
>> >
>
>> > I understand that this is maybe not expected behaviour for an optional
>
>> > package, but maybe Brad is right ?
>
>> >
>
>> > Alex
>
>>
>
>> Yes, VTK is broken on the machine, and we could fix the machine.
>
>> However, this would be the first time we did a work around for the
>
>> modules test by fixing the machine. I wonder if we could just disable
>
>> fatal error at the start of find_package optional. Then turn it back on
>
>> at the end. My concern is optional packages, maybe that is a rare case
>
>> not sure.
>
> One reason why I wanted it to be an error was to force people to fix their
> install, make it is obvious that it is not the fault of the buildsystem of
> the project being built, but of the package being used.
>
> I'm not sure it is better to fail relatively silently in this case instead
> of complaining loudly that the package is broken.
>
> Users might wonder why it doesn't work, the devel package is installed, in
> the right place, and still CMake doesn't pick it up.
>
> I can have a look how to make it not an error if the find_package() is
> optional, if we really want that.
>
> Alex
>
>
> --
>
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Please keep messages on-topic and check the CMake FAQ at:
> http://www.cmake.org/Wiki/CMake_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

One thing Brad and I discussed briefly was simply changing our
generated code to something like this pseudo-code:

if(file is missing)
  if(optional)
    message(WARNING "package is broken -- file ${file} is missing")
    set(FOUND FALSE)
    # allow find_package to fall through to trying to find the next
installation of this package if there are more possibilities...
  else()
    message(FATAL_ERROR "as we have now and have had since 2.8.7")
  endif()
endif()

And then you still have to possibly deal with non-generated
misbehaving project config files, but at least for the scripts we
generate that get included at find_package time, we will not trigger
an error in the optional case.

How does that sound as a compromise position?

Either way, I vote to put this into 'master' immediately AFTER 2.8.9,
and not hold up the release for trying to rush in a last minute change
of this magnitude. I think it's too risky, and the behavior exists in
2.8.7 and 2.8.8 already.


David C.