Merge ~sergiodj/ubuntu/+source/mysql-8.0:fix-ftbfs-riscv into ubuntu/+source/mysql-8.0:ubuntu/devel

[Sergio Durigan Junior]

This is a fix for the current FTBFS that is happening on RISC-V:

https://launchpadlibrarian.net/489348932/buildlog_ubuntu-groovy-riscv64.mysql-8.0_8.0.21-0ubuntu2_BUILDING.txt.gz

The problem happens because, on RISC-V, the always-lock-free property cannot be guaranteed for certain types, like boolean. MySQL started requiring this property since:

https://github.com/mysql/mysql-server/commit/a36bd8b297d654f88a5fd58e8dc8dc5c1538851d

And now the compilation fails on RISC-V. Fortunately, in that same commit the author implemented a failsafe mechanism: an encapsulation for the "largest always-lock-free type in the architecture", which can be used to mitigate this exact scenario. Unfortunately, in order to make the RISC-V compilation happy again, I had to resort to some ifdef'ery to make sure that the original always-lock-free mechanism is not included when compiling on the architecture. I've tried to keep the patch as clean as possible, and provide comments on it explaining the reasoning behind these decisions.

I've tested everything locally using a RISC-V virtual machine, and the compilation suceeded (after almost 24 hours compiling!). I've also tested on amd64, and it worked OK. I didn't set up a PPA for this, but I can if needed.

[Marc Deslauriers (mdeslaur)]

LGTM, thanks!

review: Approve

[Robie Basak (racb)]

Looks good, thanks! Great job pinning down the issue and figuring out the fix - I appreciate it was complex task and RISC-V + MySQL tries ones patience with the emulation!

Thank you for the detailed explanations of what is going on - that made it much easier to review and gives me confidence in the maintainability of this patch. Can I suggest though that you move the content of the MP description into the dep3 header description? There's a lot of useful information there (more than the existing dep3 header) but the MP will be difficult to find after the patch lands.

We should talk about:

1) Contacting upstream to see if they will take this (I'm not sure that they will - we may just have to continue carrying this).

2) Where to land this branch - I would prefer to keep the branch in Salsa updated, but I appreciate that it's probably behind Ubuntu again because of uploads by other Ubuntu developers without going through the VCS. We should figure out how we want to handle this on an ongoing basis on our team.

review: Approve

[Robie Basak (racb)]

PS. I was additionally going to ask for a security team ack because of the different-to-normal maintenance burden for MySQL patches not being upstream, but Marc has already approved so I assume that's OK.

[Sergio Durigan Junior (sergiodj)]

On Tuesday, July 28 2020, Robie Basak wrote:

> Thank you for the detailed explanations of what is going on - that
> made it much easier to review and gives me confidence in the
> maintainability of this patch. Can I suggest though that you move the
> content of the MP description into the dep3 header description?
> There's a lot of useful information there (more than the existing dep3
> header) but the MP will be difficult to find after the patch lands.

Thanks for the review, Robie!

I looks like Marc already uploaded the package yesterday, so I believe
this MP is now "water under the bridge", right? I can totally propose a
new MP to do what you suggested above, and perhaps set the distribution
to UNRELEASED, what do you think?

> We should talk about:
>
> 1) Contacting upstream to see if they will take this (I'm not sure that they will - we may just have to continue carrying this).

Yeah, I thought about that. I doubt upstream will accept the patch
as-is, but perhaps it's a good idea to raise the topic with them so that
they're at least aware of it. I'll open a PR and explain the situation.

> 2) Where to land this branch - I would prefer to keep the branch in
> Salsa updated, but I appreciate that it's probably behind Ubuntu again
> because of uploads by other Ubuntu developers without going through
> the VCS. We should figure out how we want to handle this on an ongoing
> basis on our team.

I don't know if I completely understand what you mean here, so I'll ask
you during our standup.

Thanks,

--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14

[Robie Basak (racb)]

On Tue, Jul 28, 2020 at 02:27:22PM -0000, Sergio Durigan Junior wrote:
> I looks like Marc already uploaded the package yesterday, so I believe
> this MP is now "water under the bridge", right? I can totally propose a
> new MP to do what you suggested above, and perhaps set the distribution
> to UNRELEASED, what do you think?

Ah - don't worry about it then. We can mark this MP as Merged.

[Sergio Durigan Junior (sergiodj)]

This has migrated.

