All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sylvain Raybaud <sylvain.raybaud@green-communications.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 7/7 v2] mysql: add mariadb galera cluster variant
Date: Fri, 21 Aug 2015 10:23:29 +0200	[thread overview]
Message-ID: <55D6E001.6010608@green-communications.fr> (raw)
In-Reply-To: <CAHXCMMKV79jKA9YcA_f6KxWuivKMjP8_zZKjHjAHDU6hgiiXCg@mail.gmail.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Samuel

Thanks for your review. Here are my replies to some points (also inlined).

On 09/07/2015 23:56, Samuel Martin wrote:
> Hi Sylvain,
> 
> Here is a partial review (inlined).
> 
> But, overall, I feel like this implementation (essentially the
> *.mk file) does not match the Buildroot way of doing things... :-/ 
> Have you look at the virtual package infrastructure [1]?

Considering your remark and the discussion that took place last week,
I repackaged the whole thing in the same way as jpeg.

> 
> On Thu, Jul 9, 2015 at 6:22 PM, Sylvain Raybaud

>> diff --git
>> a/package/mysql/10.0.17/mariadb-galera-01-fix_xtradb_cmakelist.patch
>> b/package/mysql/10.0.17/mariadb-galera-01-fix_xtradb_cmakelist.patch
>>
>> 
new file mode 100644
>> index 0000000..83eac77 --- /dev/null +++
>> b/package/mysql/10.0.17/mariadb-galera-01-fix_xtradb_cmakelist.patch
>>
>> 
@@ -0,0 +1,27 @@
>> +Do not perform test if the build is not requested anyway. The
>> test may fail, +preventing building the whole package. +
> The comment does not really reflect what is done in the patch: -
> according to the comment: this change touch tests; - AFAICS, this
> change disable a plugin.

No, I just enclose some commands in:

IF(NOT WITHOUT_XTRADB)
[..]
ENDIF()

Meaning: "if building XtraDB plugin was not requested, do not care
about the result of the test". This patch does not disable XtraDB.
This is done via an option passed in mariadb-galera.mk

> [..]

>> ++IF(NOT WITHOUT_XTRADB)
> Does this boolean come from an option? it could.
> 

It does. There is the following line in mariadb-galera.mk:

MARIADB_GALERA_EXTRA_OPTS += "-DWITHOUT_XTRADB=1"


[..]

>> diff --git
>> a/package/mysql/10.0.17/mariadb-galera-02-fix_innodb_cmakelist.patch
>> b/package/mysql/10.0.17/mariadb-galera-02-fix_innodb_cmakelist.patch
>>
>> 
new file mode 100644
>> index 0000000..1ddec5d --- /dev/null +++
>> b/package/mysql/10.0.17/mariadb-galera-02-fix_innodb_cmakelist.patch
>>
>> 
@@ -0,0 +1,251 @@
>> +Use CHECK_C_SOURCE_COMPILES instead of CHECK_C_SOURCE_RUNS in
>> order to be +cross-compile friendly. +In buildroot another
>> solution (maybe better, maybe not) could be to use 
>> +BR2_ARCH_HAS_ATOMICS in order to determine the value of
>> HAVE_IB_GCC_ATOMIC_BUILTINS_BYTE +and likes. +
> Missing SoB line [2].
> 
> It makes sense to submit this patch upstream as well.

Right. Does this mean we should wait for them to review this patch
before considering including it in buildroot?

> [..]
>> +config MYSQL_MARIADB_GALERA +       bool "mariadb-galera" +
>> select BR2_PACKAGE_NCURSES +       select BR2_PACKAGE_OPENSSL +
>> select BR2_PACKAGE_ZLIB +       select BR2_PACKAGE_LIBAIO +
>> select BR2_PACKAGE_LIBTOOL +       select BR2_PACKAGE_GALERA +
>> select BR2_PACKAGE_BASH # runtime dependency only +       select
>> BR2_PACKAGE_LSOF # runtime dependency only +       select
>> BR2_PACKAGE_PROCPS_NG # runtime dependency only +       select
>> BR2_PACKAGE_FINDUTILS # runtime dependency only +       select
>> BR2_PACKAGE_BUSYBOX_SHOW_OTHERS +       depends on
>> BR2_TOOLCHAIN_BUILDROOT_GLIBC || BR2_PACKAGE_EGLIBC # spawn()
> Missing "depends on" for the selected packages [3].
> 
>> +       help +         MariaDB Galera Cluster. +
>> https://mariadb.com/kb/en/mariadb/what-is-mariadb-galera-cluster 
>> + +endchoice + +endif + +comment "MySQL needs a toolchain w/ C++,
>> threads" +depends on !BR2_INSTALL_LIBSTDCPP ||
>> !BR2_TOOLCHAIN_HAS_THREADS
> Indent is wrong. Missing: depends on BR2_USE_MMU Move this comment
> in the choice.

I don't understand how to properly move this in the choice section.
However I modified Config.in when I made mysql a virtual package. Does
this remark still apply?

