> On Aug. 15, 2014, 8:20 a.m., Alex Merry wrote: > > modules/ECMGeneratePkgConfigFile.cmake, line 47 > > <https://git.reviewboard.kde.org/r/119798/diff/1/?file=305638#file305638line47> > > > > This belongs in KDEInstallDirs.cmake, not here (as > > CMAKE_INSTALL_PKGCONFIGDIR, ideally). Projects that don't use > > KDEInstallDirs can create their own variable. > > > > Also, pkconfig -> pkgconfig. > > Aleix Pol Gonzalez wrote: > I'm unsure about that, first ECM_MKSPECS_INSTALL_DIR is declared the same > way (again, copy&paste) then I understand that we can override the variable > from KDEInstallDirs, but we do want that the module that uses the variable > actually declares it, right? > > Alex Merry wrote: > Yeah, I don't like that about the QMake module. The thing is, this module > *doesn't* use the variable - it sets it, and then expects the caller to use > it. > > Aleix Pol Gonzalez wrote: > Maybe what feels wrong is the fact that we need to explicitly install it. > When we use such a macro, we want to generate the file and get it done.
I'm generally not in favour of magic, especially magic where you actually have to do something apparently unrelated (in this case, define CMAKE_INSTALL_LIBDIR or equivalent, possibly by including KDEInstallDirs) for it to actually work. I'd rather have an explicit argument - that, at least, is greppable. Note that getting the correct value of CMAKE_INSTALL_LIBDIR/LIB_INSTALL_DIR is *hard* - both the GNUInstallDirs and KDEInstallDirs have a reasonably large chunk of logic to figure it out. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119798/#review64585 ----------------------------------------------------------- On Aug. 19, 2014, 11:34 a.m., Aleix Pol Gonzalez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119798/ > ----------------------------------------------------------- > > (Updated Aug. 19, 2014, 11:34 a.m.) > > > Review request for Build System, KDE Frameworks and Harald Sitter. > > > Repository: extra-cmake-modules > > > Description > ------- > > So we decided we wanted those .pc files, so I created a small script that > generates one, I haven't used pc in the past, so feedback is welcome. > > > Diffs > ----- > > modules/ECMGeneratePkgConfigFile.cmake PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/119798/diff/ > > > Testing > ------- > > I added it in KCoreAddons, this is the patch: > diff --git src/lib/CMakeLists.txt src/lib/CMakeLists.txt > index 26eb5a1..3a07d1c 100644 > --- src/lib/CMakeLists.txt > +++ src/lib/CMakeLists.txt > @@ -188,4 +188,6 @@ install(FILES > > include(ECMGeneratePriFile) > ecm_generate_pri_file(BASE_NAME KCoreAddons LIB_NAME KF5CoreAddons DEPS > "core" FILENAME_VAR PRI_FILENAME INCLUDE_INSTALL_DIR > ${KF5_INCLUDE_INSTALL_DIR}/KCoreAddons) > +ecm_generate_pkgconfig_file(BASE_NAME KCoreAddons LIB_NAME KF5CoreAddons > DEPS Qt5Core INCLUDE_INSTALL_DIR ${KF5_INCLUDE_INSTALL_DIR}/KCoreAddons > INSTALL) > install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR}) > > This is the result, on my system: > > Name: KF5CoreAddons > Version: 5.1.0 > Libs: -L/home/kde-devel/kde5/lib64 -l/home/kde-devel/kde5/lib64 > Cflags: -I/home/kde-devel/kde5/include/KF5/KCoreAddons > Requires: Qt5Core > > > Thanks, > > Aleix Pol Gonzalez > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
