Page MenuHomeFreeBSD

update and clean postgis (postgresql) and depends ports
ClosedPublic

Authored by lbartoletti on Sep 25 2018, 7:27 PM.
Tags
None
Referenced Files
F103474284: D17320.id55009.diff
Mon, Nov 25, 12:12 PM
Unknown Object (File)
Mon, Nov 25, 8:16 AM
Unknown Object (File)
Mon, Nov 25, 1:05 AM
Unknown Object (File)
Sun, Nov 24, 11:37 PM
Unknown Object (File)
Sun, Nov 24, 8:36 PM
Unknown Object (File)
Sun, Nov 24, 2:51 PM
Unknown Object (File)
Sun, Nov 24, 2:06 PM
Unknown Object (File)
Sun, Nov 24, 2:06 PM

Details

Summary

Postgis version 2.5.0 was released this weekend. I take this opportunity to sort the different versions (EOLed) .

As pgRouting has also been updated this weekend, I'm upgrading to a new version in this diff.

Test Plan

poudriere 11/12 i386/amd64

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
lbartoletti retitled this revision from update, clean and flavorize postgis (postgresql) and depends ports to update, clean and flavorize postgis (postgresql) and depends ports.

Update databases/Makefile

databases/postgis/Makefile
65–66

That's a no on flavors.

The ports tree has a *default* postgresql version, only the default will be built.

If you want to add postgresql flavors, which is most certainly a bad idea, it should go in USES=pgsql.

It is a bad idea because the way postgresql works in the ports tree is bad and should be changed. We should only have one postgresql-client, most probably the latest version available, and everything, including the different versions of servers should use it. The only flavors should be for ports that need the server, and in any case, it should go in USES=pgsql.

