Bug 253612 - databases/galera26: Fix build on FreeBSD13
Summary: databases/galera26: Fix build on FreeBSD13
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Fernando Apesteguía
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-18 08:18 UTC by Tod McQuillin
Modified: 2021-02-19 12:46 UTC (History)
2 users (show)

See Also:
devel: maintainer-feedback+


Attachments
Patch to databases/galera26 (1.93 KB, text/plain)
2021-02-18 08:18 UTC, Tod McQuillin
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tod McQuillin 2021-02-18 08:18:19 UTC
Created attachment 222548 [details]
Patch to databases/galera26

galera26 is currently marked as broken on FreeBSD 13 due to __bswap_64_var being undefined.

In the unpatched source code for galera26, in galerautils/src/gu_byteswap.h, we have:

#elif defined(__FreeBSD__)
/* do not use bswapXX, because gcc44 gives old-style cast warnings */
#  define gu_bswap16 __bswap16_var
#  define gu_bswap32 __bswap32_var
#  define gu_bswap64 __bswap64_var

Based on the comment, this appears to be a workaround for a problem that no longer exists (because we don't use gcc44 anymore).  This worked by using __bswap_xx_var which in earlier versions of FreeBSD was an implementation detail of the bswapXX macros.  This detail no longer exists in FreeBSD 13 and later.

The correct thing to do is simply use the bswap16, bswap32 and bswap64 macros as provided by FreeBSD.

Later in the gu_byteswap.h file it checks for these macros and defines accordingly:

#elif defined(bswap16)
#  define gu_bswap16 bswap16
#  define gu_bswap32 bswap32
#  define gu_bswap64 bswap64

So the easiest fix here is simply to remove any special-casing for FreeBSD in galerautils/src/gu_byteswap.h.

Attached is a patch to update the port to do exactly this, and remove the BROKEN_FreeBSD_13, BROKEN_FreeBSD_14, and BROKEN_riscv64 lines.

I have tested that this change allows the port to build for FreeBSD 11.4, 12.2 and 13.0 on amd64.
Comment 1 Tod McQuillin 2021-02-18 08:30:43 UTC
(In reply to Tod McQuillin from comment #0)

On reflection it's probably a good idea to bump PORTREVISION for this change, since it does make a small change to the build (using bswapXX() instead of __bswapXX_var() ).

Although this should not change any functionality; it might actually change the binary output of the compiler so probably a PORTREVISION bump is called for.
Comment 2 Fernando Apesteguía freebsd_committer 2021-02-19 07:58:08 UTC
Q/A: WARN: /data/fernape_data/FreeBSD-repos/ports/head/databases/galera26/files/patch-galerautils_src_gu__byteswap.h: patch was not generated using ``make makepatch''.  It is recommended to use ``make makepatch'' when you need to [re-]generate a patch to ensure proper patch format.

No need to upload new patch.
Comment 3 Fernando Apesteguía freebsd_committer 2021-02-19 09:09:56 UTC
Builds fine for me.

Let's give some time to maintainer before committing this.
Comment 4 devel 2021-02-19 10:48:35 UTC
The patch looks good, though simpler solution seems to be removing

#elif defined(__FreeBSD__)
/* do not use bswapXX, because gcc44 gives old-style cast warnings */
#  define gu_bswap16 __bswap16_var
#  define gu_bswap32 __bswap32_var
#  define gu_bswap64 __bswap64_var

completely. It was needed for some ancient compilers and looks now redundant.
Comment 5 Tod McQuillin 2021-02-19 10:52:31 UTC
(In reply to devel from comment #4)
This is exactly what my patch does.  It may be hard to see just by reading the patch since the contents also references the previous FreeBSD patch in the context diff.  But the result of applying the patch is to produce a new patch like this:

--- galerautils/src/gu_byteswap.h.orig	2020-10-12 15:33:51.000000000 +0900
+++ galerautils/src/gu_byteswap.h	2021-02-18 16:52:23.630953000 +0900
@@ -53,11 +53,6 @@
 #  define gu_bswap16 _OSSwapInt16
 #  define gu_bswap32 _OSSwapInt32
 #  define gu_bswap64 _OSSwapInt64
-#elif defined(__FreeBSD__)
-/* do not use bswapXX, because gcc44 gives old-style cast warnings */
-#  define gu_bswap16 __bswap16_var
-#  define gu_bswap32 __bswap32_var
-#  define gu_bswap64 __bswap64_var
 #elif defined(__sun__)
 #  define gu_bswap16 BSWAP_16
 #  define gu_bswap32 BSWAP_32

Ideally this change could be included upstream as well so we don't have to maintain a patch in the port.
Comment 6 devel 2021-02-19 11:05:54 UTC
We will include these changes for the next release.

Btw, I tried to set maintainer approval flag for the patch, but the change does not seem to be effective. Is something else needed before that?
Comment 7 Fernando Apesteguía freebsd_committer 2021-02-19 11:09:56 UTC
(In reply to devel from comment #6)
Can you set maintainer-approval in the PR instead?
Comment 8 commit-hook freebsd_committer 2021-02-19 12:46:32 UTC
A commit references this bug:

Author: fernape
Date: Fri Feb 19 12:46:27 UTC 2021
New revision: 566059
URL: https://svnweb.freebsd.org/changeset/ports/566059

Log:
  databases/galera26: Fix build on FreeBSD13

  Use bswap macros as provided by FreeBSD.

  While here regenerate some patches and tidy Makefile up.

  PR:	253612
  Submitted by:	devin@sevenlayer.studio
  Approved by:	devel@galeracluster.com (maintainer)

Changes:
  head/databases/galera26/Makefile
  head/databases/galera26/files/patch-galerautils_src_gu__byteswap.h
Comment 9 Fernando Apesteguía freebsd_committer 2021-02-19 12:46:55 UTC
Committed,

Thank you both!