D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

[Andreas Sturmlechner]

asturmlechner marked an inline comment as done.

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28284

To: asturmlechner, #amarok, heikobecker, wbauer
Cc: amarok-devel, vmatare, pgomes, nicolamori, jyasi, mfraser, ryanmccoskrie, 
semareit, eridani, eoinoneill, jackyalcine, nhuisman, paul, domson, yaohanchen, 
malteveerman, markey, James, tbettler, Smar, ricktimmis, asturmlechner, 
schweingruber, heikobecker

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

[Andreas Sturmlechner]

asturmlechner updated this revision to Diff 78497.
asturmlechner added a comment.


  re-add accidentally removed required includes to FindMySQLe.cmake

REPOSITORY
  R181 Amarok

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28284?vs=78487&id=78497

BRANCH
  mysqle-split-for-phab

REVISION DETAIL
  https://phabricator.kde.org/D28284

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindMariaDB.cmake
  cmake/modules/FindMySQL.cmake
  cmake/modules/FindMySQLe.cmake
  src/core-impl/collections/db/sql/mysqlcollection/CMakeLists.txt
  src/core-impl/storage/sql/CMakeLists.txt
  src/core-impl/storage/sql/mysqlestorage/CMakeLists.txt
  src/core-impl/storage/sql/mysqlserverstorage/CMakeLists.txt
  tests/core-impl/collections/db/sql/CMakeLists.txt
  tests/dynamic/CMakeLists.txt
  tests/scanner/CMakeLists.txt

To: asturmlechner, #amarok, heikobecker, wbauer
Cc: amarok-devel, vmatare, pgomes, nicolamori, jyasi, mfraser, ryanmccoskrie, 
semareit, eridani, eoinoneill, jackyalcine, nhuisman, paul, domson, yaohanchen, 
malteveerman, markey, James, tbettler, Smar, ricktimmis, asturmlechner, 
schweingruber, heikobecker

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

[Wolfgang Bauer]

wbauer added a comment.


  In D28284#634644 , @wbauer wrote:
  
  > On openSUSE, it doesn't enable MySQLe anymore:
  >
  >-- Found MySQL: -L/usr/lib64 -lmariadb  
  >   /usr/bin/mysql_config: unrecognized option '--libmysqld-libs'
  >   -- Performing Test HAVE_MYSQL_OPT_EMBEDDED_CONNECTION
  >   -- Performing Test HAVE_MYSQL_OPT_EMBEDDED_CONNECTION - Failed
  >   -- Could NOT find MySQLe (missing: HAVE_MYSQL_OPT_EMBEDDED_CONNECTION) 
  >
  >
  > See inline comment for the reason.
  
  
  Erm, ignore that please.
  This was with the first revision, I didn't notice that the patch has been 
updated meanwhile.
  The new version seems to be fine, but more tomorrow.

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28284

To: asturmlechner, #amarok, heikobecker, wbauer
Cc: amarok-devel, vmatare, pgomes, nicolamori, jyasi, mfraser, ryanmccoskrie, 
semareit, eridani, eoinoneill, jackyalcine, nhuisman, paul, domson, yaohanchen, 
malteveerman, markey, James, tbettler, Smar, ricktimmis, asturmlechner, 
schweingruber, heikobecker

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

[Wolfgang Bauer]

wbauer added a comment.


  On openSUSE, it doesn't enable MySQLe anymore:
  
 -- Found MySQL: -L/usr/lib64 -lmariadb  
/usr/bin/mysql_config: unrecognized option '--libmysqld-libs'
-- Performing Test HAVE_MYSQL_OPT_EMBEDDED_CONNECTION
-- Performing Test HAVE_MYSQL_OPT_EMBEDDED_CONNECTION - Failed
-- Could NOT find MySQLe (missing: HAVE_MYSQL_OPT_EMBEDDED_CONNECTION) 
  
  See inline comment for the reason.

INLINE COMMENTS

> FindMySQLe.cmake:82
> +cmake_push_check_state()
> +set(CMAKE_REQUIRED_LIBRARIES ${MYSQLE_LIBRARIES})
> +check_cxx_source_compiles( "#include \nint main() { int i = 
> MYSQL_OPT_USE_EMBEDDED_CONNECTION; }" HAVE_MYSQL_OPT_EMBEDDED_CONNECTION )

