Opened 11 years ago

Closed 11 years ago

#237 closed defect (fixed)

loader makefile CFLAGS altered by PGXS

Reported by: kyngchaos Owned by: robe
Priority: medium Milestone: PostGIS 1.4.1
Component: postgis Version: 1.4.X
Keywords: Cc:

Description

It looks like the pgxs include in the loader makefile is altering CFLAGS. On OSX, I use CFLAGS to insert arch flags for a multiarchitecture build (PPC+Intel). PGXS separates compilation and linking flags from CFLAGS, and in it's own rules reinserts these where needed. So, all the object files that don't have rules in the loader makefile (shpopen.o, dbfopen.o, getopt.o) are compiling with my arch flags.

But the custom object rules in the makefile (ie pgsql2shp.o), and the program link rules don't get the benefit of PGXS' reinserting magic, and compile/link without my arch flags.

For example, with a CFLAGS in the makefile:

CFLAGS=-Os -arch ppc -arch i386 -arch ppc64 -arch x86_64  \
-fno-common -DPIC  -Wall -Wmissing-prototypes

make produces (I stripped out some flags to reduce clutter):

gcc -no-cpp-precomp -Os -D_FILE_OFFSET_BITS=64 [snip] -arch ppc -arch i386 -arch ppc64 -arch x86_64 
[snip] -c -o shpopen.o shpopen.c
gcc -no-cpp-precomp -Os -D_FILE_OFFSET_BITS=64 [snip] -arch ppc -arch i386 -arch ppc64 -arch x86_64 
[snip] -c -o dbfopen.o dbfopen.c
gcc -no-cpp-precomp -Os -D_FILE_OFFSET_BITS=64 [snip] -arch ppc -arch i386 -arch ppc64 -arch x86_64 
[snip] -c -o getopt.o getopt.c
gcc -no-cpp-precomp -Os -D_FILE_OFFSET_BITS=64 [snip] -arch ppc -arch i386 -arch ppc64 -arch x86_64 
[snip] -c -o shp2pgsql.o shp2pgsql.c
gcc -no-cpp-precomp -Os -D_FILE_OFFSET_BITS=64 [snip] [***arch flags missing here***]
shpopen.o dbfopen.o getopt.o shp2pgsql.o ../liblwgeom/liblwgeom.a -liconv -lm -o shp2pgsql 
gcc -no-cpp-precomp -Os -D_FILE_OFFSET_BITS=64 [snip] [***arch flags missing here***]
-I/usr/local/pgsql/include -c pgsql2shp.c
gcc -no-cpp-precomp -Os -D_FILE_OFFSET_BITS=64 [snip] [***arch flags missing here***]
shpopen.o dbfopen.o getopt.o pgsql2shp.o ../liblwgeom/liblwgeom.a -liconv -L/usr/local/pgsql/lib -lpq -lm -o pgsql2shp 

(The FILE_OFFSET_BITS and warning flags (the snipped part) between the -Os flag and the arch flags is the result of PGXS separating and reinserting parts of CFLAGS, plus its own stuff.)

If I move the CFLAGS line to after the include $(PGXS), CFLAGS is untouched and you get what was intended by the loader makefile.

Attachments (1)

postgis-loader-cflags.patch (1.3 KB) - added by mcayland 11 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 11 years ago by robe

Owner: changed from pramsey to robe
Status: newassigned

I wonder if this has anything to do with why we can't get shp2pgsql-gui to link under windows? All the os get built but the link to make the exe fails. Though haven't retried.

Does your shp2pgsql-gui get produced?

comment:2 Changed 11 years ago by kyngchaos

I didn't try to compile shp2pgsql-gui, I don't want the hassle of compiling GTK on OSX. But since it has a rule in the makefile, it would have the same problem.

It's not really a problem of not compiling at all - if I left it to build a single architecture (native) it would work. Even with the arch flags it links, just a single native architecture.

If you are adding any linking flags to CFLAGS for Windows compilation, then it's probably affecting you.

comment:3 Changed 11 years ago by mcayland

Yeah... pulling in the PGXS Makefile was a last-minute change before the 1.4 release which allows access to variables such as DESTDIR which are required for installation in different directories (useful for packagers).

So the loader Makefile should ignore any CFLAGS settings. I think the best fix here would be to save CFLAGS before including PGXS and then setting them back to their original value after the file has been included.

If you could find the time to test this and supply a patch, that would be great.

ATB,

Mark.

Changed 11 years ago by mcayland

Attachment: postgis-loader-cflags.patch added

comment:4 Changed 11 years ago by mcayland

Re-reading through this, I figured it would be easier just to move the CFLAGS variable around as per your suggestion and just comment the file appropriately. Does the attached patch seem okay to you?

ATB,

Mark.

comment:5 Changed 11 years ago by kyngchaos

That works.

comment:6 Changed 11 years ago by mcayland

Resolution: fixed
Status: assignedclosed

Committed to 1.4 branch at r4601 and trunk at r4602. Thanks for the report.

ATB,

Mark.

Note: See TracTickets for help on using tickets.