Bug 253612

Summary: databases/galera26: Fix build on FreeBSD13
Product: Ports & Packages Reporter: Tod McQuillin <devin>
Component: Individual Port(s)Assignee: Fernando Apesteguía <fernape>
Status: Closed FIXED    
Severity: Affects Many People CC: brd, devel, fernape
Priority: --- Flags: devel: maintainer-feedback+
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Patch to databases/galera26 none

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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 2021-02-19 12:46:55 UTC
Committed,

Thank you both!
Comment 10 Brad Davis freebsd_committer freebsd_triage 2021-03-03 16:31:38 UTC
Fernando, can you also apply these to databases/galera?
Comment 11 Fernando Apesteguía freebsd_committer freebsd_triage 2021-03-04 06:41:57 UTC
(In reply to Brad Davis from comment #10)
Hi Brad,

Is this necessary? I don't see any pkg-fallout messages for databases/galera.

https://docs.freebsd.org/mail/current/freebsd-pkg-fallout.html
Comment 12 Brad Davis freebsd_committer freebsd_triage 2021-03-04 13:56:47 UTC
(In reply to Fernando Apesteguía from comment #11)

Antonie marked it broken on 13 & 14 two weeks ago:

https://svnweb.freebsd.org/ports/head/databases/galera/
Comment 13 Fernando Apesteguía freebsd_committer freebsd_triage 2021-03-04 14:54:31 UTC
(In reply to Brad Davis from comment #12)

That explains a lot :-)

With this patch, the port builds in -CURRENT. I removed:

BROKEN_FreeBSD_13
BROKEN_FreeBSD_14

Although the error seems the same, I can not test the other architectures that are BROKEN.

Would you like me to commit it like this?
Comment 14 Fernando Apesteguía freebsd_committer freebsd_triage 2021-03-05 13:29:06 UTC
Committed.
Comment 15 commit-hook freebsd_committer freebsd_triage 2021-03-05 13:29:39 UTC
A commit references this bug:

Author: fernape
Date: Fri Mar  5 13:28:55 UTC 2021
New revision: 567387
URL: https://svnweb.freebsd.org/changeset/ports/567387

Log:
  databases/galera: unbreak in 13 and 14

  Apply the same patch as in databases/galera26 in r566059.

  This probably unbreaks other architectures but I can't test them.

  PR:	253612
  Reported by:	brd@

Changes:
  head/databases/galera/Makefile
  head/databases/galera/files/patch-galerautils_src_gu__byteswap.h
Comment 16 Brad Davis freebsd_committer freebsd_triage 2021-03-05 14:36:35 UTC
Thank you!