`set(CMAKE_REQUIRED_INCLUDES ${MYSQL_INCLUDE_DIR})` is missing here (it was 
present in the old FindMySQL.cmake).
Adding it makes the test succeed again here.

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28284

To: asturmlechner, #amarok, heikobecker, wbauer
Cc: amarok-devel, vmatare, pgomes, nicolamori, jyasi, mfraser, ryanmccoskrie, 
semareit, eridani, eoinoneill, jackyalcine, nhuisman, paul, domson, yaohanchen, 
malteveerman, markey, James, tbettler, Smar, ricktimmis, asturmlechner, 
schweingruber, heikobecker

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

[Andreas Sturmlechner]

asturmlechner edited the summary of this revision.

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28284

To: asturmlechner, #amarok, heikobecker, wbauer
Cc: amarok-devel, vmatare, jfowkes, pgomes, nicolamori, jyasi, mfraser, 
ryanmccoskrie, semareit, eridani, eoinoneill, jackyalcine, nhuisman, paul, 
domson, yaohanchen, malteveerman, markey, James, tbettler, Smar, ricktimmis, 
asturmlechner, schweingruber, heikobecker

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

[Andreas Sturmlechner]

asturmlechner updated this revision to Diff 78487.
asturmlechner added a comment.


  updated for the case when mysqle is shipped with mariadb

REPOSITORY
  R181 Amarok

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28284?vs=78465&id=78487

BRANCH
  mysqle-split-for-phab

REVISION DETAIL
  https://phabricator.kde.org/D28284

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindMariaDB.cmake
  cmake/modules/FindMySQL.cmake
  cmake/modules/FindMySQLe.cmake
  src/core-impl/collections/db/sql/mysqlcollection/CMakeLists.txt
  src/core-impl/storage/sql/CMakeLists.txt
  src/core-impl/storage/sql/mysqlestorage/CMakeLists.txt
  src/core-impl/storage/sql/mysqlserverstorage/CMakeLists.txt
  tests/core-impl/collections/db/sql/CMakeLists.txt
  tests/dynamic/CMakeLists.txt
  tests/scanner/CMakeLists.txt

To: asturmlechner, #amarok, heikobecker, wbauer
Cc: amarok-devel, vmatare, jfowkes, pgomes, nicolamori, jyasi, mfraser, 
ryanmccoskrie, semareit, eridani, eoinoneill, jackyalcine, nhuisman, paul, 
domson, yaohanchen, malteveerman, markey, James, tbettler, Smar, ricktimmis, 
asturmlechner, schweingruber, heikobecker

D28165: Fix compatibility with modern MySQL

[Wolfgang Bauer]

wbauer added a comment.


  In D28165#634060 , @asturmlechner 
wrote:
  
  > In D28165#633145 , @wbauer wrote:
  >
  > > The content of my /usr/include/mysql/mysql_version.h (which is just a 
symlink to mariadb_version.h):
  >
  >
  > I just went back and installed mariadb-connector-c in Gentoo in 
mysqlcompat-mode (a soon to be removed option) and interestingly, it does not 
provide that symlink.
  
  
  Indeed, the symlink is added in openSUSE's packaging. From 
mariadb-connector-c.spec:
  
# add a compatibility symlink
ln -s mariadb_config %{buildroot}%{_bindir}/mysql_config
ln -s mariadb_version.h %{buildroot}%{_includedir}/mysql/mysql_version.h

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28165

To: vmatare, asturmlechner, heikobecker
Cc: heikobecker, jfowkes, asturmlechner, wbauer, amarok-devel, pgomes, 
nicolamori, jyasi, mfraser, ryanmccoskrie, semareit, eridani, eoinoneill, 
jackyalcine, nhuisman, paul, domson, yaohanchen, malteveerman, markey, James, 
tbettler, Smar, ricktimmis, schweingruber

D28165: Fix compatibility with modern MySQL

[Victor Mataré]

vmatare added a comment.


  > I assume you don't have commit access and you'll need somebody to push this 
