Do not build tests and their dependencies by default
Opened this issue · 10 comments
qibuild configure and make build "testtime" dependencies (unless you add --build-deps-only). When you work on a large project, with lots of tests, you don't need to build those and this can drag a lot more code to compile.
I think a switch to select the type of dependencies like --testtime/--buildtime/--runtime during configure would be a good idea. Those switches can add some predefine variables to cmake (like QI_WITH_TESTS=ON) and so on...
-1: I'm in favor of safety by default. If you're working on a project you should run the tests. Building the tests also helps knowing about API breakages. So IMO not configuring the test by default is encouraging sloppy development.
And by the way configuring the tests does not mean that they should be built, we could avoid calling "make all". I suspect qibuild could be improved in that regard.
If your full worktree was a single cmake project, you would not call make all on all your deps, you would just build the stuff you really need. (That would be a big change though, probably not realistic).
-1: why adding new CLI option espcially when this can already be done with a simple profile?
we could avoid calling "make all".
No we can't. We want to install the tests and this means they must be included in the "all" target
cmake_minimum_required(VERSION 2.8)
project(foo)
add_executable(foo EXCLUDE_FROM_ALL main.cpp)
install(TARGETS foo DESTINATION bin)$ cmake ..
WARNING: Target "foo" has EXCLUDE_FROM_ALL set
and will not be built by default but an install
rule has been provided for it.
CMake does not define behavior for this case.
(In a previous version I'm pretty sure it raised an error)
If you're working on a project you should run the tests.
That's my point. If i am hacking (API,ABI,behavior) on upstream dependency (think libqi), i need those tests, to fix them, and also all the downstream project i know of that depends on libqi. So I already have to build anything, and their tests, that depends on libqi to fix them. Nothing new here.
If I work on a downstream project, i don't care about those libqi tests, and everything in between. I will not have any impact on them, at all. And all that code and dependencies are in the way. I have a project with 170 qiproject dependencies, a good chunk of that is testtime dependencies/code.
configuring the tests does not mean that they should be built
Why I would want to configure them, if i don't want to build them ? Seems like unnecessary work to me.
why adding new CLI option espcially when this can already be done with a simple profile
I may have missed that. I will look into it.
we could avoid calling "make all".
No we can't. We want to install the tests and this means they must be included in the "all" target
I think ideally the tests should be included in the "all" target (that is what "all" means by the way), but AlexandreBossard would never call "make all", but just call "make mystuff", which would depend on the upstream library but not on the upstream tests.
(however, you probably thought about all those matters more than me...)
If you're working on a project you should run the tests.
That's my point. If i am hacking (API,ABI,behavior) on upstream dependency
(think libqi), i need those tests, to fix them, and also all the downstream project
i know of that depends on libqi. So I already have to build anything, and their tests,
that depends on libqi to fix them. Nothing new here.
I don't trust people to do that. I prefer safer defaults. But that is a matter of opinion, of course.
And I also guess the usual answer holds: you don't want to build libqi tests if you don't hack it, but in such a case you should probably not build libqi at all. We'd better work on pre-built upstream projects rather than tweaking option defaults.
But maybe what you suggest is more subtle than what I understood.
How would you specify on which projects you want to build tests or not?
configuring the tests does not mean that they should be built
Why I would want to configure them, if i don't want to build them ? Seems like unnecessary work to me.
My perception is that, when doing incremental builds, qibuild configure is costly since it might trigger big rebuilds. I don't think that configuring more targets harms much.
Also, the tests listings are generated at configure time, so fiddling with reconfigure seems particularly bug prone.
I quite often switch working from upstream to downstream projects in the same worktree so this use-case matters a lot to me.
I do not want to remember how I first configured the libqi checkout 3 weeks ago. I want my worktree to be as stateless as possible.
And I want qitest to be trust-able.
Currently it is not: if I run qitest run --nightly, but did not provide -DQI_WITH_NIGHTLY_TESTS=ON when I configured the project (weeks ago), the nightly tests won't be built nor run (with no warning), I need to be extra awake to notice it.
This problem would not exist if nightly tests were configured by default (and maybe not built by default, I don't care much).
I don't trust people to do that. I prefer safer defaults. But that is a matter of opinion, of course.
People already struggle to think about running them/updating them. I see it every and i am more guilty than I want to admit it. So, that's already an issue, but that's not the issue.
I don't think that configuring more targets harms much.
More target means more dependencies which equals to more code to compile. I just want to be sure that we compile what we need to be compiled. Tests/targets that I don't need because I don't need to run them is unnecessary drag.
I want my worktree to be as stateless as possible.
A worktree is already stateful, like, a lot. Be your configure options or just your source sync (qisrc sync 5 minutes sooner/later, and you obtain i completely different worktree because someone merged), add/remove a dependency. Just move a qiproject and you are doomed to recompile a significant of your worktree. I am not sure how you want it to be stateless, if it could be.
That's why it is so stateful, maybe too much, that the configure/build need to be as fast as possible. binary package seems a bit contrived to me and raise a lot of issues in my head, maybe you are smarter that me.
My reasoning is simple. You work on a "downstream" project, have its test compiled and running in order to update them and only them, you don't need the upstream ones, you don't even know what they do, and don't have to.
Now you have to hack on a "upstream" dependency. Do it, compile and run the tests, because now you need them. If your coverage is good enough, you don't have to do much more except fixing API breakage downstream. Does this make sense, or do i miss something ?
I don't think that configuring more targets harms much.
More target means more dependencies which equals to more code to compile.
...only if you call "make all".
This assertion is completely wrong. More targets means more dependencies for the "all" target, true, but you (downstream) should not depend on the "all" target. You should depend on the library.
The view I defend is that if you don't want to compile the test you should not call "make all" (or qibuild should not do it on your behalf). Fixing build performance issues at configure time seems wrong to me.
You have the exact same problem when working on a single project:
you work on a feature with its unit test, so the workflow is
while not (all tests build and pass):
fix
while not feature0 is finished:
hack test0;
while not (test0 build and pass):
fix lib
commit
when you're in the inner loop, you just want to build test0 and its dependencies (which include your library), so you do
make test0 && ./sdk/bin/test0
You don't call "make all", except at the end, when you're ready to commit (or hope so) and you also do qitest run --nightly. You do not want to configure in between (there is no QI_WITHOUT_TESTS_EXCEPT_TEST0 option anyway).
I would love to be able to do the same when working on several projects.
Makefiles already provide the "build only what is needed" feature, qibuild under-uses it by calling "make all" everywhere.
Now you have to hack on a "upstream" dependency. Do it, compile and run the tests, because now you need them.
you did not answer this question:
How would you specify on which projects you want to build tests or not?
If what you suggest is to do it manually, I think this is already possible:
define your config with a profile setting QI_WITH_TESTS=OFF, then configure the few projects for which you want tests with qibuild configure -c yourconfig -DQI_WITH_TESTS=ON
It seems to me that the binary toolchain solution is the real solution: you
should not be building what you aren't working on. In the meantime I
workarounded by defining the following aliases in my .zshrc:
alias qc="qibuild configure -DQI_WITH_TESTS=OFF"
alias qct="qibuild configure -DQI_WITH_TESTS=ON"
2016-03-10 9:23 GMT+01:00 Sébastien BARTHÉLÉMY notifications@github.com:
@AlexandreBossard https://github.com/AlexandreBossard
I don't think that configuring more targets harms much.
More target means more dependencies which equals to more code to compile.
...only if you call "make all".
This assertion is completely wrong. More targets means more dependencies
for the "all" target, true, but you (downstream) should not depend on the
"all" target. You should depend on the library.The view I defend is that if you don't want to compile the test you should
not call "make all" (or qibuild should not do it on your behalf). Fixing
build performance issues at configure time seems wrong to me.You have the exact same problem when working on a single project:
you work on a feature with its unit test, so the workflow iswhile not (all tests build and pass):
fix
while not feature0 is finished:
hack test0;
while not (test0 build and pass):
fix lib
commitwhen you're in the inner loop, you just want to build test0 and its
dependencies (which include your library), so you domake test0 && ./sdk/bin/test0
You don't call "make all", except at the end, when you're ready to commit
(or hope so) and you also do qitest run --nightly. You do not want to
configure in between (there is no QI_WITHOUT_TESTS_EXCEPT_TEST0 option
anyway).I would love to be able to do the same when working on several projects.
Makefiles already provide the "build only what is needed" feature, qibuild
under-uses it by calling "make all" everywhere.Now you have to hack on a "upstream" dependency. Do it, compile and run
the tests, because now you need them.you did not answer this question:
How would you specify on which projects you want to build tests or not?
If what you suggest is to do it manually, I think this is already possible:
define your config with a profile setting QI_WITH_TESTS=OFF, then
configure the few projects for which you want tests with qibuild configure
-c yourconfig -DQI_WITH_TESTS=ON—
Reply to this email directly or view it on GitHub
#61 (comment).
*This email and any attachment thereto are confidential and intended solely
for the use of the individual or entity to whom they are addressed.If you
are not the intended recipient, please be advised that disclosing, copying,
distributing or taking any action in reliance on the contents of this email
is strictly prohibited. In such case, please immediately advise the sender,
and delete all copies and attachment from your system.This email shall not
be construed and is not tantamount to an offer, an acceptance of offer, or
an agreement by Aldebaran Robotics on any discussion or contractual
document whatsoever. No employee or agent is authorized to represent or
bind Aldebaran Robotics to third parties by email, or act on behalf of
Aldebaran Robotics by email, without express written confirmation by
Aldebaran Robotics’ duly authorized representatives.*
Ce message électronique et éventuelles pièces jointes sont confidentiels,
et exclusivement destinés à la personne ou l'entité à qui ils sont
adressés.
Si vous n'êtes pas le destinataire visé, vous êtes prié de ne pas
divulguer, copier, distribuer ou prendre toute décision sur la foi de ce
message électronique. Merci d'en aviser immédiatement l'expéditeur et de
supprimer toutes les copies et éventuelles pièces jointes de votre système.
Ce message électronique n'équivaut pas à une offre, à une acceptation
d’offre, ou à un accord d'Aldebaran Robotics sur toute discussion ou
document contractuel quel qu’il soit, et ne peut être interprété comme tel.
Aucun employé ou agent d’Aldebaran Robotics n'est autorisé à représenter ou
à engager la société par email, ou à agir au nom et pour le compte de la
société par email, sans qu’une confirmation écrite soit donnée par le
représentant légal d’Aldebaran Robotics ou par toute autre personne ayant
reçu délégation de pouvoir appropriée.
if I run qitest run --nightly, but did not provide -DQI_WITH_NIGHTLY_TESTS=ON when I configured the project (weeks ago), the nightly tests won't be built nor run (with no warning), I need to be extra awake to notice it.
Looks like a bug to me. You should open an other issue
qibuild under-uses it by calling "make all" everywhere.
Actually we don't call make all, we call cmake --build, but I see your point.
Not sure how we could find out which targets in the dependent project we need to build, though.
(Maybe if we rewrote qibuild to use exported targets we could do something, but currenly with only the
${name)_LIBRARIES variable it seems very hard to accomplish)
@vpaleologue-aldebaran
That's not enough, if you have "testtime" project dependencies in your qiproject, those projects will be build, their tests won't, but the project and all its dependencies will, I think it's a mistake. That's why I want to add a proper switch to qibuild configure to disable tests and not dragging "testtime" projects.
...only if you call "make all".
I use qibuild make myStuff. sometimes qibuild make -s.
This assertion is completely wrong. More targets means more dependencies for the "all" target, true, but you (downstream) should not depend on the "all" target. You should depend on the library.
Agreed, but qibuild does work like that. If you need a cmake target form another project, the whole project will be build, and its dependencies, with all their targets, even those you don't need, because your project has a dependency on this project. That can be some serious domino chained escalation.
The view I defend is that if you don't want to compile the test you should not call "make all" (or qibuild should not do it on your behalf). Fixing build performance issues at configure time seems wrong to me.
Maybe, but again, that's not how qibuild works. qibuild has a configure phase (like that good-old ./configure), where you state what and how you want to build your project, and then a build phase with qibuild make.
One of the reason to have a, hopefully correct, dependency tree is build performance. Only building what you need is the game. That's why makefile exists.
How would you specify on which projects you want to build tests or not?If what you suggest is to do it manually, I think this is already possible:
define your config with a profile setting QI_WITH_TESTS=OFF, then configure the few projects for which you want tests with qibuild configure -c yourconfig -DQI_WITH_TESTS=ON
I think this is where you, and @vpaleologue-aldebaran got me all wrong. This indeed does not build tests, but it won't prevent qibuild to build project you specify in < depends testtime="true" name="behaviormanager" / > tags, for exemple.
If you start from a clean worktree, you don't want to build everybody's test, too long, don't need that. Once you have a built worktree, go to your hack3r project, hit qibuild configure --with-tests (or whatever) then qibuild make hack3r. This will hopefully only build your current project tests and the projects you specified in the qiproject.xml testtime dependencies, if any.
Now, either building tests or not should be the default in qibuild configure, is matter of beliefs and opinions, that I am not too much interested to discussed here.