Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#1694 closed defect (fixed)

PostGIS 2.0.0 does not build with clang

Reported by: vince Owned by: pramsey
Priority: medium Milestone: PostGIS 2.0.1
Component: postgis Version: trunk
Keywords: history Cc: sharpie

Description

The transformation of sql.in files into sql by means of CPP -traditional-cpp fails if CPP = clang -E.

Besides, I've tried various overrides to change the CPP value to some gcc version, but to no avail. The build system seems particularly obscure when it comes to how CPP is inferred.

Can it be simplified? Or the preprocessing of sql.in files adapted for clang?

Attachments (1)

use_gpp_if_available.patch (4.4 KB) - added by strk 6 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 7 years ago by sharpie

Builds fine for me using Clang 3.1 b318 from Xcode 4.3 on OS X 10.7.3.

comment:2 Changed 7 years ago by vince

On my side, I get a bunch of these:

/usr/bin/clang -E -I../../postgis/ -I../../ -traditional-cpp rtpostgis.sql.in.c | grep -v '^#' > rtpostgis.sql.in
rtpostgis.sql.in.c:276:83: error: empty character constant
        AS $$ SELECT st_band($1, regexp_split_to_array(regexp_replace($2, '[[:space:]]', '', 'g'), $3)::int[]) $$
                                                                                         ^
rtpostgis.sql.in.c:3174:79: warning: missing terminating ' character
                                                -- Check first if the pixel intersects with the geometry. Often many won't.

comment:3 Changed 7 years ago by sharpie

Which version of Clang are you using? Also, does this happen during make or make install?

comment:4 Changed 7 years ago by vince

-> clang -v
Apple clang version 4.0 (tags/Apple/clang-420.0.9) (based on LLVM 3.1svn)

comment:5 in reply to:  4 Changed 7 years ago by sharpie

Replying to vince:

-> clang -v
Apple clang version 4.0 (tags/Apple/clang-420.0.9) (based on LLVM 3.1svn)

Hmm... interesting. Looks like you are trying to compile on a Mac same as I am.

Which version of PostGIS/version of Xcode/version of Mac OS are you using?

I should note that I am building the beta3 release of PostGIS.

comment:6 Changed 7 years ago by vince

Oops. I am compiling from trunk. I wasn’t aware there was a tarball available, I'm going to retry with it. I'm preparing a suitable Portfile for the macports project.

comment:7 in reply to:  6 Changed 7 years ago by sharpie

Replying to vince:

I'm preparing a suitable Portfile for the macports project.

Off-topic, but I recently updated the Homebrew installation script for the PostGIS 2.0 betas to work around the opinionated nature of the PostGIS installation routines. Maybe the script would help in your efforts:

https://github.com/mxcl/homebrew/blob/master/Library/Formula/postgis.rb

I recently worked with the PostGIS Debian package, so when 2.0 is released I will probably clean the script up by using make install DESTROOT=<temporary staging directory> and then move the pieces from the staging directory into their final places.

comment:8 Changed 7 years ago by vince

Maybe we should carry on using emails. You can contact me using my Macports address, which is the same five letters pseudo at macports.org. Thanks! Vince

comment:9 Changed 7 years ago by robe

BTW - I'm having issues on mingw64 as well though it only seems to be an issue during make check (e.g. build of load_topology.sql load_topo not the actual build of the final files. Not sure they are related though. Used to work fine for me in make check before so wasn't sure if it was when I upgraded my mingw64 environment.

comment:10 Changed 7 years ago by vince

I have to replace $(CPP) by cpp in the various Makefiles where $(CPP) appears to get the SQL functions properly generated. Then, however, xsltproc phase fails:

/opt/local/bin/xsltproc ./xsl/topology_comments.sql.xsl postgis-out.xml > topology_comments.sql
http://www.oasis-open.org/docbook/xml/4.5/ent/isoamsn.ent:1: parser error : Content error in the external subset
HTTP/1.1 200 OK
^
http://www.oasis-open.org/docbook/xml/4.5/ent/isoamsn.ent:1: validity error : All markup of the conditional section is not in the same entity
HTTP/1.1 200 OK
^
http://www.oasis-open.org/docbook/xml/4.5/ent/isoamsn.ent:1: parser error : Content error in the external subset
HTTP/1.1 200 OK
   ^
http://www.oasis-open.org/docbook/xml/4.5/ent/isoamsn.ent:1: validity error : All markup of the conditional section is not in the same entity
HTTP/1.1 200 OK
   ^
http://www.oasis-open.org/docbook/xml/4.5/ent/isoamsn.ent:1: parser error : Content error in the external subset
HTTP/1.1 200 OK
      ^
unable to parse postgis-out.xml

And I have to alter manually the LDFLAGS line of the liblwgeom Makefile because it seems not to take into account the LDFLAGS environment variable…

comment:11 Changed 7 years ago by pramsey

