Makefile cleanup #75
Makefile cleanup #75
Conversation
df7cb
commented
Oct 4, 2023
df7cb
commented
- Simplify moc test handling in Makefile
- Drop support for "create extension from unpackaged"
- Remove support from upgrading from pre-1.2.0 versions
- Remove ancient "alter extension add" files
|
@df7cb wrote:
It doesn't affect our installations, but I'm not sure I agree with this in principle. What's the harm in supporting upgrading from pre-1.2.0 versions? |
|
Look at the mess of files in there, and the code in the Makefile that was used to support the pre-1.2 mess. This is utterly painful to look at, and will likely drive away some people that might have conrtibuted to pgsphere otherwise. It took me several attempts over several months of looking at this to figure out how it actually works, and I do know how to read PG extension Makefiles. 8da3fee#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52 (this commit) might still look okayish, but if you put e2bfda1 next to it, half of the code in the old Makefile was dedicated to the old stuff which is very likely not used anywhere anyway. I don't fancy removing support for old versions, but in this case, I think it does help a lot. @msdemlei, could you chime in here? |
|
On Thu, Oct 05, 2023 at 09:22:03AM +0000, Christoph Berg wrote:
I don't fancy removing support for old versions, but in this case,
I think it does help a lot.
@msdemlei, could you chime in here?
I agree that the upgrade scripts are extremely painful.
You can certainly drop everything that has "gavo" in it. Having an
upgrade option from unpackaged would probably be still a good thing,
so I suspect an unpackaged -> 1.0 script would make sense.
On dropping upgrade scripts for released versions... Well, we just
don't know what versions are still running out there. So, for those
I'd rather be cautious in terms of dropping.
Before I say anything more: Is this problem particularly bad for us?
If so, why? Is it because we have more releases we may have to
upgrade from? Is it because our upgrade script management is
particularly clumsy? Is it because others have more generic upgrade
script that work between many versions thanks to tastefully applied
IF EXISTS-s and friends?
|
|
Fwiw "remove everything with gavo in it" is exactly what this patch is doing. The upgrade path from unpackaged went to some gavo version, then through some more gavo versions, and then to 1.2. The problem is worse for pgsphere than the average extension because all the .sql files are compiled from smaller bits. This is basically a good idea, but the Makefile rules for compiling the intermediate gavo (and "unpackaged) files were extremely convoluted and painful to look at. With the cleanup, the Makefile now makes sense when looking at it. |
|
I upgraded our pgsphere installations from 1.1.something not that long ago (on the order of months), so I feel it is premature to drop support for upgrading from older versions. While I'm sure the files could be tidied up some, personally, I did not find the Makefile or the upgrade scripts to be particularly confusing when I submitted a PR a while back. I recommend adding more comments and structure to the Makefile to make it less confusing over dropping support for upgrading from older versions. |
|
What about the "from unpackaged" part? That's unsupported since PG13, can we remove that yet? |
Sure! I don't think anyone has objected to that (or the other changes). |
|
Aye, I'll rebase that part when I get back from vacation after next week. |
Remove has_explain_summary since it's only relevant with PG 9.x; move all the if(USE_HEALPIX) sections into one.
PG 13 drops support for "create extension from old_version". Remove support for "from unpackaged". Existing users will likely have converted to an extension-style install years ago.
|
I rebased this branch to current master and dropped the "drop 1.2" part. I verified that upgrading from 1.0 (Debian package version There is a problem with the spoint3 change from #74 (the opclass from 1.0 already contained the function at that time), but that problem is independent from this PR; this PR is good to be merged as far I am concerned. I'll address the problem separately. [*] The repo doesn't contain any tags from before 1.1.5, so I used that version instead |
|
Fwiw if we want the git history to look pretty, I can rebase each of the other PRs after one of then has been merged. |
|
@df7cb Thank you for the patch! I think we can merge it. I propose to merge this patch first. About tags. I restored the tags in this repo down to 1.1.5 but I was not able to identify commits of older versions (issue #9). If you have some ideas how to find commits for older tags, please, let me know. |
|
@df7cb One more forgotten thing... Could you please squash the commits? I use the merge option to put PR into the master. It preserves the authorship. I'm not sure about preserving of the authorship when I squash the commits using github. Thank you! |
|
@vitcpp the changes are logically separated, so I would recommend not to squash them. (In any case, GitHub preserves the commit authors.) |