patch for you, right?
  
  Yes, please go ahead if you think this has been tested well enough.

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28165

To: vmatare, asturmlechner, heikobecker
Cc: heikobecker, jfowkes, asturmlechner, wbauer, amarok-devel, pgomes, 
nicolamori, jyasi, mfraser, ryanmccoskrie, semareit, eridani, eoinoneill, 
jackyalcine, nhuisman, paul, domson, yaohanchen, malteveerman, markey, James, 
tbettler, Smar, ricktimmis, schweingruber

D28165: Fix compatibility with modern MySQL

[Wolfgang Bauer]

wbauer added a comment.


  In D28165#634404 , @asturmlechner 
wrote:
  
  > Waiting for @wbauer to confirm it works as well
  
  
  Yes, the latest version builds fine here (the previous did as well), and the 
embedded collection still works as well.
  
  > btw, does openSUSE still provide mysqle to test -DWITH_MYSQL_EMBEDDED=on?
  
  Yes, as part of mariadb. (the "real" mysql is not included in the 
distribution anymore since a while)

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28165

To: vmatare, asturmlechner, heikobecker
Cc: heikobecker, jfowkes, asturmlechner, wbauer, amarok-devel, pgomes, 
nicolamori, jyasi, mfraser, ryanmccoskrie, semareit, eridani, eoinoneill, 
jackyalcine, nhuisman, paul, domson, yaohanchen, malteveerman, markey, James, 
tbettler, Smar, ricktimmis, schweingruber

D24817: Restore scripting functionalities and Qt bindings.

[Andreas Sturmlechner]

Herald added a project: Amarok.
Herald removed a subscriber: Amarok.

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D24817

To: nicolamori, amarok-devel, heikobecker
Cc: asturmlechner, malteveerman, heikobecker, amarok-devel, jfowkes, pgomes, 
nicolamori, jyasi, mfraser, ryanmccoskrie, semareit, eridani, eoinoneill, 
jackyalcine, nhuisman, paul, domson, yaohanchen, markey, James, tbettler, Smar, 
ricktimmis, schweingruber, #amarok

D27949: Conclude porting of Last.fm to Amarok KF5

[Andreas Sturmlechner]

asturmlechner added a comment.


  scrobble it does 👍

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D27949

To: pgomes, amarok-devel, #amarok
Cc: heikobecker, asturmlechner, amarok-devel, #amarok, jfowkes, pgomes, 
nicolamori, jyasi, mfraser, ryanmccoskrie, semareit, eridani, eoinoneill, 
jackyalcine, nhuisman, paul, domson, yaohanchen, malteveerman, markey, James, 
tbettler, Smar, ricktimmis, schweingruber

D27949: Conclude porting of Last.fm to Amarok KF5

[Heiko Becker]

heikobecker added inline comments.

INLINE COMMENTS

