Opened 14 years ago

Closed 14 years ago

#311 closed defect (fixed)

Rename postgis.sql to postgis-1.5.sql

Reported by: pramsey Owned by: mcayland
Priority: medium Milestone: PostGIS 1.5.0
Component: postgis Version: master
Keywords: Cc:

Description

Make the SQL install file reflect the version it was birthed from. Also the uninstall file.

Attachments (3)

postgis-contrib-directory-hack.patch (639 bytes ) - added by mcayland 14 years ago.
postgis-file-versioning.patch (3.6 KB ) - added by mcayland 14 years ago.
postgis-contrib-directory.patch (5.0 KB ) - added by mcayland 14 years ago.

Download all attachments as: .zip

Change History (38)

comment:1 by robe, 14 years ago

I guess I will state my preferred here for the record. I would much prefer keep the sql file names the same and instead have a versioned script folder

So we would have contrib\ postgis-1.5\postgis.sql postgis-1.5\spatial_ref_sys.sql postgis-1.5\postgis_…upgrade from here to there.sql (like hmm 3 of these things) postgis-1.5\postgis_comments.sql postgis-1.5\extras\allthose extra things we package like topology.sql ….and whatever nonsense we dream of in the future. postgis-1.5\postgis_uninstall.sql

The reason being look already how many sql files I have itemized. Imagine having to deal with different versioned named things of those all in one directory. And oops I want to uninstall …

Yah we can say but why do we need to version spatial_ref_sys.sql. Hmm because people may install stuff out of order. Lets say for whatever reason you need to install a PostGIS 1.3 — your stupid PostGIS 1.3 wipes out the newer PostGIS 1.5 you had already installed. Thus I think spatial_ref_sys.sql should be versioned like everything else.

To Kevin — NO we will not be MICRO versioning the script files for the simple reason that when I install a PostGIS 1.4.1 I expect it to wipe out my PostGIS 1.4 scripts without having to uninstall my prior scripts.

I have spoken.

comment:2 by pramsey, 14 years ago

Owner: changed from pramsey to mcayland

comment:3 by mcayland, 14 years ago