diff --git a/debian/changelog b/debian/changelog
index 1d23196..9f3f298 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+mysql-8.0 (8.0.21-0ubuntu3) groovy; urgency=medium
+
+  * Fix FTBFS on RISC-V.
+    - d/p/use-largest-lock-free-type-selector-on-riscv.patch: Force
+      the use of Largest_lock_free_type_selector instead of
+      Lock_free_type_selector when compiling for RISC-V, since the
+      latter will cause a compilation failure due to RISC-V's
+      inability to provide the always-lock-free property for some
+      specific types.
+
+ -- Sergio Durigan Junior <[e-mail]>  Mon, 27 Jul 2020 09:00:46 -0400
+
 mysql-8.0 (8.0.21-0ubuntu2) groovy; urgency=medium
 
   * debian/tests/upstream: disable some tests that have expired
diff --git a/debian/patches/series b/debian/patches/series
index 72909ba..3538593 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -3,3 +3,4 @@ mysql-test-run-paths
 fix_grant_user_lock_as_root.patch
 atomic-test-words.patch
 charset_file_crash.patch
+use-largest-lock-free-type-selector-on-riscv.patch
diff --git a/debian/patches/use-largest-lock-free-type-selector-on-riscv.patch b/debian/patches/use-largest-lock-free-type-selector-on-riscv.patch
new file mode 100644
index 0000000..dadc0fc
--- /dev/null
+++ b/debian/patches/use-largest-lock-free-type-selector-on-riscv.patch
@@ -0,0 +1,65 @@
+From: Sergio Durigan Junior <[e-mail]>
+Date: Mon, 27 Jul 2020 08:54:58 -0400
+Subject: Use Largest_lock_free_type_selector on RISC-V
+
+This patch is necessary because RISC-V doesn't guarantee the
+always-lock-free property on certain types (like boolean), which
+causes a static_assert to trigger when compiling MySQL on the
+architecture.
+
+Author: Sergio Durigan Junior <[e-mail]>
+Forwarded: no
+Last-Updated: 2020-07-27
+---
+ .../temptable/include/temptable/lock_free_type.h   | 24 +++++++++++++++++++++-
+ 1 file changed, 23 insertions(+), 1 deletion(-)
+
+diff --git a/storage/temptable/include/temptable/lock_free_type.h b/storage/temptable/include/temptable/lock_free_type.h
+index 691137f..99bbeba 100644
+--- a/storage/temptable/include/temptable/lock_free_type.h
++++ b/storage/temptable/include/temptable/lock_free_type.h
+@@ -65,6 +65,19 @@ enum class Alignment { NATURAL, L1_DCACHE_SIZE };
+  * More details and motivation can be found at:
+  *   http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0152r0.html
+  * */
++
++/** Ubuntu patch for RISC-V:
++
++    On RISC-V, some types are not always lock-free.  For example,
++    ATOMIC_BOOL_LOCK_FREE is 1, meaning that it is sometimes
++    lock-free.  This causes a compilation error of
++    Lock_free_type_selector because of the static_asserts below.  For
++    this reason, we have to guard the Lock_free_type_selector code
++    with an ifndef when compiling for RISC-V.  We also have to force
++    the use of Largest_lock_free_type_selector instead of
++    Lock_free_type_selector.  */
++
++#ifndef __riscv
+ template <typename T, typename V = void>
+ struct Lock_free_type_selector {
+   static_assert(
+@@ -187,6 +200,13 @@ struct Lock_free_type_selector<
+                 "always-lock-free property. Bailing out ...");
+ #endif
+ };
++#else
++  /** As explained above, if we're compiling for RISC-V then we have
++      to force the use of Largest_lock_free_type_selector instead of
++      Lock_free_type_selector, because the former will work
++      normally, while the latter will fail to compile.  */
++#define Lock_free_type_selector Largest_lock_free_type_selector
++#endif /* ! __riscv */
+ 
+ /** Largest lock-free type selector, a helper utility very much similar
+  * to Lock_free_type_selector with the difference being that it tries hard
+@@ -253,7 +273,9 @@ struct Largest_lock_free_type_selector<
+  *
+  * Always-lock-free guarantee is implemented through the means of
+  * Lock_free_type_selector or Largest_lock_free_type_selector. User code can
+- * opt-in for any of those. By default, Lock_free_type_selector is used.
++ * opt-in for any of those. By default, Lock_free_type_selector is
++ * used, except on RISC-V, where Largest_lock_free_type_selector is
++ * used by default due to atomic type limitations.
+  *
+  * In addition, this type provides an ability to redefine the
+  * alignment-requirement of the underlying always-lock-free type, basically