> LastFmServiceConfig.h:37
> +#endif
> +
>  /**

Any reason not to use a generated export header for that?

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D27949

To: pgomes, amarok-devel, #amarok
Cc: heikobecker, asturmlechner, amarok-devel, #amarok, jfowkes, pgomes, 
nicolamori, jyasi, mfraser, ryanmccoskrie, semareit, eridani, eoinoneill, 
jackyalcine, nhuisman, paul, domson, yaohanchen, malteveerman, markey, James, 
tbettler, Smar, ricktimmis, schweingruber

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

[Andreas Sturmlechner]

asturmlechner created this revision.
asturmlechner added reviewers: Amarok, heikobecker, wbauer.
Herald added a project: Amarok.
Herald added a subscriber: amarok-devel.
asturmlechner requested review of this revision.

REVISION SUMMARY
  - Previously, MariaDB lib was only found w/ distro provided compat links
  - In fact both MySQL and MariaDB client libs can be installed side by side
  
  FindMySQL.cmake: Split out MySQL Embedded detection into FindMySQLe.cmake
  
  - MySQLe is pretty much dead now
  - If MySQLe is not found, we will not attempt to link MYSQLE_LIBRARIES anymore
  - Move ZLIB detection behind MySQLe conditional
  - WITH_MYSQL_EMBEDDED is now obsolete
  - Drop superfluous set_package_properties if defined in Find*.cmake module
  
  Drop superfluous ZLIB linking
  
  - Amarok builds fine without, according to CMakeLists.txt only required
  
  for MySQLe anyway.

TEST PLAN
  Built fine against mysql-connector-c as well as mariadb-connector-c.
  Unfortunately, MySQLe is long gone in Gentoo so I rely on sb. else's test.

REPOSITORY
  R181 Amarok

BRANCH
  mysqle-split-for-phab

REVISION DETAIL
  https://phabricator.kde.org/D28284

AFFECTED FILES
  CMakeLists.txt
  cmake/modules/FindMariaDB.cmake
  cmake/modules/FindMySQL.cmake
  cmake/modules/FindMySQLe.cmake
  src/core-impl/collections/db/sql/mysqlcollection/CMakeLists.txt
  src/core-impl/storage/sql/CMakeLists.txt
  src/core-impl/storage/sql/mysqlestorage/CMakeLists.txt
  src/core-impl/storage/sql/mysqlserverstorage/CMakeLists.txt
  tests/core-impl/collections/db/sql/CMakeLists.txt
  tests/dynamic/CMakeLists.txt
  tests/scanner/CMakeLists.txt

To: asturmlechner, #amarok, heikobecker, wbauer
Cc: amarok-devel, vmatare, jfowkes, pgomes, nicolamori, jyasi, mfraser, 
ryanmccoskrie, semareit, eridani, eoinoneill, jackyalcine, nhuisman, paul, 
domson, yaohanchen, malteveerman, markey, James, tbettler, Smar, ricktimmis, 
asturmlechner, schweingruber, heikobecker

D28165: Fix compatibility with modern MySQL

[Heiko Becker]

heikobecker accepted this revision.
heikobecker added a comment.


  Builds fine here with mariadb and WITH_MYSQL_EMBEDDED=TRUE.
  
  I assume you don't have commit access and you'll need somebody to push this 
patch for you, right?

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28165

To: vmatare, asturmlechner, heikobecker
Cc: heikobecker, jfowkes, asturmlechner, wbauer, amarok-devel, pgomes, 
nicolamori, jyasi, mfraser, ryanmccoskrie, semareit, eridani, eoinoneill, 
jackyalcine, nhuisman, paul, domson, yaohanchen, malteveerman, markey, James, 
tbettler, Smar, ricktimmis, schweingruber

D28165: Fix compatibility with modern MySQL

[Andreas Sturmlechner]

asturmlechner added a comment.


  Waiting for @wbauer to confirm it works as well - btw, does openSUSE still 
provide mysqle to test -DWITH_MYSQL_EMBEDDED=on?

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28165

To: vmatare, asturmlechner
Cc: jfowkes, asturmlechner, wbauer, amarok-devel, pgomes, nicolamori, jyasi, 
mfraser, ryanmccoskrie, semareit, eridani, eoinoneill, jackyalcine, nhuisman, 
paul, domson, yaohanchen, malteveerman, markey, James, tbettler, Smar, 
ricktimmis, schweingruber, heikobecker

D28165: Fix compatibility with modern MySQL

[Andreas Sturmlechner]

asturmlechner accepted this revision.
asturmlechner added a comment.
This revision is now accepted and ready to land.


  In D28165#634067 , @vmatare wrote:
  
  > Please test again, and thanks a lot for the review so far.
  
  
  Successfully tested with:
  mariadb-connector-c-3.0.6 (mysqlcompat mode)
  mysql-connector-c-8.0.19
  mariadb-connector-c-3.1.7 (with my local modifications)

REPOSITORY
  R181 Amarok

REVISION DETAIL
  https://phabricator.kde.org/D28165

To: vmatare, asturmlechner
Cc: jfowkes, asturmlechner, wbauer, amarok-devel, pgomes, nicolamori, jyasi, 
mfraser, ryanmccoskrie, semareit, eridani, eoinoneill, jackyalcine, nhuisman, 
paul, domson, yaohanchen, malteveerman, markey, James, tbettler, Smar, 
ricktimmis, schweingruber, heikobecker