Sadly creating subdirectories for module content is something which PGXS in its current incarnation cannot do particularly well :(

After wrestling for several hours with various bits of Makefile, I eventually realised that I was trying to make things more complicated than they should be. Looking at the PGXS source file, the installation code uses the following to install into datadir:

$(INSTALL_DATA) $$file '$(DESTDIR)$(datadir)/contrib'

So I've implemented the attached hack that alters datadir from its original value so that "make install" will now place things under ~/share/postgresql/contrib/postgis-X.Y/contrib. Alas it should be obvious from the above that I can't remove the trailing contrib from the PATH without altering PGXS :(

Can people have a play with this and let me know if they think it's an acceptable solution? Ultimately I'd like to bump something better up to the PostgreSQL guys, however this should go some way to solving the immediate problem.

by mcayland, 14 years ago

comment:4 by pramsey, 14 years ago

Doesn't feel like an improvement to me. I'd rather just dump all the files into share/contrib as currently happens.

in reply to:  4 comment:5 by kneufeld, 14 years ago

I concur. Having a named directory sounds like it could be a nice thing to have, but with the trailing config … I think it would cause more questions as it looks like a mistake. My preference would be to do as Mark suggests - punt this back to the PostgreSQL folks. We can use the current convention for now.

comment:6 by mcayland, 14 years ago

Well I prefer the directory hack version for its simplicity, but I have also engineered the alternative which is putting the version number in the .sql and _comments.sql files. Please try the attached proof of concept patch.

One thing to note using this method is that it will be necessary to patch the documentation to insert version numbers into any references to postgis.sql/uninstall_postgis.sql.

by mcayland, 14 years ago

comment:7 by robe, 14 years ago

Oh boy do I hate both of these. I hate Mark's folder cludge a little less than the lots of files in one folder solution. I vote for a DO NOTHING until we have a more sane solution.

People are confused enough as it is with file names changing.

comment:8 by pramsey, 14 years ago

I vote for versioning them in one folder. We'll do better for 2.0. Continuing to have a versioned .so/.dll and not having a corresponding versioned .sql file defeats the purpose.

comment:9 by robe, 14 years ago

Can I keep the windows installer the way it is? Its kind of annoying to change right now and I already had instituted a separate folders with postgis-1.4, because I really can't use pgxs for packaging windows anyway.

Sorry to be a "I don't care what you do on your OS, but leave my OS alone", kind of person.

Personally since we do seem somewhat undecided — though we haven't written it in our PSC (where is our code of conduct? hmm). I think the solution is to bring it up on PostGIS dev / Users for a vote to break the stalemate. Since its users and packagers who are affected most. I personally don't want decisions hanging on my feelings of course.

My concern here is that PostGIS 1.4 is already realease so there is a postgis.sql out there we techncailly can't get rid of. so in light of a postgis.sql and postgis-1.5.sql (and all those n upgrade .sqls), what is a user going to do.

comment:10 by pramsey, 14 years ago

It's important that all the installs be fundamentally similar, so we don't end up with a situation where we have to write "if you're on Windows, if you're on Linux" codas in every step-by-step document we produce.

comment:11 by robe, 14 years ago

Well sorry windows is already different has separate folders and too late to change now (well definitely not for the 1.4 series). The names can be changed as you said, but still will be separate folders on windows. Besides folders aren't consistent on linux anyway.

So if you really want to name the files different — okay — but I keep my separate folders too on windows :). Its just way easier for uninstall — I just remove the folder.

comment:12 by pramsey, 14 years ago

Well, this is the very last thing that needs to be finalized before release.

Can you write down precisely *what* the Windows install schema is, so I don't have to figure it out by installing and then reading back?

And BTW, shame on you Cowboy Regina for breaking cross platform compatibility!

comment:13 by robe, 14 years ago

Just download the experimental and that's the way the structure is.

contrib/postgis (this is folder shared by all that contains proj) contrib/postgis-1.4 contrib/postgis-1.5

Only difference, which I really regret not being more of a cowboy on is the template database is still called template_postgis when I install instead of being versioned (unlike what the experimental batch scripts have)

comment:14 by mcayland, 14 years ago

Okay I've just written a prototype patch for the PGXS makefile which allows subdirectories under contrib/ and submitted it to the pgsql-hackers list. I'd suggest we wait for feedback on the patch, and then when it's in a suitable form we simply ifdef…endif an include to an external file containing the aneurism-inducing Makefile hackery so it can be easily removed in the future.

comment:15 by pramsey, 14 years ago

What's the timeline on this? Can we close out next week?

comment:16 by robe, 14 years ago

FWIW, I saw Mark's post to Hackers list and other's responses, so I assume we are really close.

http://archives.postgresql.org/pgsql-hackers/2009-12/msg02259.php

comment:17 by mcayland, 14 years ago

Yeah; I've just submitted a revised version to -hackers for comment today.

comment:18 by mcayland, 14 years ago

Okay folks. Tom committed a revised version of my patch here:

http://archives.postgresql.org/pgsql-committers/2010-01/msg00025.php

And so please find attached the relevant hackery for PostgreSQL < 8.5 as postgis-contrib-directory.patch. So it's a particularly ugly hack, but by basing it upon the 8.5 patch and putting all the horrible nastiness in a separate file, it's fairly easy to switch the logic based upon the PostgreSQL version and remove it as soon as PostgreSQL 8.5 becomes our minimum supported version.

The only slightly annoying part is that because we are overriding 3 of the PGXS targets then we get the following warnings when compiling on PostgreSQL < 8.5:

Makefile.pgxs:17: warning: overriding commands for target `install' /home/pg83/rel-8.3.7/lib/postgresql/pgxs/src/makefiles/pgxs.mk:96: warning: ignoring old commands for target `install' Makefile.pgxs:60: warning: overriding commands for target `installdirs' /home/pg83/rel-8.3.7/lib/postgresql/pgxs/src/makefiles/pgxs.mk:143: warning: ignoring old commands for target `installdirs' Makefile.pgxs:79: warning: overriding commands for target `uninstall'

These are, fortunately, harmless but I couldn't find a way of suppressing them. I've tested this on PostgreSQL 8.3 and 8.5 and it works as expected for me, so I'm putting it out here for the others to test.

by mcayland, 14 years ago

comment:19 by pramsey, 14 years ago

Works on OS/X

comment:20 by pramsey, 14 years ago

Committed the postgis-contrib-directory.patch patch at r5105 to trunk.

comment:21 by pramsey, 14 years ago

Resolution: fixed
Status: newclosed

comment:22 by robe, 14 years ago

Resolution: fixed
Status: closedreopened

Okay finally got around to testing and testing the 1.50.b1. This doesn't quite work.

It puts the .sql files in the right place and installs the loader exes, but it never installs postgis-1.5.dll (though I do see it built as libpostgis-1.5.dll and sitting in the postgis source folder).

Is there a hard-coding somewhere. I got this notice during configure and I don't even have my mingw compiled postgres 8.3 in /usr/local/pgsql/8.3 but I do have all those paths in my /c/postgres83/lib folder so why on earth is it looking at that location.

make[1]: Entering directory `/c/thesrc/postgis-1.5.0b1/postgis'
Makefile:80: /usr/local/pgsql/8.3/lib/pgxs/src/makefiles/pgxs.mk: No such file or directory
make[1]: *** No rule to make target `/usr/local/pgsql/8.3/lib/pgxs/src/makefiles/pgxs.mk'.  Stop.
make[1]: Leaving directory `/c/thesrc/postgis-1.5.0b1/postgis'

comment:23 by mcayland, 14 years ago

Nope, I'm afraid there is no hard-coding anywhere - the path to the PGXS Makefile is always generated by "pg_config —pgxs".

Did you try and point to a different pg_config using —with-pgconfig? If so, you may be suffering from the old PGXS bug where a pg_config in PATH will still always take precendence over one specified using —with-pgconfig. Note this bug was fixed in PostgreSQL 8.3.

So search hard for multiple pg_configs on your system…

comment:24 by nicklas, 14 years ago

For me in MinGW it installs postgis-1.5.dll in lib-directory and the sql-files in share\contrib\postgis-1.5\

so I think it works as it should for me.

/Nicklas

comment:25 by robe, 14 years ago

Actually that problem went away second time around when I compiled —with-gui for my 8.3 install so you are probably right it might be a path thing since this is the box I have 8.2,8.3, and 8.4 installed on and maybe the changes in gui flipped path.

But for my 8.3 install (which is the one I had tried earlier), it does not install the postgis-1.5.dll, but for my 8.4 install everything works perfectly.

Nicklas are you compiling for 8.3 or 8.4?

I'll give 8.3 another try.

comment:26 by nicklas, 14 years ago

I was trying 8.4.

I can try 8.3 too.

comment:27 by robe, 14 years ago

Okay I think the difference betweeen 8.3 and 8.4 is that for 8.3 the dll gets built as libpostgis-1.5.dll and via some magic when we install we make that postgis-1.5.dll

For 8.4 it gets built as postgis-1.5.dll so no magic is needed. So I suspect its something amiss in this magical step of taking the libpostgis-1.5.dll and making it postgis-1.5.dll that is not working.

comment:28 by nicklas, 14 years ago

As expected I get the same result as Regina.

The libpostgis-1.5.dll is created but nothing is installed.

comment:29 by pramsey, 14 years ago

This really should be a new ticket… anyhow, it is also affecting the in-place 'make check'. In my regress directory I have

paul-ramseys-macbook-2:regress pramsey$ ls -l 00-regress-install/*
00-regress-install/bin:
total 1944
-rwxr-xr-x  1 pramsey  staff  320296 11 Jan 20:26 pgsql2shp
-rwxr-xr-x  1 pramsey  staff  320952 11 Jan 20:26 shp2pgsql
-rwxr-xr-x  1 pramsey  staff  344384 11 Jan 20:26 shp2pgsql-gui

00-regress-install/share:
total 0
drwxr-xr-x  3 pramsey  staff  102 11 Jan 20:26 contrib

Note: no ./lib directory. It's like that part isn't being installed at all.

comment:30 by pramsey, 14 years ago

A little more investigation: note that I'm testing under PgSQL 8.3. The Makefile.pgxs seems not to be doing the lib install because of an 'ifdef MODULES" test which is coming up false. For whatever reason, MODULES is actually not defined. And MODULE_big, which is defined, is 'postgis-1.5' which does not match the library we generate, which is 'libpostgis-1.5.so'

comment:31 by mcayland, 14 years ago

Ah wait a second - because of the differences between the old and new Makefiles, I missed the MODULE_big section which had been moved somewhere else :( I've just done a test with latest SVN on 8.3 and 8.4, and I believe this is now fixed. Please test and feed back.

comment:32 by pramsey, 14 years ago

That fixed the in-place regression for me against 8.3

comment:33 by robe, 14 years ago

Okay my 8.3 compiles and installs fine now under mingW. I still have an issue with make check that I didn't have with PostGIS 1.4 ever since that whole pre-install make check thing was introduced. Could be something funky with my batch script for packaging though. I haven't investigated the error yet.

Nicklas, have you been able to do a full make check under mingW.

comment:34 by mcayland, 14 years ago

From what people are reporting, it seems that the sed command is working correctly and "make install" now works.

So could it just be a simple file permissions problem? Bear in mind that the build directories will have files owned by the currently logged in user, whereas in order for the backend to access the files under regress/00-regress-install then they must also be readable by the postgres Windows user (or whatever user was specified as part of the PostgreSQL install).

comment:35 by robe, 14 years ago

Resolution: fixed
Status: reopenedclosed

fixed after Mark's last change. The make check is a separate issue so will open another ticket for that.

Note: See TracTickets for help on using tickets.