Milestone: PostGIS 2.0.0PostGIS 2.0.1

comment:12 Changed 7 years ago by vince

Okay, I’ve got a build. In summary, I had to:

  1. Substitute $(CPP) for cpp (or /usr/bin/cpp) in all the makefiles using $(CPP) for transforming *.sql.in in *.sql;
  2. Modify the Makefile in extensions/postgis to pull out any comment processing by xsltproc;
  3. Add a line LDFLAGS += … in the Makefile of liblwgeom in order to correctly build a OS X universal version.

comment:13 Changed 6 years ago by strk

I'd like to look at http://www.nothingisreal.com/gpp/ for a more general approach to preprocessing.

Changed 6 years ago by strk

Attachment: use_gpp_if_available.patch added

comment:14 Changed 6 years ago by strk

Try use_gpp_if_available.patch and installing gpp

comment:15 Changed 6 years ago by strk

robe: also, do you have cpp installed ? Does it accept -traditional-cpp and -P switches ?

comment:16 Changed 6 years ago by vince

I can confirm that cpp works. It is part of the patch I developed.

comment:17 Changed 6 years ago by strk

Please try r9639 -- it will use a standalone cpp if available (passing also the -P switch).

If it fails, report the 'SQL preprocessor' line reported by ./configure

comment:18 Changed 6 years ago by strk

If we can count on "-P" working we could also get rid of some "grep -v '#'" in the makefiles...

comment:19 Changed 6 years ago by vince

The problem is that PostGIS makefile derives its values for CPP, CFLAGS… from the PGXS Makefile. There should be a way to override these default values.

An other bug caused by this dependency: if, for example, I build PostgreSQL as a universal Apple binary (i386/x86_64) then I am forced to build also a universal version of PostGIS, even if I don’t want it… because the flags used to compile PostGreSQL are engraved in PGXS Makefile…

comment:20 Changed 6 years ago by strk

Vince: did you try r9639 ? I'm using CPPBIN to build an SQLPP, so CPP is only used IFF a cpp executable isn't found.

You'll need to run ./autogen.sh and ./configure again after an svn up..

comment:21 Changed 6 years ago by vince

No, I was speaking about the 2.0.0 release. I'll try a fresh svn version as soon as I can. Thanks for pointing this out!

comment:22 Changed 6 years ago by vince

It works. You can safely close the ticket (at least for me). Thanks!

comment:23 Changed 6 years ago by strk

Resolution: fixed
Status: newclosed

Thanks for testing

comment:24 Changed 6 years ago by robe

Keywords: history added

comment:25 Changed 6 years ago by sharpie

Cc: sharpie added

Looks like this is still a bug.

I wasn't experiencing it earlier because Homebrew was using /usr/bin/cpp which is a wrapper script on OS X that points to llvm-gcc. However, we recently made some changes that cause clang to be used as the pre-processor if it is also being used as the C compiler. After these changes, I see the same errors Vince reported earlier.

A diff of the SQL scripts and Postgres extension files produced by llvm-gcc as compared to clang (version 3.1) can be found here:

https://gist.github.com/4d943acc25f81b1ffe9c

(un-truncated versions can be found at https://gist.github.com/4d943acc25f81b1ffe9c/366d6bd1379e80f1042adfe0b580653a97fd9e19)

The problems with Clang seem to be related to whitespace---it looks like all the indentation is being stripped. The trunk version of Clang has downgraded the errors to warnings, but the end result is still the same and indentation is stripped.

Trying with GPP produces the following result:

https://gist.github.com/65057cf9fb6a56a5929c

GPP seems like a drop in replacement for the GCC pre-processor---except it has a problem inserting URLs into the comments.

There are two unresolved issues here:

  • Apple will be dropping llvm-gcc from XCode followig the next major release, so it would be prudent to ensure Clang can be used as a pre-processor.
  • GPP provides a reasonable fall-back if supporting Clang is unfeasible. However, there is currently no way to configure a build using gpp as the preprocessor short of hacking configure.ac and re-running autogen.sh. This is distasteful from a packaging standpoint.

comment:26 Changed 6 years ago by vince

Hi Sharpie,

Thanks for pointing out that /usr/bin/cpp is just a script wrapper in 10.8. I haven’t noticed. Well, then the workaround I wrote for Macports is fragile and, as you rightly emphasize, limited in time to the next XCode major release. Since it seems utterly preposterous to build a gcc compiler just for cpp availability, something must be done upstream. However, I faintly remember that clang 3.2 has now a -traditional-cpp option. Did you test it?

Cheers, Vincent

comment:27 Changed 6 years ago by strk

Sharpie: try to add -C to the CPP call ? in configure.ac:

  • SQLPP="${CPPBIN} -traditional-cpp -P"

+ SQLPP="${CPPBIN} -traditional-cpp -P -C"

Note: See TracTickets for help on using tickets.