postgrespro / pgsphere

PgSphere provides spherical data types, functions, operators, and indexing for PostgreSQL.
https://pgsphere.org
BSD 3-Clause "New" or "Revised" License
16 stars 15 forks source link

Remove the obsolete text from the doc #65

Closed vitcpp closed 9 months ago

esabol commented 9 months ago

While you are at it, could you also remove the USE_PGXS=1 from the make commands on lines 75 and 81 of doc/install.sgm? That's completely outdated and unnecessary.

Setting PG_CONFIG still works, but that's mostly unnecessary as well.

df7cb commented 9 months ago

I agree that setting PGXS on the command line is obsolete since the Makefile already contains it, but setting PG_CONFIG is still the primary interface to selecting the target PG version.

make PG_CONFIG=/usr/lib/postgresql/15/bin/pg_config

esabol commented 9 months ago

Sure, but I feel that's part of the postgres user's environment in most cases. At least I always have it set, and I always update that setting whenever I install a new major version release. So I stand by my statement that setting it in the make command is "mostly unnecessary", but I have no objection to keeping it in the documentation.

vitcpp commented 9 months ago

@esabol I think, in past, there was the scenario when pgsphere was built as a part of postgresql sources tree (contrib/pg_sphere) - lines 88-100 in the top Makefile. Now, pgsphere is not the part of the postgresql tree. I guess, it is why USE_PGXS option was introduced. If we still need to support such scenario we probably should remove USE_PGXS=1 from the makefile and oblige users to set this option in the Makefile as shown in the doc. If we do not need to support such scenario we should cleanup the Makefile, remove obsolete code.

vitcpp commented 9 months ago

I think to keep USE_PGXS=1 in the doc because it does nothing. The pg_config is used unconditionally. But, if we decide to support the build in the contribution directory without pg_config, we have to remove the explicit setting of USE_PGXS=1 from the Makefile. It will not affect the developers, who get used to specifying USE_PGXS=1.

df7cb commented 9 months ago

Please don't remove USE_PGXS from the Makefile. People would wish to build pgsphere from within a PG source tree can still use make USE_PGXS=. But I've never seen anyone do that, so let's not bother the other 99.9% of the users with that flag.

vitcpp commented 9 months ago

@df7cb Yes, sure, i'm not going to remove USE_PGXS=1 from the Makefile without discussions and approvals. I just concerned if to put pgsphere into contrib and to run make world, we will have some problems with USE_PGXS=1 flag in the Makefile. It is why I propose to keep USE_PGXS=1 option in the doc. It may be redundand but it may help us to do the changes in the future.

esabol commented 9 months ago

It is why I propose to keep USE_PGXS=1 option in the doc. It may be redundand but it may help us to do the changes in the future.

I disagree. It is useless and confusing (because it disagrees with the instructions in README). I have no objection to keeping it in the Makefile, however. I just don't think it should be in the documentation.

But whatever. It's no big deal. :smile:

I compile pgsphere in my contrib directory all the time, and it compiles just fine with USE_PGXS=1 set in the Makefile.

vitcpp commented 9 months ago

@esabol pg_config is used to get the path to the postgresql Makefile.global which implements some common rules. When you set USE_PGXS=1, pg_config at your PATH is used to get the path to Makefile.global. It doesn't matter where you put your pgsphere code.

If you want to compile pgsphere with using of postgresql Makefile.global that is located in your project tree, then you have to set USE_PGXS=0 to disable pg_config because it can return the path to Makefile.global of another installation. In this case, pgsphere Makefile will try to find the postgresql Makefile at path ../..//src/Makefile.global (line 98 of pgsphere Makefile).

pgsphere is not the part of the contrib directory. I'm ok to remove USE_PGXS from the doc. But in this case we have to rewrite the Installation chapter, because there are two build scenarios are described. I propose to keep the only one compilation scenario and to remove the description how to compile in the contrib.

What do you think?

esabol commented 9 months ago

Yeah, I understand how the build system works.

I propose to keep the only one compilation scenario and to remove the description how to compile in the contrib.

What do you think?

Yes, please.

vitcpp commented 9 months ago

@esabol I've updated the Installation section. Could you please verify it? Thank you in advance!

vitcpp commented 9 months ago

Rebased