Review Request 127638: build system option to build "kdev-clang" and the rest of the IDE separately

René J.V. Bertin rjvbertin at gmail.com
Mon Jan 9 12:15:43 UTC 2017



> On April 12, 2016, 11:31 p.m., Milian Wolff wrote:
> > -2
> > 
> > the ide without the plugins is unusable, this patch makes zero sense to me.
> > 
> > also note that you can already disable sub folders if you really want to do that (I doubt it). Just look at the cache via ccmake e.g.:
> > 
> > ```
> >  BUILD_TESTING                    ON
> >  BUILD_clang                      ON
> >  BUILD_cmake                      ON
> >  BUILD_cmakebuilder               ON
> >  BUILD_custom-buildsystem         ON
> >  BUILD_custom-definesandinclude   ON
> >  BUILD_custommake                 ON
> >  BUILD_executeplasmoid            ON
> >  BUILD_ghprovider                 ON
> >  BUILD_kdeprovider                ON
> >  BUILD_kdev_includepathsconvert   ON
> >  BUILD_kdev_makefileresolver      OFF
> >  BUILD_kdev_msvcdefinehelper      OFF
> >  BUILD_manpage                    ON
> >  BUILD_plugins                    ON
> >  BUILD_qmake                      ON
> >  BUILD_qmake_parser               OFF
> >  BUILD_qmakebuilder               ON
> >  BUILD_qmljs                      ON
> >  BUILD_qthelp                     ON
> > ```
> 
> Aleix Pol Gonzalez wrote:
>     Agreed, I don't see the value in the patch either.
> 
> René J.V. Bertin wrote:
>     Have you even looked at what this does exactly? 
>     
>     > the ide without the plugins is unusable
>     
>     This patch *makes it possible* for someone *who has a reason to do so* to build the IDE with *all* plugins *except* the kdev-clang plugin. That leaves all other plugins, so the IDE should remain usable for everything that doesn't require libclang.
>     
>     This patch is intended as a convenience for packagers. It was *not* designed to allow installing the IDE *without* ever installing the corresponding kdev-clang plugin, only to be able to build and package the IDE and the plugin separately. With the emphasis on *build and*; in case it still isn't clear not all packaging/distribution systems support creating multiple packages from a single build.
>     
>     Do I have to understand that `-DKDEVELOP_BUILD_CLANG_PARSER=OFF` is equivalent to `-DBUILD_clang=OFF`? That isn't apparent from the CMake files.
> 
> Milian Wolff wrote:
>     Yes, you can use `-DBUILD_*=OFF` to disable features. It works for all others, so I suggest you fix your packaging system if it doesn't work. Worst-case, ship this patch there if you really need it. I don't want to maintain that in our code base.
> 
> René J.V. Bertin wrote:
>     It doesn't look there's a way to build only kdev-clang with that approach which isn't an option anyway because -DBUILD_clang=OFF does NOT disable the required dependency on clang.
>     
>     Let me ask something else just to know exactly how I might fix the (*NOT* _mine_) packaging system:
>     
>     how is the standalone-app-bundle build going to be implemented in the cmake files? Will it be a hardcoded choice (`if(APPLE)`) like the marble project currently does it? Or will it be under proper control of an option variable? That should be hardly any more complicated (`option(APPLE_BUILD_APPBUNDLE,"build as standalone app bundle",YES)` and then `if(APPLE_BUILD_APPBUNDLE)` for everything that's only related to install locations and not additional link properties). But the present patch isn't exactly complicated either ...
> 
> Kevin Funk wrote:
>     > It doesn't look there's a way to build only kdev-clang with that approach which isn't an option anyway because -DBUILD_clang=OFF does NOT disable the required dependency on clang.
>     
>     Then do:
>     ```
>     if (NOT BUILD_clang)
>        find_package(...)
>     ...
>     ```
> 
> René J.V. Bertin wrote:
>     Or, you move the `find_package(Clang ...)` logic into languages/clang/CMakeLists.txt so that `ecm_optional_add_subdirectory(clang)` can indeed be turned off with `-DBUILD_clang=OFF` on the commandline. I'd appreciate if that could be done, btw.
>     
>     Other than that this more or less what I'm already doing, with a different option. But skipping the clang parser build is only part of what I'm still doing in my packaging scripts, which also need to be able to build only the clang parser.
>     
>     The alternative would be to patch languages/clang/CMakeLists.txt so that it can act as a standalone project, something I'll explore when the maintenance load of my current patch becomes too high.
> 
> Kevin Funk wrote:
>     > The alternative would be to patch languages/clang/CMakeLists.txt so that it can act as a standalone project, something I'll explore when the maintenance load of my current patch becomes too high.
>     
>     Unlikely that we're going to accept that either (just to make sure you don't waste time asking us to review that). You'd need to copy-paste lots of CMake setup code (cf. head of kdevelop.git:CMakeLists.txt) to get this done. => Maintenance burden.
>     
>     As said earlier. We/I don't see a lot of value in this patch either, and we don't want to maintain it upstream.
>     
>     No other distro has problems with the libclang dependency.

It would definitely make less sense to incorporate such a patch upstream, but in terms of maintenance burden it should be easier. I think the requirements of the clang plugin are less likely to change very often, while the patch to make the entire build system build only that plugin can require refactoring much more frequently. Again in terms of maintenance burden my current patch shouldn't be hard to maintain upstream at all given that the source tree is well organised (it just requires a bit of discipline, but you already have that ;)) but I've decided not to insist quite a while ago already.

As to other distros most probably either have support for creating multiple packages from a single build and use that for KDevelop and Clang OR they build against a single central clang install (which can evolve independently) OR don't care Arch/Gentoo-style about simply building everything for every install. OR they never bothered bringing the topic up. Only the last 2 alternatives are options for me.


- René J.V.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127638/#review94558
-----------------------------------------------------------


On April 12, 2016, 6:34 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127638/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 6:34 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> This patch is in reaction to a discussion on kdevelop-devel about weakening KDevelop's dependency on a specific Clang (libclang) version.
> 
> The patch introduces 2 CMake options:
> - `KDEVELOP_BUILD_IDE` builds just the IDE, without the clang-based parser (formerly kdev-clang)
> - `KDEVELOP_BUILD_CLANG_PARSER` builds just the clang-based parser
> 
> Nothing changes when both options are on (the default): all of KDevelop is built. When both options are off, an error is raised.
> 
> The idea is that this should make it a little bit easier for distributions/packaging systems and users to change the clang toolchain.
> 
> I presume that the languages subdirectory could be built first or last (that would the patch a bit simpler)?
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 2d3faa7 
>   languages/CMakeLists.txt 4e2fde2 
>   languages/clang/CMakeLists.txt c19a951 
> 
> Diff: https://git.reviewboard.kde.org/r/127638/diff/
> 
> 
> Testing
> -------
> 
> On OS X and Linux, against Qt 5.6.0, FWs 5.20.0, prefix=/opt/local
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170109/229f86c4/attachment.html>


More information about the KDevelop-devel mailing list