Re: [amarok] cmake/modules: Revert FindMySQLAmarok.cmake fix argumets when calling mysql_config
On Sunday 18 December 2011 23:27:08 =?utf-8?B?TWF0xJtq?= Laitl wrote: On 18. 12. 2011 Christophe Giboudeaux wrote: Revert FindMySQLAmarok.cmake fix argumets when calling mysql_config This reverts a fix for -Wmissing-include-dirs warnings. MYSQL_INCLUDE_DIR must be a path and not contain -Ipath Sorry about the incorrect fix, I will build with -Wmissing-include-dirs from now on. However I wonder how to fix the original problem. Your commit doesn't mention the issue you're trying to fix. However, having the same directory included several times is not an issue (and I'm not aware of any GCC flag that would throw a warning). Would it be legal to remove MYSQL_INCLUDE_DIR entirely and just rely on MYSQL_CFLAGS to specify -I/path/to/mysql ? Well, they don't have the same purpose. MYSQL_CFLAGS is used by the compiler (used in 'add_definitions..' lines) while MYSQL_INCLUDE_DIR is used by cmake. Also, if mysql_config is not available, MYSQL_CFLAGS is empty and you can only count on MYSQL_INCLUDE_DIR. E.g. is cmake's include_directories(/dir) just a syntactic sugar for add_definitions(-I/dir) or it would break build on non-gcc platforms? I'd rather avoid filtering -I out of MYSQL_INCLUDE_DIR in CMakeLists. Without hint about what you're trying to fix, it's hard to answer this question. Christophe signature.asc Description: This is a digitally signed message part. ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Should we support building utilities without building player?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 19-12-2011 06:32, Paweł Hajdan, Jr. wrote: On 12/19/11 12:15 AM, Matěj Laitl wrote: I can do the b), but is there an actual use-case for building only utilities? Would linking to kdelibs hamper this use-case? (Paweł, Thomas?) The use case is that arch testers/developers test with various USE flag combinations, and we're pretty good at detecting the breaking ones. I tested with USE=-player only because it was possible. I'm totally fine with removing that USE flag. The use flag was added to support a feature that upstream amarok developers decided to provide to users: being able to use amarok on a non-dekstop box to do the tagging and scanning of the collection. If upstream wants to drop the support for that, we'll just update the use flags. This is not the first time that this support is broken: check commit b0bef274be1a9741c54c8d32e13d17600c79d300 for a revert done by Jeff Mitchell to allow building utilities without building the player. ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel - -- Regards, Jorge Vicetto (jmbsvicetto) - jmbsvicetto at gentoo dot org Gentoo- forums / Userrel / Devrel / KDE / Elections / RelEng -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.18 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQIcBAEBAgAGBQJO7x9JAAoJEC8ZTXQF1qEPXdEP/2+mkAi4hbRdYL/jw59J+s0S ew1HGmYm8WAHQ8YOr5D5bHEVYy9JKqS74Tb6SGiOJ/6DHY4rA1yJ4RKQBqbP2AHE zbH6XIunSrwZxVBLNmRvfHse37Ck0Uc8opfZ0DCEBuQSfX+A2TWKjDq055W+cekv 50wJAY3jt+yhBhke5qwzIUS4tUVJuIuFWvN+ERBe9iDpxSmNRzHE6ZVnotduZ6fM i0u8UsmyeLMxHnH+mqGOMNr9r1Lt3jYXaIFKEkdDEs1WkUj++pt4IM9ewsZKdpBb sKWV92V4kvz7TSZlBqxEsEQu/XPzV6qjgkUlcwKDHfJGW5ap2/4OTYL9+uXEXKlW ZVKpGs1R/T/M2lllhhxJQd4ZSzp10hq6ZYPMar+TUUMOyU31C/314xDoQpfjF6SV vjJYbC7+74sDPaMZCLxio07Dpe+9NgBcf6/pwB7/yq+8G62fR31dCLZn92/nymh1 HS8PxLiBZCNTxoWO0I2YO6ZyE+hm9Y1Az+IHA5eL03w9MYGYI3gDTaJreY/WFs0O sLABIiUI6lDhhCBNDcFrOtBH4PZsa9s+/rp6F6Z1mIqH15J+clt+Fgghp1TDQoEG iRsohFuGi4+++k1G2HiL3NdzOiSxdaCqKi/pdq1ebsSm7LSUliZ83GEBZKRhv6Ab O004fK/M4qMwoV86gYeT =v8Dm -END PGP SIGNATURE- ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: Fix for USB storage mode media players
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100194/#review9081 --- src/core-impl/collections/ipodcollection/support/IpodConnectionAssistant.cpp http://git.reviewboard.kde.org/r/100194/#comment7510 This would break identification of iPods. KDE's solid with udisks udev backend currently doesn't attach PMP interface to them. - Matěj Laitl On Dec. 8, 2010, 9:55 p.m., Lukáš Tinkl wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100194/ --- (Updated Dec. 8, 2010, 9:55 p.m.) Review request for Amarok. Description --- This patch fixes identifying general USB storage mode media players, plus adds some minor fixes and cleanups. The main change is in UmsConnectionAssistant::identify method. Diffs - src/MediaDeviceCache.cpp babb8ff src/core-impl/collections/ipodcollection/support/IpodConnectionAssistant.cpp 92339ff src/core-impl/collections/umscollection/support/UmsConnectionAssistant.cpp 5956a2b Diff: http://git.reviewboard.kde.org/r/100194/diff/diff Testing --- Screenshots --- Amarok in USB mode http://git.reviewboard.kde.org/r/100194/s/21/ Thanks, Lukáš Tinkl ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] cmake/modules: Revert FindMySQLAmarok.cmake fix argumets when calling mysql_config
On 19. 12. 2011 Christophe Giboudeaux wrote: I see two solutions: - only rely on find_path(MYSQL_INCLUDE_DIR...), so move it outside the else(MYSQLCONFIG_EXECUTABLE) loop [1] - Drop FindMysqlAmarok.cmake and use FindAmarok.cmake from kdelibs instead (which worked fine here). I prefer the second solution as the FindMysql from kdelibs is really cleaner than the Amarok copy: - MYSQL_CFLAGS doesn't look that important - the linked targets gathered with 'mysql_config --libs' are already explicitly added in the CMakeLists.txt files (./src/core- impl/collections/db/sql/mysqlecollection/CMakeLists.txt, ./src/core- impl/collections/db/sql/mysqlservercollection/CMakeLists.txt and some unit tests) and the output variable is different if mysql_config is not present: * with mysql_config, MYSQL_LIBRARIES=-L/usr/lib64 -lmysqlclient -lpthread - lz -lm -lrt -lssl -lcrypto -ldl * without it, MYSQL_LIBRARIES=/usr/lib64/libmysqlclient.so. The second one is enough - the kdelibs FindAmarok has better chances to detect mysql.h under Windows, - and it also uses a cleaner way to decide if mysqle is there. I'd also like to drop FindMysqlAmarok.cmake entirely and use kdelibs one, code duplication is never good. Let's do it early in the 2.6 phase to get the most testing. Anyone opposed to me dropping FindMysqlAmarok.cmake in master for now? (can always be reverted if problems arise) Matěj Laitl ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
[Bugzilla] Release blockers
The following bugs are blocking the next Amarok release: This search was scheduled by bcooks...@kde.org. Bug 233170: http://bugs.kde.org/show_bug.cgi?id=233170 Priority: HI Severity: crash Platform: Unlisted Binaries Assignee: amarok-bugs-d...@kde.org Status: REOPENED Summary: Crash when browsing a category on Jamendo [@ CollectionTreeItem::row] ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] cmake/modules: Revert FindMySQLAmarok.cmake fix argumets when calling mysql_config
On Monday 19 December 2011 00:38:47 =?utf-8?B?TWF0xJtq?= Laitl wrote:
On 19. 12. 2011 Christophe Giboudeaux wrote:
On Sunday 18 December 2011 23:27:08 Matej Laitl wrote:
Sorry about the incorrect fix, I will build with -Wmissing-include-dirs
from now on. However I wonder how to fix the original problem.
Your commit doesn't mention the issue you're trying to fix. However,
having
the same directory included several times is not an issue (and I'm not
aware of any GCC flag that would throw a warning).
Ah, I tried to solve http://mail.kde.org/pipermail/amarok-devel/2011-
December/009663.html
Specifically, the problem is that mysql_config (since at least 5.1.56)
doesn't accept --variable=pkgincludedir - it accepts only --include
argument which returns something like -I/usr/include/mysql
-- Found MySQL: Usage: /usr/bin/mysql_config [OPTIONS]
[cut]
So I guess we'll have to strip that -I out of `mysql_config --include`. Can
we safely assume that it is only a single directory?
ok, then it only exists in higher version (the option exists in 5.5.x).
I see two solutions:
- only rely on find_path(MYSQL_INCLUDE_DIR...), so move it outside the
else(MYSQLCONFIG_EXECUTABLE) loop [1]
- Drop FindMysqlAmarok.cmake and use FindAmarok.cmake from kdelibs instead
(which worked fine here).
I prefer the second solution as the FindMysql from kdelibs is really cleaner
than the Amarok copy:
- MYSQL_CFLAGS doesn't look that important
- the linked targets gathered with 'mysql_config --libs' are already
explicitly added in the CMakeLists.txt files (./src/core-
impl/collections/db/sql/mysqlecollection/CMakeLists.txt, ./src/core-
impl/collections/db/sql/mysqlservercollection/CMakeLists.txt and some unit
tests) and the output variable is different if mysql_config is not present:
* with mysql_config, MYSQL_LIBRARIES=-L/usr/lib64 -lmysqlclient -lpthread -
lz -lm -lrt -lssl -lcrypto -ldl
* without it, MYSQL_LIBRARIES=/usr/lib64/libmysqlclient.so. The second one
is enough
- the kdelibs FindAmarok has better chances to detect mysql.h under Windows,
- and it also uses a cleaner way to decide if mysqle is there.
The first solution is the easy one, the second one needs more testing on
different platforms/distributions.
Christophe
[1] attached: fixMySQLAmarok.diff
diff --git a/cmake/modules/FindMySQLAmarok.cmake b/cmake/modules/FindMySQLAmarok.cmake
index 74805ca..44a3fb5 100644
--- a/cmake/modules/FindMySQLAmarok.cmake
+++ b/cmake/modules/FindMySQLAmarok.cmake
@@ -16,8 +16,18 @@ if(NOT WIN32)
find_program(MYSQLCONFIG_EXECUTABLE NAMES mysql_config mysql_config5 PATHS ${BIN_INSTALL_DIR} ~/usr/bin /usr/local/bin)
endif(NOT WIN32)
+find_path(MYSQL_INCLUDE_DIR mysql.h
+/opt/local/include/mysql5/mysql
+/opt/mysql/mysql/include
+/opt/mysqle/include/mysql
+/opt/ports/include/mysql5/mysql
+/usr/include/mysql
+/usr/local/include/mysql
+/usr/mysql/include/mysql
+~/usr/include/mysql
+)
+
if(MYSQLCONFIG_EXECUTABLE)
-exec_program(${MYSQLCONFIG_EXECUTABLE} ARGS --variable=pkgincludedir RETURN_VALUE _return_VALUE OUTPUT_VARIABLE MYSQL_INCLUDE_DIR)
exec_program(${MYSQLCONFIG_EXECUTABLE} ARGS --cflags RETURN_VALUE _return_VALUE OUTPUT_VARIABLE MYSQL_CFLAGS)
exec_program(${MYSQLCONFIG_EXECUTABLE} ARGS --libs RETURN_VALUE _return_VALUE OUTPUT_VARIABLE MYSQL_LIBRARIES)
exec_program(${MYSQLCONFIG_EXECUTABLE} ARGS --libmysqld-libs RETURN_VALUE _return_VALUE OUTPUT_VARIABLE MYSQL_EMBEDDED_LIBSTEMP)
@@ -49,18 +59,6 @@ if(MYSQLCONFIG_EXECUTABLE)
else(MYSQLCONFIG_EXECUTABLE)
-find_path(MYSQL_INCLUDE_DIR mysql.h
- ~/usr/include/mysql
- /opt/local/include/mysql5/mysql
- /opt/mysqle/include/mysql
- /opt/mysql/mysql/include
- /usr/mysql/include/mysql
- /usr/include/mysql
- /usr/local/include/mysql
- /opt/local/include/mysql
- /opt/ports/include/mysql5/mysql
-)
-
if(WIN32)
set(MYSQL_CLIENT_LIBRARY_NAME libmysql)
else(WIN32)
signature.asc
Description: This is a digitally signed message part.
___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
