Bug 328364 - Unwanted build targets passed to ninja build system
Summary: Unwanted build targets passed to ninja build system
Status: CONFIRMED
Alias: None
Product: kdevelop
Classification: Applications
Component: Build tools: Ninja (show other bugs)
Version: 4.6.60
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-03 09:41 UTC by Kevin Funk
Modified: 2017-02-16 11:08 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Funk 2013-12-03 09:41:17 UTC
When using the ninja builder plugin, sometimes unwanted targets are built.

See steps to reproduce.

Reproducible: Always

Steps to Reproduce:
1. Right click on kdevplatform/shell subdirectory
2. Select 'Build'
Actual Results:  
The following command is called: /home/krf/devel/build/kdevplatform> ninja kdevplatformshell kcm_kdevsourceformattersettings kcm_kdev_ccsettings kcm_kdev_projectsettings kcm_kdev_uisettings kcm_kdev_envsettings kcm_kdev_bgsettings kcm_kdev_pluginsettings shell-sessioncontrollertest shell-documentoperationtest shell-buddytest shell-documentcontrollertest shell-uicontrollertest shell-testcontrollertest krossdeclaration krossidocument krossiuicontroller krosscontextmenuextension krossiprojectbuilder krossqtoolbar krossprojectmodel krosscontext krossvcsrevision krosstopducontext krossducontext krossiproblem krosseditorcontext krossidentifier krossduchainlock krossibuildsystemmanager krossvcslocation krossilanguage krossuse krossiprojectfilemanager krossWrappers duchaintokross

Expected Results:  
The kross* targets shouldn't be part of the build. That didn't happen with the make plugin.

This is probably due to our ninja plugin doing some magic in order to make ninja act as if it supported "building in a subdirectory" (c.f. http://www.cmake.org/pipermail/cmake/2012-January/048507.html).
Comment 1 Aleix Pol 2013-12-03 11:55:20 UTC
Agreed, we should probably only add the ones that are part of the main build.
Comment 2 Kevin Funk 2013-12-30 17:38:09 UTC
Git commit 7e9ccb12797b43ad9deaef00a75a11b8afe92302 by Kevin Funk.
Committed on 30/12/2013 at 17:28.
Pushed by kfunk into branch 'master'.

NinjaBuilder: Don't fetch targets recursively

My argumentation for this patch: *If* there are targets in any of the subdirectories,
these are marked as dependency of one the targets in the current
directory. => We don't need to pass those targets explicitely to the ninja call.

This fixes the issue of importing targets from subdirectories that are
not marked as dependency of one of the targets in the current directory.
C.f. comment #1 in 328364.

M  +0    -3    projectbuilders/ninjabuilder/kdevninjabuilderplugin.cpp

http://commits.kde.org/kdevelop/7e9ccb12797b43ad9deaef00a75a11b8afe92302
Comment 3 Kevin Funk 2013-12-30 17:42:15 UTC
Some more food for thought:

Just had a discussion with Aleix in #kdevelop:
There's another interesting part here which will likely break the ninja integration in KDevelop:
Assume there are two targets in CMakeLists.txt: "uninstall" (which is disruptive) and "all" (which builds all the files). Currently, KDevelop would pass both "uninstall" and "all" to ninja in case we would try to build the folder containing the CMakeLists.txt.

We don't want that. Indeed the solution would be to be more clever regarding the selection of targets we want to build.