Created attachment 589245 [details] A fix for FindPostgreSQL.cmake Description of problem: This is a fedora leg of a bug reported filed against RHEL, bug 825186. The problem is in CMakeOnly.AllFindModules, which fails for FindPostgreSQL module. (This finding is courtesy of Gregor Jasny, whom I CC.) This failure is caused by PostgreSQL header files having a different layout. Apparently, due to multiarch conflicts, /usr/include/pg_config.h is now a dispatcher to a number of pg_config_$arch.h header files that contain what used to be pg_config.h. I'm not sure whether this is an upstream arrangement or a Fedora-specific patch. I'm CCing Tom Lane, who might actually know about this. The fix that I present simply checks for pg_config_$arch.h first, and falls back on pg_config.h if that is not found. Version-Release number of selected component (if applicable): cmake-2.8.8-4 How reproducible: Always. Steps to Reproduce: 1. install postgresql-devel 2. in CMake build directory, issue bin/ctest -R CMakeOnly.AllFindModules 3. observe failure Actual results: The following tests FAILED: 196 - CMakeOnly.AllFindModules (Failed) Expected results: The following tests passed: CMakeOnly.AllFindModules
(In reply to comment #0) > This failure is caused by PostgreSQL header files having a different layout. > Apparently, due to multiarch conflicts, /usr/include/pg_config.h is now a > dispatcher to a number of pg_config_$arch.h header files that contain what > used to be pg_config.h. Um, yeah, it's been that way in RHEL and Fedora since about 2005 ...
I checked that neither Debian sid (the package is called libpq-dev), nor openSUSE 10.3 do this. I'm not sure it makes sense to upstream this patch. Maybe it should compile a program that outputs the version to stdout, so that we don't depend on parsing arbitrary C preprocessor expressions. This might be upstreamable and would work across distros. I'll contact upstream about their opinions.
Not sure why you're finding this quite so astonishing. AFAIK this is the standard method for dealing with build-time-generated include files that contain architecture dependencies. In the rather small set of packages that I maintain, there are three others that use the same trick; and a quick poke around /usr/include on my Fedora box finds a few more. If CMake can't cope with this then postgresql is just the tip of the iceberg.
Tom's right of course. Imo, cmake checks that grep header files are particularly fragile, and should be avoided if at all possible. In this case, trying to grok PG_VERSION. I wonder if we could get away with parsing the output from `pg_config --version` instead (or something else that's reasonably simple and safer)
Or maybe feed #include <pg_config.h> PG_VERSION to the C preprocessor, and look at the output of that?
(In reply to comment #3) > Not sure why you're finding this quite so astonishing. I'm not. I point out that others don't do this merely to illustrate that this is RHEL and Fedora specific, and upstream therefore may be reluctant to accept the patch. > If CMake can't cope with this then postgresql is just the tip of the iceberg. It can, as witnessed by patch attached to comment 0. (In reply to comment #5) > Or maybe feed > > #include <pg_config.h> > PG_VERSION > > to the C preprocessor, and look at the output of that? Yes, this is roughly what I had in mind in comment 2, except I'd probably make it a full binary that just outputs the string and exits, so that we don't have to deal with all the directives that the cpp produces. This seems to be the most portable solution.
Re: the last comment 'make it a full binary tha tjust outputs the string and exits' => or just parse output of already-existing 'pg_config --version' as suggested in comment #4 (no extra compiling required)
Well, the issue with either pg_config or a special binary is whether you care about the possibility of the header files not matching that executable. I'm not clear on what the goal of this test is at all. Is it to know whether postgres is installed (and for that why don't you just ask rpm)? Or is it to know whether you can compile stuff against the headers? If the latter, probing pg_config seems a couple of steps removed from what you really want to test.
via /usr/share/cmake/Modules/FindPostgreSQL.cmake # - Find the PostgreSQL installation. # In Windows, we make the assumption that, if the PostgreSQL files are installed, the default directory # will be C:\Program Files\PostgreSQL. # # This module defines # PostgreSQL_LIBRARIES - the PostgreSQL libraries needed for linking # PostgreSQL_INCLUDE_DIRS - the directories of the PostgreSQL headers # PostgreSQL_VERSION_STRING - the version of PostgreSQL found (since CMake 2.8.8) This is all the information that currently needs to be queried
(In reply to comment #9) > # This module defines > # PostgreSQL_LIBRARIES - the PostgreSQL libraries needed for linking > # PostgreSQL_INCLUDE_DIRS - the directories of the PostgreSQL headers > # PostgreSQL_VERSION_STRING - the version of PostgreSQL found (since CMake > 2.8.8) > > This is all the information that currently needs to be queried In that case I think you can get it all just by asking pg_config, and that would probably be the most reasonable way to go about it. The main downside I can think of is pg_config not being available --- but I don't know of any distros where it's not included in either the base package or the -devel package, so that seems probably not a case to worry about.
Created attachment 596289 [details] A fix for FindPostgreSQL.cmake This fetches all necessary values from pg_config. This required more changes in the code, and frankly I have no idea whether it works on Windows.
Filed upstream: http://public.kitware.com/Bug/view.php?id=13378
One problem - This fails the cmake build test checks with: 25: module: /builddir/build/BUILD/cmake-2.8.8/Tests/FindModulesExecuteAll/../../Modules/FindPostgreSQL.cmake 25: CMake Error at /builddir/build/BUILD/cmake-2.8.8/Modules/FindPostgreSQL.cmake:114 (message): 25: Couldn't determine PostgreSQL configuration. 25: Call Stack (most recent call first): 25: /builddir/build/BUILD/cmake-2.8.8/Modules/FindPostgreSQL.cmake:130 (fail_if) 25: CMakeLists.txt:16 (include) I think we need to change SEND_ERROR to WARNING or similar.
Building with that change.
(In reply to comment #14) > Building with that change. Thanks, apparently I didn't test the case that PostgreSQL is not installed.
Last upstream comment: I think a middle-ground approach is to run pg_config if it is available and otherwise fall back to parsing the header. Any multiarch install should provide pg_config and any non-multiarch install (e.g. on Windows) should not have a forwarding header. Petr - could you produce such a patch against the current cmake git?