[prev in list] [next in list] [prev in thread] [next in thread]
List: amarok-devel
Subject: Re: Review Request 127523: Only link with MYSQL_EMBEDDED_LIBRARIES if WITH_MYSQL_EMBEDDED
From: Matt Whitlock <kde () mattwhitlock ! name>
Date: 2016-03-30 17:13:49
Message-ID: 20160330171349.12706.97563 () mimi ! kde ! org
[Download RAW message or body]
--===============8550701582527680018==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
> On March 29, 2016, 6:48 p.m., Myriam Schweingruber wrote:
> > hm, I fail to see why you would not build amarok with MySQL embedded, as that is \
> > the default database. Could you please specify why this patch is needed?
>
> Matt Whitlock wrote:
> Gentoo's philosophy is all about lean-and-mean, compiling support only for those \
> options that one actually uses. There is not "one size fits all," and all users \
> build all software from source, using the configure flags best suited to their \
> needs. The Gentoo ebuild for Amarok has a USE flag "embedded" that enables or \
> disables support for MySQL Embedded using `-DWITH_MYSQL_EMBEDDED` on the CMake \
> command line. I have been running an Amarok built without MySQL Embedded for many \
> years, as I run a full MySQL daemon on my system at all times anyway. The latest \
> Amarok beta no longer builds successfully when MySQL Embedded is disabled. This \
> patch is straightforward and logical and seems to be the obvious way to correct the \
> build problem. (Indeed, I am listening to music in Amarok 2.8.90 right now that has \
> been compiled without MySQL Embedded support by applying this patch to the build \
> system.)
> Myriam Schweingruber wrote:
> Well, then apply the patch to Gentoo, but why should we change what has always been \
> the default, namely MySQL embedded? We leave it to the users if they want to use an \
> external MySQL server, but that is not the default setup, default definitely is and \
> always has been the embedded version. The external MySQL setup is entirely up to \
> the users discretion, and not the default one. Understand me well: if distributions \
> think they want to change this, it's up to them, but it is not our default setup, \
> so I see no reason to change this in our source.
> Stefano Pettini wrote:
> Exactly because of this reason, the mechanism that allows users/distributions to \
> select whether to compile with/without the embedded MySQL should work. And the \
> patch fixes this mechanism, that aparently doesn't work. Nobody is trying to change \
> the default.
Gentoo's Amarok ebuild even defaults to USE="+embedded", which enables support for \
MySQL Embedded. But the point is that it's a user-configurable option (exposing the \
upstream build configuration option), and both possible values of the option ought to \
result in a successful build (or else the option ought to be removed). But I see no \
reason to remove the option when it's easy to fix it so that it works correctly.
- Matt
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127523/#review94120
-----------------------------------------------------------
On March 29, 2016, 12:27 p.m., Matt Whitlock wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127523/
> -----------------------------------------------------------
>
> (Updated March 29, 2016, 12:27 p.m.)
>
>
> Review request for Amarok.
>
>
> Repository: amarok
>
>
> Description
> -------
>
> This patch corrects a build failure when compiling Amarok without MySQL Embedded.
>
>
> Diffs
> -----
>
> src/core-impl/collections/db/sql/mysqlcollection/CMakeLists.txt 244cde1
>
> Diff: https://git.reviewboard.kde.org/r/127523/diff/
>
>
> Testing
> -------
>
> See [Gentoo bug 566980](https://bugs.gentoo.org/show_bug.cgi?id=566980).
>
>
> Thanks,
>
> Matt Whitlock
>
>
--===============8550701582527680018==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127523/">https://git.reviewboard.kde.org/r/127523/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On March 29th, 2016, 6:48 p.m. EDT, <b>Myriam \
Schweingruber</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px \
solid #d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">hm, I fail to see why you would not build amarok with \
MySQL embedded, as that is the default database. Could you please specify why this \
patch is needed?</p></pre> </blockquote>
<p>On March 29th, 2016, 11:05 p.m. EDT, <b>Matt Whitlock</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Gentoo's philosophy is all about lean-and-mean, compiling support only for \
those options that one actually uses. There is not "one size fits all," and all users \
build all software from source, using the configure flags best suited to their needs. \
The Gentoo ebuild for Amarok has a USE flag "embedded" that enables or disables \
support for MySQL Embedded using <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">-DWITH_MYSQL_EMBEDDED</code> on the CMake command line. I have been running \
an Amarok built without MySQL Embedded for many years, as I run a full MySQL daemon \
on my system at all times anyway. The latest Amarok beta no longer builds \
successfully when MySQL Embedded is disabled. This patch is straightforward and \
logical and seems to be the obvious way to correct the build problem. (Indeed, I am \
listening to music in Amarok 2.8.90 right now that has been compiled without MySQL \
Embedded support by applying this patch to the build system.)</p></pre> \
</blockquote>
<p>On March 30th, 2016, 8:29 a.m. EDT, <b>Myriam Schweingruber</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Well, \
then apply the patch to Gentoo, but why should we change what has always been the \
default, namely MySQL embedded? We leave it to the users if they want to use an \
external MySQL server, but that is not the default setup, default definitely is and \
always has been the embedded version. The external MySQL setup is entirely up to the \
users discretion, and not the default one. Understand me well: if distributions think \
they want to change this, it's up to them, but it is not our default setup, so I see \
no reason to change this in our source.</p></pre> </blockquote>
<p>On March 30th, 2016, 9:57 a.m. EDT, <b>Stefano Pettini</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Exactly because of this reason, the mechanism that allows \
users/distributions to select whether to compile with/without the embedded MySQL \
should work. And the patch fixes this mechanism, that aparently doesn't work. Nobody \
is trying to change the default.</p></pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Gentoo's Amarok ebuild even defaults to USE="+embedded", which enables \
support for MySQL Embedded. But the point is that it's a user-configurable option \
(exposing the upstream build configuration option), and both possible values of the \
option ought to result in a successful build (or else the option ought to be \
removed). But I see no reason to remove the option when it's easy to fix it so that \
it works correctly.</p></pre> <br />
<p>- Matt</p>
<br />
<p>On March 29th, 2016, 12:27 p.m. EDT, Matt Whitlock wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Amarok.</div>
<div>By Matt Whitlock.</div>
<p style="color: grey;"><i>Updated March 29, 2016, 12:27 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
amarok
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">This patch corrects a build failure when compiling \
Amarok without MySQL Embedded.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">See <a \
href="https://bugs.gentoo.org/show_bug.cgi?id=566980" style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Gentoo \
bug 566980</a>.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/core-impl/collections/db/sql/mysqlcollection/CMakeLists.txt <span \
style="color: grey">(244cde1)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127523/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============8550701582527680018==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic