<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/109157/">http://git.reviewboard.kde.org/r/109157/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 25th, 2013, 9:48 p.m. UTC, <b>Edward Hades Toroshchin</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;">Why do you want the intermediate libs at all?</pre>
</blockquote>
<p>On February 25th, 2013, 10:01 p.m. UTC, <b>Matěj Laitl</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;">In general, all points mentioned in http://www.cmake.org/Wiki/CMake/Tutorials/Object_Library#Motivation hold here, especially "This approach is easy to use and helps organize the project source tree.". (Note: approach used here doesn't use CMake "Object Libraries" at all)</pre>
</blockquote>
<p>On February 25th, 2013, 10:04 p.m. UTC, <b>Matěj Laitl</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;">Also, one CMakeLists.txt per directory was agreed upon in Randa 2012 by all present Amarok devs, see http://community.kde.org/Amarok/Development/Randa2012/Architecture</pre>
</blockquote>
<p>On February 26th, 2013, 5:31 p.m. UTC, <b>Alexander Neundorf</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;">If you want my opinion: if you really want those "convenience libraries", require at least cmake 2.8.9, and use cmake's OBJECT libraries, but really stay away from what you are doing in amarok_link_intermediate_libraries(). This is not portable at all and creates work the buildsystem should do (and can do if you use the features it provides for that).
Once you require cmake 2.8.9, you can also start using cmake's built-in automoc feature.
</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;">> If you want my opinion: if you really want those "convenience libraries", require at least cmake 2.8.9, and use cmake's OBJECT libraries, but really stay away from what you are doing in amarok_link_intermediate_libraries(). This is not portable at all and creates work the buildsystem should do (and can do if you use the features it provides for that).
I also considered using CMake object libraries, however, if I understand it correctly:
a) CMake won't add -fPIC for me for object libraries that end up in shared libraries. [1] In 2.8.9 there's POSITION_INDEPENDENT_CODE which is much more portable, but this is not specific to object libraries at all and I'll still have to set it manually.
b) CMake won't propagate -DBUILD_FOO_LIB from "parent" shared library to the "child" object library for me, I'll have to do it manually.
At this point I don't see what is more portable on using object libaries, as amarok_link_intermediate_libraries() does exactly a) and b) and I'll have to do it in some form too with object libraries.
[1] http://www.cmake.org/pipermail/cmake/2012-June/050941.html</pre>
<br />
<p>- Matěj</p>
<br />
<p>On February 25th, 2013, 9:36 p.m. UTC, Matěj Laitl wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok and Build System.</div>
<div>By Matěj Laitl.</div>
<p style="color: grey;"><i>Updated Feb. 25, 2013, 9:36 p.m.</i></p>
<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;">Hi fellow devs, this is a preview of how I'd like to clean up our (Amarok) buildsystem. The main idea is that every directory should has its own CMakeLists.txt, forming a small static library (build-time only), these small "intermediate" static libs get linked to bigger libraries like amaroklib.so etc. This preview does it on the shared/ directory which is turned into a separate shared library first. This review request are 3 commits squashed. Please speak up if you see some issues or something is not clear/too ugly for you.
Biggie: introduce libamarokshared.so library to avoid building things in shared/ twice
Shared libraries exist to *cough* share built code, use them! This is first part
of much larger buildsystem changes planned in Randa 2012.
This also brings some other changes that were needed, namely avoiding UTILITIES_BUILD,
changing Meta::Tag::writeTags() slightly etc.
@all git builders: you may need to remove build/CMakeCache.txt as some cached variables
may become invalid.
@Patrick: this needs at least build testing on Windows to ensure I've done the
KDE_IMPORT/EXPORT macros right.
CCMAIL: amarok-devel@kde.org
CCMAIL: Patrick von Reth <vonreth@kde.org>
Split AMAROK_EXPORT and AMAROK_CORE_EXPORT code and move to appropriate locations
...made possible by the previous commit.
buildsystem: start transition to per-directory CMakeLists.txt; introduce AmarokIntermediateLib.cmake
This is an example how transitioning to per-directory CMakeLists.txt
can be made. We introduce AmarokIntermediateLib.cmake file with
convenience macros amarok_add_intermediate_library and
amarok_link_intermediate_libraries.
We immediately use them to make shared/collectionscanner and
shared/collectionscanner free-standing with their own CMakeLists.txt</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;">Builds, works, tests pass</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>CMakeLists.txt <span style="color: grey">(d96cbc96de94fcc898b6d8af64c65634a28dd594)</span></li>
<li>cmake/modules/AmarokIntermediateLib.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>shared/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>shared/FileType.h <span style="color: grey">(1b6facf66384bb70842c30e5a8ccd771c3009e1d)</span></li>
<li>shared/FileType.cpp <span style="color: grey">(f02605845778b6d99b74155fdd8974082bb915fd)</span></li>
<li>shared/FileTypeResolver.cpp <span style="color: grey">(f7c3eab8c9287f238c4eeec3a66def94aade3254)</span></li>
<li>shared/MetaReplayGain.h <span style="color: grey">(bf130cfe6f3a05b3852d6508447b3fc74e560e23)</span></li>
<li>shared/MetaTagLib.h <span style="color: grey">(cfa9910a7a2b0b24d8051c1c01866d239fdbe891)</span></li>
<li>shared/MetaTagLib.cpp <span style="color: grey">(2e785e03b07193367d42491602f3e33cd862420c)</span></li>
<li>shared/README <span style="color: grey">(3d84f0deca111240087c97fc2290f1c6748a3b51)</span></li>
<li>shared/TagsFromFileNameGuesser.h <span style="color: grey">(7f4443ef7c662970931a1fc06471c00bf053ab5d)</span></li>
<li>shared/amarok_export.h <span style="color: grey">(cdab8033e35dcca817f275211b5a3bef4652a2c2)</span></li>
<li>shared/amarokshared_export.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>shared/collectionscanner/Album.h <span style="color: grey">(91c3501ce615c170d440564a0e57fd2a30840e5b)</span></li>
<li>shared/collectionscanner/Album.cpp <span style="color: grey">(520452360e2b5ed4f1b52f6ff270721d08996bf3)</span></li>
<li>shared/collectionscanner/BatchFile.h <span style="color: grey">(61192770366e17e85637729b051a2f7a353cfc48)</span></li>
<li>shared/collectionscanner/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>shared/collectionscanner/Directory.h <span style="color: grey">(03f07ce3f189825bdc2eeff6e7c79fea2ed0ad8c)</span></li>
<li>shared/collectionscanner/Directory.cpp <span style="color: grey">(a6b02f86d8473645fd56958f3c5a6df01b65c2e1)</span></li>
<li>shared/collectionscanner/Playlist.h <span style="color: grey">(420bafc32bb80efe170c99b7e783163a9b4801b6)</span></li>
<li>shared/collectionscanner/ScanningState.h <span style="color: grey">(dedf04998b4c0cd501fb22c8814754ed622c99dc)</span></li>
<li>shared/collectionscanner/Track.h <span style="color: grey">(96d243e898aa06319fa7347239098d6486d63adb)</span></li>
<li>shared/tag_helpers/APETagHelper.h <span style="color: grey">(2da873434b536af62cbbb546790dd90d1f733496)</span></li>
<li>shared/tag_helpers/ASFTagHelper.h <span style="color: grey">(99818949e8781847980a88ae59c7f7dd47ee955f)</span></li>
<li>shared/tag_helpers/ASFTagHelper.cpp <span style="color: grey">(966c29838106e722ff8606cc32d35c417ddf565f)</span></li>
<li>shared/tag_helpers/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>shared/tag_helpers/ID3v2TagHelper.h <span style="color: grey">(5aec4ed82b7b6fc37d52b9809452ffdb3063af9c)</span></li>
<li>shared/tag_helpers/ID3v2TagHelper.cpp <span style="color: grey">(7d50ca64888f856541cb6c56b3d6e720d17a4df8)</span></li>
<li>shared/tag_helpers/MP4TagHelper.h <span style="color: grey">(8a5b1851939b8f20f450a8852db1d1d35e478b3c)</span></li>
<li>shared/tag_helpers/MP4TagHelper.cpp <span style="color: grey">(37fcc1767a931f76a04fcd9fa72031594a0633e3)</span></li>
<li>shared/tag_helpers/StringHelper.h <span style="color: grey">(6b31b684abf2ef5cb4eb3736f18b6f31a1729037)</span></li>
<li>shared/tag_helpers/TagHelper.h <span style="color: grey">(898a76bb1d7e73afdf84a1d4e466cf314b53b807)</span></li>
<li>shared/tag_helpers/TagHelper.cpp <span style="color: grey">(6909483a4df8d880143b5250991ad21daff129bb)</span></li>
<li>shared/tag_helpers/VorbisCommentTagHelper.h <span style="color: grey">(c5e87b3df344baecefb5d1d194f004a566d69804)</span></li>
<li>shared/tag_helpers/VorbisCommentTagHelper.cpp <span style="color: grey">(85689fb5db0ad26333281454b7826688a4ad0503)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(6f453e2097c861e085c0c9c3c4c1534a45d477f8)</span></li>
<li>src/EngineController.h <span style="color: grey">(f4835bc97e6deb84c9c556e9859cfede5bf48f04)</span></li>
<li>src/amarok_export.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/browsers/playlistbrowser/UserPlaylistModel.h <span style="color: grey">(186837bc07df982d31a394333f5974b06669abd2)</span></li>
<li>src/core-impl/capabilities/AlbumActionsCapability.h <span style="color: grey">(789e6c7106d7db5dae6bf4a4baffd4c878a70ac0)</span></li>
<li>src/core-impl/capabilities/timecode/TimecodeBoundedPlaybackCapability.h <span style="color: grey">(05749d35ecc5d34f4567f17f3c5f2e905abdaff2)</span></li>
<li>src/core-impl/capabilities/timecode/TimecodeEditCapability.h <span style="color: grey">(bfd50407acbfe1d5b1f02186032c927cd7b04b39)</span></li>
<li>src/core-impl/collections/db/sql/CMakeLists.txt <span style="color: grey">(d6ba891c93327dbe5fa21eb0499ce83ef6523151)</span></li>
<li>src/core-impl/collections/db/sql/SqlMeta.cpp <span style="color: grey">(2032a2b03c40a66f91bbc6afb0cfc8f4244a410b)</span></li>
<li>src/core-impl/collections/ipodcollection/CMakeLists.txt <span style="color: grey">(1dc5bce67c88d7207f76c3ca3590e66a876549b9)</span></li>
<li>src/core-impl/collections/playdarcollection/CMakeLists.txt <span style="color: grey">(afa9300be9e6791d8e753bbf2cf0529408d639da)</span></li>
<li>src/core-impl/collections/proxycollection/ProxyCollectionMeta.h <span style="color: grey">(aeabcfe0aa4442ae7899c8b082a4ff52ec41e9c9)</span></li>
<li>src/core-impl/collections/support/jobs/WriteTagsJob.cpp <span style="color: grey">(21d5c1b170b8a1809276ca84bf69bded3c511795)</span></li>
<li>src/core-impl/collections/umscollection/CMakeLists.txt <span style="color: grey">(ff3dd39116eefe0ea1cb6bffe9b4f14cfc6c22f8)</span></li>
<li>src/core-impl/collections/umscollection/UmsCollection.cpp <span style="color: grey">(08f40b379a155ff1bed67d11acd01c46c4687448)</span></li>
<li>src/core-impl/meta/file/File.h <span style="color: grey">(10132c6cda8444a3653c304d8d366239691b20e5)</span></li>
<li>src/core-impl/meta/file/File_p.h <span style="color: grey">(652787010099e39acbac5baf56c1b234fbc1cbb6)</span></li>
<li>src/core-impl/meta/proxy/MetaProxy.h <span style="color: grey">(7097a59a33938f8df8c926cea61ddc3fe22adc97)</span></li>
<li>src/core-impl/playlists/providers/user/UserPlaylistProvider.h <span style="color: grey">(48d020735775e0112fc6548882f66fca8ebce05d)</span></li>
<li>src/core-impl/playlists/types/file/PlaylistFile.h <span style="color: grey">(4322da992261e4fac564a7bcd5bc2d5ae1fec989)</span></li>
<li>src/core-impl/support/PersistentStatisticsStore.h <span style="color: grey">(752c7044e7ef62b88dd1947b3cbbae6e868583eb)</span></li>
<li>src/core/CMakeLists.txt <span style="color: grey">(2809dc3db1525d87d78fcbc276c7b4aa0f60b203)</span></li>
<li>src/core/amarokcore_export.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core/capabilities/ActionsCapability.h <span style="color: grey">(6cf53d93d091bc6cdf02da5a16b8e23a841a3d59)</span></li>
<li>src/core/capabilities/BookmarkThisCapability.h <span style="color: grey">(b0072e09e2f2500f078e062f959107265c7497a1)</span></li>
<li>src/core/capabilities/Capability.h <span style="color: grey">(ba7dadccf72373954b1838ea6c10bb24e52d8da3)</span></li>
<li>src/core/capabilities/CollectionImportCapability.h <span style="color: grey">(ed4fdc64d98d3ce1c4a3c590a886601aacb1ba89)</span></li>
<li>src/core/capabilities/CollectionScanCapability.h <span style="color: grey">(0b650c54e4e074797205db6ca44b334f00a8160d)</span></li>
<li>src/core/capabilities/EditCapability.h <span style="color: grey">(9cc76be585d3ee2ce7ba0a10ca1cbc58b58113c2)</span></li>
<li>src/core/capabilities/FindInSourceCapability.h <span style="color: grey">(f720dde4e6e556233003e2cd0a1058979fafe717)</span></li>
<li>src/core/capabilities/OrganiseCapability.h <span style="color: grey">(547e1b78ff9f2043eed2f6c6ec3d582ecc7d16da)</span></li>
<li>src/core/capabilities/ReadLabelCapability.h <span style="color: grey">(5536d9967914c1e98589212448c85a87d11a6035)</span></li>
<li>src/core/capabilities/SourceInfoCapability.h <span style="color: grey">(0c58878b7807b486b5d57127bff2588b17892d7a)</span></li>
<li>src/core/capabilities/StreamInfoCapability.h <span style="color: grey">(f53640229f28ea3c76798ee9132ae9393d73a9e9)</span></li>
<li>src/core/capabilities/TranscodeCapability.h <span style="color: grey">(be5f217d7cc19683e9fd25fb3081c1dbe276afa4)</span></li>
<li>src/core/capabilities/WriteLabelCapability.h <span style="color: grey">(b23700dce6d881bafd2235ab858106cfed720061)</span></li>
<li>src/core/collections/Collection.h <span style="color: grey">(48f9a08d3b008028e9c88b77c170522122a68671)</span></li>
<li>src/core/collections/CollectionLocation.h <span style="color: grey">(11a64d1be7272b38cbd6c816986c509cf75e0618)</span></li>
<li>src/core/collections/CollectionLocationDelegate.h <span style="color: grey">(6f39c19792ba2e245a5cba74bc8b1a809c21d80c)</span></li>
<li>src/core/collections/QueryMaker.h <span style="color: grey">(a30b94fef31bcdea29e3bc8a60ed1fbee067879f)</span></li>
<li>src/core/collections/support/SqlStorage.h <span style="color: grey">(c2ce8adad6a8e9b15d5da30a4d4f7943757f2726)</span></li>
<li>src/core/collections/support/TrackForUrlWorker.h <span style="color: grey">(c41917605fb41a6fbd6610a22875cd4e887a15fa)</span></li>
<li>src/core/interfaces/Logger.h <span style="color: grey">(346fbb623b2843c6424c491780a660feb017fca5)</span></li>
<li>src/core/interfaces/MetaCapability.h <span style="color: grey">(14885b0a81d1c3b4585795a778ac146fac72b746)</span></li>
<li>src/core/meta/Meta.h <span style="color: grey">(d0fea2064bb2107478350d2464265323166f9707)</span></li>
<li>src/core/meta/Statistics.h <span style="color: grey">(3007592b604bcc5d6731639413d445095be2dd79)</span></li>
<li>src/core/meta/support/MetaConstants.h <span style="color: grey">(97881a04a5579203fb745c6c6d5b1c13c9126055)</span></li>
<li>src/core/meta/support/MetaKeys.h <span style="color: grey">(3c97cc9a3da422059dac880a39b7cf9a58887e18)</span></li>
<li>src/core/meta/support/MetaUtility.h <span style="color: grey">(66dce9203b883f8b31d3f54d2170390863c51d53)</span></li>
<li>src/core/playlists/Playlist.h <span style="color: grey">(b4e4f404a2c07d6dc66e0cb1c4f8e31f12bd10a3)</span></li>
<li>src/core/playlists/PlaylistFormat.h <span style="color: grey">(21baac68ec359e4dd38a28af05cec8892f4603a9)</span></li>
<li>src/core/playlists/PlaylistProvider.h <span style="color: grey">(365792de089a9263fb80591ecffbe695cfc56bed)</span></li>
<li>src/core/podcasts/PodcastMeta.h <span style="color: grey">(cacc9948709241b8b43df5c01e95f9756d37476b)</span></li>
<li>src/core/support/Amarok.h <span style="color: grey">(9c6f37ac78796f60c57e1f393ace47f38ce68d41)</span></li>
<li>src/core/support/Components.h <span style="color: grey">(d8cdd3ff84fecb92882b60d47fcfb115b7896b04)</span></li>
<li>src/core/support/Debug.h <span style="color: grey">(085d033a3d5a7f06b7cd64b82519de8ed9d927fe)</span></li>
<li>src/core/support/PluginFactory.h <span style="color: grey">(a16259aad84ed7da3fb396bc9fd6adf9b40c99cc)</span></li>
<li>src/core/support/SmartPointerList.h <span style="color: grey">(37d0bab7a0d9c3afd65a10a6ba895cda07e752b5)</span></li>
<li>src/core/transcoding/TranscodingConfiguration.h <span style="color: grey">(d0738fb4ac01b77db252b2cc13a82b4e2c8bb9e7)</span></li>
<li>src/core/transcoding/TranscodingController.h <span style="color: grey">(5e313e45cc303832bfdf34ef4517687ea4aab960)</span></li>
<li>src/core/transcoding/TranscodingFormat.h <span style="color: grey">(ce9ed198d8ea20c5f5e194285eab6513a970b163)</span></li>
<li>src/core/transcoding/TranscodingProperty.h <span style="color: grey">(9cc10aa17a665ac9208baae0543936efca772be3)</span></li>
<li>src/covermanager/CoverFetchingActions.h <span style="color: grey">(a3d2112477a091c02ce0269a80afaca1865c4af5)</span></li>
<li>src/covermanager/CoverViewDialog.h <span style="color: grey">(41478911d52ecfca0a6624f8fea40dd1523cd024)</span></li>
<li>src/dbus/mpris1/PlayerHandler.h <span style="color: grey">(8f7a77fb2fb41fef21afb895be29b3d42b27b3b3)</span></li>
<li>src/dialogs/TrackOrganizer.h <span style="color: grey">(8c4ab84d2d9bf0c35c4e000169c10c61d39a2dee)</span></li>
<li>src/playlist/PlaylistActions.h <span style="color: grey">(e8e541be8d60e87156f716ee6efd10ed948fd6cb)</span></li>
<li>src/playlist/PlaylistController.h <span style="color: grey">(368b89e6707582c50d8a40c38a6dd53d85ea2977)</span></li>
<li>src/playlist/PlaylistModel.h <span style="color: grey">(bf2052c74cf5401c5cc0fff8951662be0fabc591)</span></li>
<li>src/scriptengine/MetaTypeExporter.h <span style="color: grey">(334dc1e60bcbbe5301db47f2f0fbd57a83c05a79)</span></li>
<li>src/services/gpodder/CMakeLists.txt <span style="color: grey">(f2cca2e78d65580deb6cf5f3ce26ac2d05089fd5)</span></li>
<li>tests/core-impl/collections/db/sql/CMakeLists.txt <span style="color: grey">(bf387af80ae541d464ec1090e153e5b35ec8bc24)</span></li>
<li>tests/core-impl/collections/db/sql/TestSqlScanManager.cpp <span style="color: grey">(26b6943d816cc6045eafc6a83f1d00f7e309a99e)</span></li>
<li>utilities/collectionscanner/CMakeLists.txt <span style="color: grey">(f3d9f40dd97a793cd184384b962d45e87539c7a8)</span></li>
<li>utilities/collectionscanner/CollectionScanner.h <span style="color: grey">(de9309ce7ead5553004996e320d73fbb5d92058a)</span></li>
<li>utilities/collectionscanner/CollectionScanner.cpp <span style="color: grey">(bac3e59ae6c460d08b51634d84570d36482c3d0c)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109157/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>