Opened 14 years ago

Closed 13 years ago

#447 closed defect (fixed)

Changes in liblwgeom do not trigger rebuild of postgis shared object

Reported by: strk Owned by: mcayland
Priority: medium Milestone: PostGIS 2.0.0
Component: postgis Version: master
Keywords: Cc:

Description

Very annoyingly, it always take a make clean when you only change liblwgeom…

Attachments (2)

postgis-liblwgeom-dependency.patch (1.6 KB ) - added by mcayland 13 years ago.
postgis-liblwgeom-dependency-v2.patch (482 bytes ) - added by mcayland 13 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 by mcayland, 14 years ago

Yeah. I've had an idea as to how to fix this by breaking the list of OBJs for liblwgeom into a separate include file that we can use in both Makefiles. But in the meantime, I normally do a "touch postgis/lwgeom_pg.c; make install" which works for me.

Mark.

comment:2 by pramsey, 14 years ago

Milestone: PostGIS 2.0.0

comment:3 by pramsey, 13 years ago

Owner: changed from pramsey to mcayland

comment:4 by mcayland, 13 years ago

What do you think about the following patch?

by mcayland, 13 years ago

comment:5 by mcayland, 13 years ago

Actually - some more testing here with that patch shows that make combines the dependency information from an .o file with no actions with the PGXS build actions so that everything "just works"TM. This means I can get away with the attached revised v2 patch too.

comment:6 by strk, 13 years ago

it's weird for .o files to depend on an .a file isn't it ? Shouldn't it be the final linked thing only ? This patch would rebuild all .o files which is unneeded, unless _headers_ from liblwgeom are changed.

comment:7 by mcayland, 13 years ago

I think there are several different scenarios going on here, for example:

1) liblwgeom API/struct change → you want to rebuild everything 2) liblwgeom tinkering (e.g. adding extra LWDEBUG(F) statements) → you only want to rebuild one particular file

What you're suggesting would work for scenario 1) but not scenario 2) as there is no API change involved :(

comment:8 by strk, 13 years ago

I'm suggesting to encode _more_ dependencies.

*.o files would depend on *.h files in liblwgeom libpostgis-<version>.<ext> would depend on liblwgeom.a

does it make sense ?

comment:9 by mcayland, 13 years ago

So you mean break out the headers to one .h file per .c file? Sure that could work, but that's going to involve quite a bit more fiddling around and there are still edge cases where it wouldn't work (e.g. if a struct was declared in a .c rather than a .h file). Given the amount of work involved, I'm inclined just to stick with the v2 patch unless you want to do the extra refactoring?

comment:10 by strk, 13 years ago

Not really. It's fine for a rule to rebuild _more_ than needed. Main problem here is building _less_ so as long as that's fixed I'm happy. I was just suggest to slightly reduce the unnecessary rebuilds but not as much as tracking individual .h files.

I say go ahead with your patch and we see how it goes. We'll always be able to file new tickets if we want to improve further. So we avoid getting stuck on details…

comment:11 by mcayland, 13 years ago

Resolution: fixed
Status: newclosed

Agreed - it's a starting point that we can always refine later, but at least it solves the main issue which is people forgetting to force-rebuild PostGIS when working with liblwgeom.

Committed to trunk at r7571 - I don't feel we should port this to 1.5 branch as it's a new behavour and people may have some objections to it. We could reconsider at a later date if everyone gets on with it though.

Note: See TracTickets for help on using tickets.