> On Feb. 25, 2013, 9:48 p.m., Edward Hades Toroshchin wrote: > > Why do you want the intermediate libs at all? > > Matěj Laitl wrote: > 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) > > Matěj Laitl wrote: > 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 > > Alexander Neundorf wrote: > 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. > > > > Matěj Laitl wrote: > > 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
Not portable: --whole-archive works with gcc on Linux, it does not work everywhere, at least not with the Microsoft compiler and with the Sun compilers, don't know about others. You should at least test that those flags are available when using them. For propagating flags etc., please ask on the cmake mailing list, Stephen has put a lof ot work into propagating such things, but I am not sure about OBJECT libraries. In doubt, I would just not use those intermediate libraries. Beside that, whether you agreed on something in Randa or not, it doesn't make sense to agree on something which is not supported by the tool you are using. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109157/#review28081 ----------------------------------------------------------- On Feb. 25, 2013, 9:36 p.m., Matěj Laitl wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109157/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2013, 9:36 p.m.) > > > Review request for Amarok and Build System. > > > Description > ------- > > 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 <vonr...@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 > > > Diffs > ----- > > CMakeLists.txt d96cbc96de94fcc898b6d8af64c65634a28dd594 > cmake/modules/AmarokIntermediateLib.cmake PRE-CREATION > shared/CMakeLists.txt PRE-CREATION > shared/FileType.h 1b6facf66384bb70842c30e5a8ccd771c3009e1d > shared/FileType.cpp f02605845778b6d99b74155fdd8974082bb915fd > shared/FileTypeResolver.cpp f7c3eab8c9287f238c4eeec3a66def94aade3254 > shared/MetaReplayGain.h bf130cfe6f3a05b3852d6508447b3fc74e560e23 > shared/MetaTagLib.h cfa9910a7a2b0b24d8051c1c01866d239fdbe891 > shared/MetaTagLib.cpp 2e785e03b07193367d42491602f3e33cd862420c > shared/README 3d84f0deca111240087c97fc2290f1c6748a3b51 > shared/TagsFromFileNameGuesser.h 7f4443ef7c662970931a1fc06471c00bf053ab5d > shared/amarok_export.h cdab8033e35dcca817f275211b5a3bef4652a2c2 > shared/amarokshared_export.h PRE-CREATION > shared/collectionscanner/Album.h 91c3501ce615c170d440564a0e57fd2a30840e5b > shared/collectionscanner/Album.cpp 520452360e2b5ed4f1b52f6ff270721d08996bf3 > shared/collectionscanner/BatchFile.h > 61192770366e17e85637729b051a2f7a353cfc48 > shared/collectionscanner/CMakeLists.txt PRE-CREATION > shared/collectionscanner/Directory.h > 03f07ce3f189825bdc2eeff6e7c79fea2ed0ad8c > shared/collectionscanner/Directory.cpp > a6b02f86d8473645fd56958f3c5a6df01b65c2e1 > shared/collectionscanner/Playlist.h > 420bafc32bb80efe170c99b7e783163a9b4801b6 > shared/collectionscanner/ScanningState.h > dedf04998b4c0cd501fb22c8814754ed622c99dc > shared/collectionscanner/Track.h 96d243e898aa06319fa7347239098d6486d63adb > shared/tag_helpers/APETagHelper.h 2da873434b536af62cbbb546790dd90d1f733496 > shared/tag_helpers/ASFTagHelper.h 99818949e8781847980a88ae59c7f7dd47ee955f > shared/tag_helpers/ASFTagHelper.cpp > 966c29838106e722ff8606cc32d35c417ddf565f > shared/tag_helpers/CMakeLists.txt PRE-CREATION > shared/tag_helpers/ID3v2TagHelper.h > 5aec4ed82b7b6fc37d52b9809452ffdb3063af9c > shared/tag_helpers/ID3v2TagHelper.cpp > 7d50ca64888f856541cb6c56b3d6e720d17a4df8 > shared/tag_helpers/MP4TagHelper.h 8a5b1851939b8f20f450a8852db1d1d35e478b3c > shared/tag_helpers/MP4TagHelper.cpp > 37fcc1767a931f76a04fcd9fa72031594a0633e3 > shared/tag_helpers/StringHelper.h 6b31b684abf2ef5cb4eb3736f18b6f31a1729037 > shared/tag_helpers/TagHelper.h 898a76bb1d7e73afdf84a1d4e466cf314b53b807 > shared/tag_helpers/TagHelper.cpp 6909483a4df8d880143b5250991ad21daff129bb > shared/tag_helpers/VorbisCommentTagHelper.h > c5e87b3df344baecefb5d1d194f004a566d69804 > shared/tag_helpers/VorbisCommentTagHelper.cpp > 85689fb5db0ad26333281454b7826688a4ad0503 > src/CMakeLists.txt 6f453e2097c861e085c0c9c3c4c1534a45d477f8 > src/EngineController.h f4835bc97e6deb84c9c556e9859cfede5bf48f04 > src/amarok_export.h PRE-CREATION > src/browsers/playlistbrowser/UserPlaylistModel.h > 186837bc07df982d31a394333f5974b06669abd2 > src/core-impl/capabilities/AlbumActionsCapability.h > 789e6c7106d7db5dae6bf4a4baffd4c878a70ac0 > src/core-impl/capabilities/timecode/TimecodeBoundedPlaybackCapability.h > 05749d35ecc5d34f4567f17f3c5f2e905abdaff2 > src/core-impl/capabilities/timecode/TimecodeEditCapability.h > bfd50407acbfe1d5b1f02186032c927cd7b04b39 > src/core-impl/collections/db/sql/CMakeLists.txt > d6ba891c93327dbe5fa21eb0499ce83ef6523151 > src/core-impl/collections/db/sql/SqlMeta.cpp > 2032a2b03c40a66f91bbc6afb0cfc8f4244a410b > src/core-impl/collections/ipodcollection/CMakeLists.txt > 1dc5bce67c88d7207f76c3ca3590e66a876549b9 > src/core-impl/collections/playdarcollection/CMakeLists.txt > afa9300be9e6791d8e753bbf2cf0529408d639da > src/core-impl/collections/proxycollection/ProxyCollectionMeta.h > aeabcfe0aa4442ae7899c8b082a4ff52ec41e9c9 > src/core-impl/collections/support/jobs/WriteTagsJob.cpp > 21d5c1b170b8a1809276ca84bf69bded3c511795 > src/core-impl/collections/umscollection/CMakeLists.txt > ff3dd39116eefe0ea1cb6bffe9b4f14cfc6c22f8 > src/core-impl/collections/umscollection/UmsCollection.cpp > 08f40b379a155ff1bed67d11acd01c46c4687448 > src/core-impl/meta/file/File.h 10132c6cda8444a3653c304d8d366239691b20e5 > src/core-impl/meta/file/File_p.h 652787010099e39acbac5baf56c1b234fbc1cbb6 > src/core-impl/meta/proxy/MetaProxy.h > 7097a59a33938f8df8c926cea61ddc3fe22adc97 > src/core-impl/playlists/providers/user/UserPlaylistProvider.h > 48d020735775e0112fc6548882f66fca8ebce05d > src/core-impl/playlists/types/file/PlaylistFile.h > 4322da992261e4fac564a7bcd5bc2d5ae1fec989 > src/core-impl/support/PersistentStatisticsStore.h > 752c7044e7ef62b88dd1947b3cbbae6e868583eb > src/core/CMakeLists.txt 2809dc3db1525d87d78fcbc276c7b4aa0f60b203 > src/core/amarokcore_export.h PRE-CREATION > src/core/capabilities/ActionsCapability.h > 6cf53d93d091bc6cdf02da5a16b8e23a841a3d59 > src/core/capabilities/BookmarkThisCapability.h > b0072e09e2f2500f078e062f959107265c7497a1 > src/core/capabilities/Capability.h ba7dadccf72373954b1838ea6c10bb24e52d8da3 > src/core/capabilities/CollectionImportCapability.h > ed4fdc64d98d3ce1c4a3c590a886601aacb1ba89 > src/core/capabilities/CollectionScanCapability.h > 0b650c54e4e074797205db6ca44b334f00a8160d > src/core/capabilities/EditCapability.h > 9cc76be585d3ee2ce7ba0a10ca1cbc58b58113c2 > src/core/capabilities/FindInSourceCapability.h > f720dde4e6e556233003e2cd0a1058979fafe717 > src/core/capabilities/OrganiseCapability.h > 547e1b78ff9f2043eed2f6c6ec3d582ecc7d16da > src/core/capabilities/ReadLabelCapability.h > 5536d9967914c1e98589212448c85a87d11a6035 > src/core/capabilities/SourceInfoCapability.h > 0c58878b7807b486b5d57127bff2588b17892d7a > src/core/capabilities/StreamInfoCapability.h > f53640229f28ea3c76798ee9132ae9393d73a9e9 > src/core/capabilities/TranscodeCapability.h > be5f217d7cc19683e9fd25fb3081c1dbe276afa4 > src/core/capabilities/WriteLabelCapability.h > b23700dce6d881bafd2235ab858106cfed720061 > src/core/collections/Collection.h 48f9a08d3b008028e9c88b77c170522122a68671 > src/core/collections/CollectionLocation.h > 11a64d1be7272b38cbd6c816986c509cf75e0618 > src/core/collections/CollectionLocationDelegate.h > 6f39c19792ba2e245a5cba74bc8b1a809c21d80c > src/core/collections/QueryMaker.h a30b94fef31bcdea29e3bc8a60ed1fbee067879f > src/core/collections/support/SqlStorage.h > c2ce8adad6a8e9b15d5da30a4d4f7943757f2726 > src/core/collections/support/TrackForUrlWorker.h > c41917605fb41a6fbd6610a22875cd4e887a15fa > src/core/interfaces/Logger.h 346fbb623b2843c6424c491780a660feb017fca5 > src/core/interfaces/MetaCapability.h > 14885b0a81d1c3b4585795a778ac146fac72b746 > src/core/meta/Meta.h d0fea2064bb2107478350d2464265323166f9707 > src/core/meta/Statistics.h 3007592b604bcc5d6731639413d445095be2dd79 > src/core/meta/support/MetaConstants.h > 97881a04a5579203fb745c6c6d5b1c13c9126055 > src/core/meta/support/MetaKeys.h 3c97cc9a3da422059dac880a39b7cf9a58887e18 > src/core/meta/support/MetaUtility.h > 66dce9203b883f8b31d3f54d2170390863c51d53 > src/core/playlists/Playlist.h b4e4f404a2c07d6dc66e0cb1c4f8e31f12bd10a3 > src/core/playlists/PlaylistFormat.h > 21baac68ec359e4dd38a28af05cec8892f4603a9 > src/core/playlists/PlaylistProvider.h > 365792de089a9263fb80591ecffbe695cfc56bed > src/core/podcasts/PodcastMeta.h cacc9948709241b8b43df5c01e95f9756d37476b > src/core/support/Amarok.h 9c6f37ac78796f60c57e1f393ace47f38ce68d41 > src/core/support/Components.h d8cdd3ff84fecb92882b60d47fcfb115b7896b04 > src/core/support/Debug.h 085d033a3d5a7f06b7cd64b82519de8ed9d927fe > src/core/support/PluginFactory.h a16259aad84ed7da3fb396bc9fd6adf9b40c99cc > src/core/support/SmartPointerList.h > 37d0bab7a0d9c3afd65a10a6ba895cda07e752b5 > src/core/transcoding/TranscodingConfiguration.h > d0738fb4ac01b77db252b2cc13a82b4e2c8bb9e7 > src/core/transcoding/TranscodingController.h > 5e313e45cc303832bfdf34ef4517687ea4aab960 > src/core/transcoding/TranscodingFormat.h > ce9ed198d8ea20c5f5e194285eab6513a970b163 > src/core/transcoding/TranscodingProperty.h > 9cc10aa17a665ac9208baae0543936efca772be3 > src/covermanager/CoverFetchingActions.h > a3d2112477a091c02ce0269a80afaca1865c4af5 > src/covermanager/CoverViewDialog.h 41478911d52ecfca0a6624f8fea40dd1523cd024 > src/dbus/mpris1/PlayerHandler.h 8f7a77fb2fb41fef21afb895be29b3d42b27b3b3 > src/dialogs/TrackOrganizer.h 8c4ab84d2d9bf0c35c4e000169c10c61d39a2dee > src/playlist/PlaylistActions.h e8e541be8d60e87156f716ee6efd10ed948fd6cb > src/playlist/PlaylistController.h 368b89e6707582c50d8a40c38a6dd53d85ea2977 > src/playlist/PlaylistModel.h bf2052c74cf5401c5cc0fff8951662be0fabc591 > src/scriptengine/MetaTypeExporter.h > 334dc1e60bcbbe5301db47f2f0fbd57a83c05a79 > src/services/gpodder/CMakeLists.txt > f2cca2e78d65580deb6cf5f3ce26ac2d05089fd5 > tests/core-impl/collections/db/sql/CMakeLists.txt > bf387af80ae541d464ec1090e153e5b35ec8bc24 > tests/core-impl/collections/db/sql/TestSqlScanManager.cpp > 26b6943d816cc6045eafc6a83f1d00f7e309a99e > utilities/collectionscanner/CMakeLists.txt > f3d9f40dd97a793cd184384b962d45e87539c7a8 > utilities/collectionscanner/CollectionScanner.h > de9309ce7ead5553004996e320d73fbb5d92058a > utilities/collectionscanner/CollectionScanner.cpp > bac3e59ae6c460d08b51634d84570d36482c3d0c > > Diff: http://git.reviewboard.kde.org/r/109157/diff/ > > > Testing > ------- > > Builds, works, tests pass > > > Thanks, > > Matěj Laitl > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
