Bug 354255

Summary: [PATCH]: Handle removed --myisam-recover option in MySQL 5.7
Product: amarok Reporter: Terje Rosten <terje.rosten>
Component: Collections/LocalAssignee: Amarok Developers <amarok-bugs-dist>
Status: RESOLVED FIXED    
Severity: normal CC: matej, ralf-engels, rdieter, robert, stefano
Priority: NOR    
Version: unspecified   
Target Milestone: 2.9   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 2.9
Attachments: Patch to fix issue with removed option in MySQL 5.7

Description Terje Rosten 2015-10-23 13:18:58 UTC
MySQL option --myisam-recover  has been deprecated since MySQL 5.5.3, new name for option is --myisam-recover-options, forks are also using this name.

In MySQL 5.7 (just released as GA) --myisam-recover  has been removed.

To make amarok work with MySQL 5.7 option name --myisam-recover-options should be used.

See also related bug: https://bugs.kde.org/show_bug.cgi?id=323802




Reproducible: Always

Steps to Reproduce:
1. Build amarok against MySQL 5.7.
2. Run amarok.

Actual Results:  
Crash in MySQL init

Expected Results:  
No crash

Patch to fix issue attached.
Comment 1 Terje Rosten 2015-10-23 13:22:11 UTC
Created attachment 95092 [details]
Patch to fix issue with removed option in MySQL 5.7
Comment 2 Myriam Schweingruber 2015-10-25 14:48:54 UTC
Thank you for your patch, which exact Amarok version did you build?
Please submit your patch to http://reviewboard.kde.org specifying if it is based on master or a specific release tag

FWIW: we do not require mysql 2.7 to build Amarok currently (README states MySQL 5.0 or newer), why should we patch the current master, as we are already porting to Qt5 and will update the dependencies once the port is done? Amarok 2.9 will be the last release in Qt4, and it is due in a few weeks.

FWIW2: wasn't this fixed already in master by this commit: http://commits.kde.org/amarok/00dd8b41143ce08d1f529b439a943fd9bd052341 ?
Comment 3 Stefano Pettini 2016-06-12 06:25:24 UTC
Hi All,

I spend 2h debugging mysql_library_init returning 1 and nothing else. It was due to the usage of deprecated --myisam-recover, instead of --myisam-recover-options.

The commit of 2013 linked above tried to fix a problem occurred in the past by using the deprecated --myisam-recover instead of the proper one --myisam-recover-options. I assume there was a good reason for this choice, but now recent MySQL versions don't accept the deprecated option anymore so that fix should be reverted.

Terje's fix should be applied instead, possibly before releasing 2.9, as it's the safest fix: it leaves the behavior unchanged for pre-5.7 MySQL versions, and switch to the proper option for 5.7 and newer.

I'll post his patch on reviewboard.
Comment 4 Myriam Schweingruber 2016-06-12 10:00:11 UTC
(In reply to Stefano Pettini from comment #3)
> Hi All,
> 
> I spend 2h debugging mysql_library_init returning 1 and nothing else. It was
> due to the usage of deprecated --myisam-recover, instead of
> --myisam-recover-options.
> 
> The commit of 2013 linked above tried to fix a problem occurred in the past
> by using the deprecated --myisam-recover instead of the proper one
> --myisam-recover-options. I assume there was a good reason for this choice,
> but now recent MySQL versions don't accept the deprecated option anymore so
> that fix should be reverted.
> 
> Terje's fix should be applied instead, possibly before releasing 2.9, as
> it's the safest fix: it leaves the behavior unchanged for pre-5.7 MySQL
> versions, and switch to the proper option for 5.7 and newer.
> 
> I'll post his patch on reviewboard.

There has been another commit since, is this fixed now or is there still something pending?
Comment 5 Stefano Pettini 2016-06-12 11:44:06 UTC
Which commit? If you're talking about this:

> FWIW2: wasn't this fixed already in master by this commit: http://commits.kde.org/amarok/
> 00dd8b41143ce08d1f529b439a943fd9bd052341 ?

this is the commit that introduced the problem. The bug is with the latest 2.8-git and recent versions of MySQL 100% reproducible, and will be fixed when Terje's patch is merged.

See my comment:

The commit of 2013 linked above tried to fix a problem occurred in the past by using the deprecated --myisam-recover instead of the proper one --myisam-recover-options. I assume there was a good reason for this choice, but now recent MySQL versions don't accept the deprecated option anymore so that fix should be reverted.

His patch is now on https://git.reviewboard.kde.org/r/128157/
Comment 6 robert marshall 2016-06-16 18:33:26 UTC
I can also confirm this problem with kubuntu 16.04 KDE Version: 4.14.16 Qt Version: 4.8.7 and that the patch solves the problem
Comment 7 Matěj Laitl 2016-08-07 09:23:58 UTC
Git commit a07f44ddcd61bac1bb2f1e17cfc07d40a940bb5a by Matěj Laitl, on behalf of Terje Rosten.
Committed on 07/08/2016 at 09:05.
Pushed by laitl into branch 'master'.

Substitute deprecated MySQL option --myisam-recover

Deprecated MySQL option --myisam-recover has been removed in MySQL 5.7,
replacement is --myisam-recover-options.

Use MYSQL_VERSION_ID to handle this.

Signed-off-by: Terje Rosten <terje.rosten@oracle.com>
Signed-off-by: Stefano Pettini <stefano.pettini@gmail.com>

CCMAIL: Terje Rosten <terje.rosten@oracle.com>
REVIEW: 128157
FIXED-IN: 2.9

M  +2    -0    ChangeLog
M  +4    -0    src/core-impl/storage/sql/mysqlestorage/MySqlEmbeddedStorage.cpp

http://commits.kde.org/amarok/a07f44ddcd61bac1bb2f1e17cfc07d40a940bb5a