postgis/Makefile
22 ↗(On Diff #48497)

Why += ?

37 ↗(On Diff #48497)

Wrong place in the Makefile. See Chapter 15. Order of Variables in Port Makefiles.

61–62 ↗(On Diff #48497)

USES=localbase

80 ↗(On Diff #48497)

Remove this, and use options helpers.

In D17320#369596, @mat wrote:

If you want to add postgresql flavors, which is most certainly a bad idea, it should go in USES=pgsql.

It is a bad idea because the way postgresql works in the ports tree is bad and should be changed. We should only have one postgresql-client, most probably the latest version available, and everything, including the different versions of servers should use it. The only flavors should be for ports that need the server, and in any case, it should go in USES=pgsql.

Thank you, @mat.
Indeed, I also think there is some change to be made on the postgresql side. I had already started to create a USE for postgis before the existence of flavours, but which was not committeed at the time (and which I did not continue).

Here is my need, to be able to install pgrouting versions that can take the latest version of postgis and the versions of postgresql present (95, 96, 10) directly in the pkg, without the need to compile; ok, I can make packages with poudriere but if it is directly in the official pkg it was better.

Following your advice, I will propose a new one diff that removes flavors and does only the cleaning of postgis, its upgrade and the upgrade of pgrouting.

RASTER is a mandatory to postgis extension: CONTROL file

lbartoletti retitled this revision from update, clean and flavorize postgis (postgresql) and depends ports to update and clean postgis (postgresql) and depends ports.Oct 30 2018, 8:57 PM

Is it OK for you?

rhurlin added inline comments.
databases/postgis/Makefile
79

D17320 in general works fine for me on a semi production box, many thanks.

I wonder if the lib version should be something other than 0.0.0? This never changed in our PostGIS ports before. Looking into [1], PostGIS in its internal structures uses macros for it:

#define LIBLWGEOM_VERSION "2.5.1dev"
#define LIBLWGEOM_VERSION_MAJOR "2"
#define LIBLWGEOM_VERSION_MINOR "5"
#define LIBLWGEOM_GEOS_VERSION "38"

[1] PostGIS liblwgeom.h File Reference

lbartoletti added inline comments.
databases/postgis/Makefile
79

Should be ${STRIP_CMD} ${STAGEDIR}${PREFIX}/lib/liblwgeom-${VER}.so.${PORTVERSION} instead?

I don't quite understand the rationale here. According to https://postgis.net/source/, 2.3 and 2.4 are supported. Why remove them.

databases/postgis/Makefile
62

Please, remove this, and use options target helpers.

databases/postgis/Makefile
79

To be honest, I am not an expert in library naming schemes. For me, it seems ok.

In D17320#383536, @mat wrote:

I don't quite understand the rationale here. According to https://postgis.net/source/, 2.3 and 2.4 are supported. Why remove them.

I had made a proposal to be able to have a postgis USE to select the version we want but it was never followed (except rainer ;) ).

What I would like is to be able to have packages postgresql10-postgis25 postgresql95-postgis25 ... postgresql10-postgis24, postgresql95-postgis24... etc. ala debian, but I don't think this is feasible and what about maintainability?

In any case, I doubt that there is a real need to maintain several versions of postgis concurrently. Rainer, what's your opinion?

If you think we should leave the other ports, I will. On the other hand, all ports that must depend on postgis will depend on the latest current version.
I also recommend that a "pkg install postgis" install the latest version.

databases/postgis/Makefile
79

If I'm not wrong, it's generated in a file. You should leave it like that. see https://github.com/postgis/postgis/blob/svn-trunk/Version.config#L10

In D17320#385326, @lbartoletti_tuxfamily.org wrote:

[..snip..]

In any case, I doubt that there is a real need to maintain several versions of postgis concurrently. Rainer, what's your opinion?

If you think we should leave the other ports, I will. On the other hand, all ports that must depend on postgis will depend on the latest current version.
I also recommend that a "pkg install postgis" install the latest version.

I would be fine with having only one PostGIS version for all PostgreSQL versions. Version 2.5 will work with all existing PostgreSQL versions except of 9.3, which is EOL now[1].

On the other hand, there are several older versions already installed. For these versions, the users have to migrate their PostGIS data to the newer version 2.5. This migration can be painful with complex data ...

Another problem could be, that future versions of PostgreSQL on FreeBSD again will need two PostGIS versions, e.g. 2.5 and 3.x

[1] https://trac.osgeo.org/postgis/wiki/UsersWikiPostgreSQLPostGIS

[..snip..]
I would be fine with having only one PostGIS version for all PostgreSQL versions. Version 2.5 will work with all existing PostgreSQL versions except of 9.3, which is EOL now[1].

On the other hand, there are several older versions already installed. For these versions, the users have to migrate their PostGIS data to the newer version 2.5. This migration can be painful with complex data ...

mmm... I agree with you. OK to let all versions still supported upstream. But pgrouting and (others?) will be linked by default with the latest version of PostGIS.

Another problem could be, that future versions of PostgreSQL on FreeBSD again will need two PostGIS versions, e.g. 2.5 and 3.x

These are two major versions, so for me, it doesn't cause me any problems to have these two versions together.

[1] https://trac.osgeo.org/postgis/wiki/UsersWikiPostgreSQLPostGIS

This revision is now accepted and ready to land.Nov 26 2018, 3:56 PM

Mat, Rainer, do you accept the latest changes?
Thanks

In D17320#393738, @lbartoletti_tuxfamily.org wrote:

Mat, Rainer, do you accept the latest changes?

Hi Loïc,
Yes, for me, it seems ok. I am fine with your changes.

Thanks for your work, really appreciated.

databases/postgis/Makefile
21

Wrong place in the Makefile. See Chapter 15. Order of Variables in Port Makefiles.

38

Why +=?
Also, wrong place in the Makefile.

57–58

Wrong place in the Makefile.

databases/postgis/pkg-install
25–27 ↗(On Diff #50648)

I really do not like this.

Beside the in-line comment:

  • You could save some lines with "${STRIP_CMD} ${STAGEDIR}${PREFIX}/bin/*" in post-install:.
  • Please fix %%PORTVERSION%% substitution in PLIST, e.g. "%%ADDRSTD%%share/postgresql/extension/address_standardizer--%%PORTVERSION%%--2.5.0next.sql".
  • Please sort PLIST.

Thank you for your hard work on postgis.

MOVED
10531–10533

Please use databases/postgis for the second field.

databases/postgis/Makefile
8
23

According to PostGIS Support Matrix, PostGIS 2.5 supports pgsql:9.4+.

49–50

It is SFCGAL_CONFIGURE_WITH=sfcgal=${LOCALBASE}/bin/sfcgal-config

54–55

Please remove it if not needed.

57–58

Is it still needed when you have USES=localbase already?

60

Since you use it in the Makefile, please also use it in PLIST via PLIST_SUB for consistency, e.g. lib/liblwgeom-2.5.so.0.0.0.

databases/postgis/pkg-descr
16
databases/postgis/pkg-install
1–37 ↗(On Diff #50648)

Is it needed? The hard-coded stdc++ in geos has been removed in its Makefile.

databases/postgis/Makefile
63

This seems dubious, PRE-INSTALL is what gets shown when the package is being installed, just before the installation. I do not see how things could be needed to be done twice, once before configure runs, and once before the package is installed.

lbartoletti added inline comments.
databases/postgis/Makefile
21

There is no information on this subject in the handbook. I modified using the example of security/libressl

38

PLIST_SUB is also not indicated in the manual.

lbartoletti marked 2 inline comments as done.
This revision now requires review to proceed.Dec 15 2018, 9:29 PM

Thank you mat and sunpoet.
I corrected the different points and deleted the pkg-install file which was obsolete (thanks sunpoet).

In D17320#395984, @lbartoletti_tuxfamily.org wrote:

Thank you mat and sunpoet.
I corrected the different points and deleted the pkg-install file which was obsolete (thanks sunpoet).

@mat @sunpoet gentle ping to a (last hope ;) ) review. Thank you

LGTM. BTW, it would be great to sort USES, e.g. USES=... pgsql pkgconfig ...

Thanks for your work!

databases/postgis/Makefile
68–73

Similar to stripping bin/*, these can be merged to post-install: as "${STRIP_CMD} ${STAGEDIR}${PREFIX}/lib/postgresql/*.so"

LGTM. BTW, it would be great to sort USES, e.g. USES=... pgsql pkgconfig ...

Thanks for your work!

Done. Thank you

global post-install strip and sort USES for pgrouting

I feel databases/postgis should be named databases/postgis25.

In D17320#412341, @mat wrote:

I feel databases/postgis should be named databases/postgis25.

On the contrary, I have changed this logic so that the latest version is simply called "postgis". This simplifies the updating of the ports that depend on it and you can install the latest version without worrying about the version number.

The only disadvantage to this is that in case of package updates, it will be necessary to update the database with the new extension. What do you think about that @rhurlin_gwdg.de ?

Hi Loïc,

I also tend to simplifying the name of the latest postgis ports version. The problem is, as you said before, that 'automated' updates like with pkg, poudriere, portmaster etc. do not point out, that a second step like 'ALTER EXTENSION postgis* UPDATE' is necessary.

It is something like updating PostgreSQL itself and doing a dump before and a restore after the update, or pg_upgrade, depending on the situation.

It would be nice to have a really good message for users after the PostGIS update process is done. This is often tricky, because several databases could need this ALTER EXTENSION, accessible by different users. But this is also the case with a PostGIS port, numbered by its version (e.g. postgis25).

Perhaps, something like a user tunable knob and a mechanism for automated post upgrade steps could be of interest here?

Hi rainer,

It's a good idea and I think it should be complementary to https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=213038

For this tour, I propose to stay on the name "postgis25" in order to merge it and we look at that in another ticket.

Hi Loïc,

For me, it would be totally fine to stay with the name postgis25 for now :)

rename databases/postgis -> databases/postgis25

brooks added a subscriber: brooks.

Remove core, the herald rule was over broad.

Personal note: I'm unconvinced the triggering text "GNU General Public License" belongs in pkg-descr.

Personal note: I'm unconvinced the triggering text "GNU General Public License" belongs in pkg-descr.

This pkg-descr was here before me, I can change it, but please I would like to do it in another PR to update all Postgis ports.

Update postgis to 2.5.2 and pgroutin 2.6.1

This revision was not accepted when it landed; it landed in state Needs Review.Mar 15 2019, 12:47 AM
This revision was automatically updated to reflect the committed changes.