> 
>> + +comment "MariaDB needs an (e)glibc toolchain" +depends on
>> !BR2_TOOLCHAIN_BUILDROOT_GLIBC && !BR2_PACKAGE_EGLIBC # spawn()
> Indent is wrong. Move this comment in the choice.

Same question about moving the comment.

Thanks again!

Cheers,

- -- 
Sylvain Raybaud
www.green-communications.fr
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJV1uABAAoJEEkkwl4JtJ9yNCIP/ic8RsfhCdG56I3fH55KFDqi
ABnVSsYB3HP9vczjCfFzLQq3YbbTX1HMhB72L83OdyBoBmoUmEedXO9cOCcZw75q
Fer4OBgGhrl11uy3KvwH2E/nfGgVtjdiGSW0o+7mB041E/ix2E+SnlKwIQu9mTFS
4UZq7m7UpeWBQeMwRE3VpNnNCimpW+pcIXIjzMdbDMNfH5xAy6Da3OtJPay4vRBk
99PZ4Ly6wOcytRnTvuDOz2u8OMbb6GOshSTFyqDucenculWPada+fVa0QKWJlDMo
wFpLPv5HlcVKXcFZO8T+USK3GFnWgdjeJget1r8vXUUDzh/rEFD6VDCaYIC86dQN
pWHpqosPCUcd/d4q6hI1rutCO4w9djKmDDC8AFB2iPnwK7EuPV8AnWXE0RLMA+Rf
+CKoNG9D/yny3po37SouxajkIkg/3FOK/3cPVlTCEnMUiQtHh7Alcd6ll7CvTMEk
0gNA8Sia1vQ6B72O7ASJOfMCLQsOIiqXsODZhI9pFv2K5xGvap6E7LBShSuADE8o
qb7ig6wsTJnvdOBlDTyva7KPVQ4V/4nBQGT7njvsAtcwe46dNbYhIT2wRv7i7btW
5tBBy9HDZ8ex6AcM7pDaraU5gu6edEowRSTdZWLoM7SOBgNT8/USeRofVHBd4tgv
NMtaizcTJNxxjmiQSpwB
=UmBa
-----END PGP SIGNATURE-----

  parent reply	other threads:[~2015-08-21  8:23 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-09 16:21 [Buildroot] [PATCH 0/7 v2] Set of patches to add MariaDB galera cluster Sylvain Raybaud
2015-07-09 16:21 ` [Buildroot] [PATCH 1/7 v2] check: new package Sylvain Raybaud
2015-07-10 23:06   ` Yann E. MORIN
2015-07-09 16:21 ` [Buildroot] [PATCH 2/7 v2] libaio: add host variant Sylvain Raybaud
2015-07-10 22:30   ` Yann E. MORIN
2015-07-10 23:01     ` Yann E. MORIN
2015-07-10 23:00   ` Thomas Petazzoni
2015-07-09 16:21 ` [Buildroot] [PATCH 3/7 v2] galera: new package Sylvain Raybaud
2015-07-09 21:29   ` Samuel Martin
2015-07-09 21:53     ` Arnout Vandecappelle
2015-08-21 13:20       ` Sylvain Raybaud
2015-08-21 13:39     ` Sylvain Raybaud
2015-07-09 16:21 ` [Buildroot] [PATCH 4/7 v2] pkg-cmake: add PKG_CONFIG_* variables to help cmake find host packages Sylvain Raybaud
2015-07-10 22:47   ` Samuel Martin
2015-07-09 16:21 ` [Buildroot] [PATCH 5/7 v2] busybox: adjust configuration to add fancy options to the sleep applet Sylvain Raybaud
2015-07-10 22:48   ` Samuel Martin
2015-07-10 22:58   ` Thomas Petazzoni
2015-07-10 23:06     ` Sylvain Raybaud
2015-07-09 16:22 ` [Buildroot] [PATCH 6/7 v2] mysql: move patches into a version-specific subdirectory Sylvain Raybaud
2015-07-09 16:22 ` [Buildroot] [PATCH 7/7 v2] mysql: add mariadb galera cluster variant Sylvain Raybaud
2015-07-09 21:56   ` Samuel Martin
2015-07-10  7:54     ` Thomas Petazzoni
2015-08-07 13:44       ` Sylvain Raybaud
2015-08-08  8:43         ` Thomas Petazzoni
2015-08-08 23:22           ` Yann E. MORIN
2015-08-09  8:46             ` Thomas Petazzoni
2015-08-09 12:59               ` Yann E. MORIN
2015-08-22 22:21             ` Arnout Vandecappelle
2015-08-24 10:14               ` Sylvain Raybaud
2015-08-20 12:05           ` Sylvain Raybaud
2015-08-20 12:32             ` Thomas Petazzoni
2015-08-21  8:23     ` Sylvain Raybaud [this message]
2015-08-26 21:45     ` Sylvain Raybaud
2015-10-08 15:15     ` Sylvain Raybaud

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55D6E001.6010608@green-communications.fr \
    --to=sylvain.raybaud@green-communications.fr \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.