Bug 276559
| Summary: | databases/pgbackrest: Update to 2.51 | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Michael Schout <schoutm> | ||||||||||||||||||||
| Component: | Individual Port(s) | Assignee: | Zsolt Udvari <uzsolt> | ||||||||||||||||||||
| Status: | Closed FIXED | ||||||||||||||||||||||
| Severity: | Affects Only Me | CC: | uzsolt | ||||||||||||||||||||
| Priority: | --- | ||||||||||||||||||||||
| Version: | Latest | ||||||||||||||||||||||
| Hardware: | Any | ||||||||||||||||||||||
| OS: | Any | ||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||
Could you please update to 2.51? Could you please check "portfmt -D Makefile" and "portclippy Makefile" (ports-mgmt/portfmt)? Thanks! I've been traveling. I will update today. Created attachment 249679 [details]
Upgrade to pgbackrest 2.51
Update to pgbacrest 2.51, fix portfmt/portclippy issues.
Thanks. What do you think the "note to packagers"? https://github.com/pgbackrest/pgbackrest/releases/tag/release%2F2.51 "NOTE TO PACKAGERS: The build system for pgBackRest is now meson. The autoconf/make build will not receive any new features and will be removed after a few releases." Do you change to meson at the next version? Well, thats cheerful news. I will take a stab at changing to meson. (In reply to Michael Schout from comment #5) Okay, I'll wait it. Thanks. Created attachment 249837 [details]
Ugrade to pgbackrest 2.51, switch to meson, add flags for liblz4 and libssh2
I switched the build system to meson and took the time to pull in config args for new features.
- Switch to meson build system
- Added patch for src/meson.build to make "meson install" actually install the binary
- Added config options for optional libs: liblz4, libssh2
Didn't see any obvious way to replicat the "autoconf fatal if optional lib is missing" in meson, so I dropped these:
ZSTD_CONFIGURE_OFF= ac_cv_lib_zstd_ZSTD_isError=no
ZSTD_CONFIGURE_ON= ac_cv_lib_zstd_ZSTD_isError=yes
I suspect it would involve significant gymnastics if we were to try to replicate that in meson. If I'm mistaken let me know.
(In reply to Michael Schout from comment #7) Thanks! You should use USES=python instead of BUILD_DEPENDS (see https://docs.freebsd.org/en/books/porters-handbook/book/#using-python and https://cgit.freebsd.org/ports/tree/Mk/Uses/python.mk#n7). The poudriere reports it ("Error: lang/python3 should not be depended upon. Instead, use USES=python:xy with a specific version."). Options: don't you set any meson-variable? If a user build this port with "make install" and the liblz4 is installed on her machine the pgbackrest will use it but the LZ4 option is off. What do you think: are needed these options? The lz4, ssh2, zstd is widely used? If yes remove them from options and simply add to LIB_DEPENDS. I'll fix USES python today. As for the options: - ssh2 is almost certainly *not* widely used as it was not even available until the past couple of releases upstream. - zstd, same situation, but added longer ago. - lz4 has been there for very long time and we should probably just depend on that one automatically. And yeah, if they do "make install" with e.g. lz4 turned off but its installed, the port is going to use it. Will see if I can work out how to get around that (IE explicitly turn it off). Ok, worked out how get the features disabled. Will involve some patching of `meson.build`. When done will submit new patch. Will also send the build patches upstream to make this easier in future releases. Created attachment 249860 [details]
Explicitly disable deps that are turned off, switch to meson builds, update to v2.51
Updated patch which ensures we do not link against any options that are disabled.
I think this addresses all remaining concerns. Please let me know if there is anything else thanks!
Created attachment 249876 [details]
Use meson build system, update to 2.51, add SFTP config option
I updated the patch to use the one that got merged upstream.
I also updated the SSH option description to match the one from upstream (it is only used to enable SFTP support).
I am good to get this merged now.
Created attachment 249877 [details]
Use meson build system, update to 2.51, add SFTP config option
Fix portclippy warnig.
Created attachment 249878 [details]
Use meson build system, update to 2.51, add SFTP config option
Accidentally updloaded wrong patch file last time, sorry.
Thanks. The post-patch target: if you want use sed, it's better the REINPLACE_CMD (see https://docs.freebsd.org/en/books/porters-handbook/book/#slow-patch-automatic-replacements). In your special case the best solution is shebangfix: https://docs.freebsd.org/en/books/porters-handbook/book/#uses-shebangfix (In reply to Michael Schout from comment #12) "I also updated the SSH option description to match the one from upstream (it is only used to enable SFTP support)." If this option enables the SFTP support, maybe want rename the option SSH to SFTP? I think it would be more logic and the SSH option is a base option (https://cgit.freebsd.org/ports/tree/Mk/bsd.options.desc.mk#n478). (In reply to Zsolt Udvari from comment #16) "shebangfix": sorry, I misunderstood your patch. In your case should use BINARY_ALIAS (https://docs.freebsd.org/en/books/porters-handbook/book/#binary-alias). Created attachment 249971 [details]
Update to pgbackrest 2.51, use meson build system, add SFTP config option
Latest patch which I think addresses all of your concerns.
If anything else is needed please let me know
(In reply to Michael Schout from comment #18) Thanks for your hard work. IMHO this patch is perfect but should review by mentors: https://reviews.freebsd.org/D44791 (In reply to Zsolt Udvari from comment #19) Could you please check https://reviews.freebsd.org/D44791#inline-270417 and https://reviews.freebsd.org/D44791#inline-270416 ? Regarding the review comments about where the patches originate, they are direct from upstream, added after 2.51 was released. When 2.52 is available these patches wont be part of the port any longer. They are only here for purposes of updating FreeBSD to v2.51. I'll change the makefile to use helpers, but wont have time until tomorrow at earliest. I can see with upstream about changing how to find the pythong binary. Not sure if that one would be worth holding up the 2.51 update for or not. Created attachment 250182 [details]
Use meson build system, update to 2.51, add SFTP config option
Addressed MR comments:
- Use Helpers for MESON config options
I submitted the "python3" path patch upstream. If its accepted, we can just remove the "BINARY_ALIAS" at next release.
Please let me know if there is anything else as this port is outdated now for quite some time.
The patch to dynamically find "python" got merged upstream. Again, will be in 2.52 but is not in current release version. I'd prefer to stick with the short couple-lines patch I made here meanwhile since the upstream one adds a lot more changes for CI and tests. In 2.52 we should not need any local patches as all of the issues were addressed upstream. See: https://github.com/pgbackrest/pgbackrest/pull/2338 Regarding your comment in review:
> Use of GH_TAGNAME would be better than patches? Or the Example 18. in Porter's Handbook?
This is a bad idea IMO.
We should *NOT* base the package on an unreleased upstream version that potentially has other non-build-related unproven changes.
Especially for something as mission critical as "pgbackrest"
We should stick to released code only. The patches involved here are solely for the build and nothing else.
I would further propse that if pulling in the upstreamed build patches here is somehowi problematic we should just skip 2.51 and wait for the next release. Alternatively, drop use of meson for 2.51 and revisit in 2.52 when patches aren't needed. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=43ecb93463973dbe210d4fd5dc2158b962a012bc commit 43ecb93463973dbe210d4fd5dc2158b962a012bc Author: Michael Schout <schoutm@gmail.com> AuthorDate: 2024-04-14 16:08:30 +0000 Commit: Zsolt Udvari <uzsolt@FreeBSD.org> CommitDate: 2024-04-27 05:48:06 +0000 databases/pgbackrest: update to 2.51 Switch to meson build system. Rename option SSH to SFTP. The origin of patch is https://github.com/pgbackrest/pgbackrest/commit/7b95fd3bd29346e7325aa161bfee45efd71e22eb It is upstreamed and can remove it at update to 2.52. Changelog: https://pgbackrest.org/release.html#2.51 PR: 276559 Approved by: diizzy (mentor) Differential Revision: https://reviews.freebsd.org/D44791 databases/pgbackrest/Makefile | 30 +++++++++++++--------- databases/pgbackrest/distinfo | 6 ++--- databases/pgbackrest/files/patch-meson.build (new) | 28 ++++++++++++++++++++ .../files/patch-meson__options.txt (new) | 8 ++++++ .../pgbackrest/files/patch-src_meson.build (new) | 10 ++++++++ 5 files changed, 67 insertions(+), 15 deletions(-) Committed, thanks! |
Created attachment 247893 [details] Upgrade to pgbackrest 2.50 Attached patch updates databases/pgbackrest to version 2.50