Fixup clean-all, clean-all-async and invalidate dependencies.

Review Request #638 — Created July 8, 2014 and submitted

jsirois
pants
jsirois/goals/cleanall_dep_fix
319
pants-reviews
benjyw, ity
Under the RoundEngine Goal.dependencies is not repected, so arrange {clean-all, clean-all-async} -> invalidate -> ng-killall dep by hand.
This exposes the awkward relationship of ng-killall in the jvm backend to the core pants clean-up tasks in the core backend.  No solution is offered to that awkwardness in this review.

commit 649b62a5e4914d5a5a01772785bf152593538e01
Author: John Sirois <jsirois@twitter.com>
Date:   Mon Jul 7 08:55:48 2014 -0700

    Fixup clean-all, clean-all-async and invalidate dependencies.
    
    In the wake of Goal.dependencies no longer being used for scheduling,
    clean-all and invalidate no longer properly depend on ng-killall.

 src/python/pants/backend/core/register.py | 14 +++++++-------
 src/python/pants/backend/jvm/register.py  | 10 ++++++----
 2 files changed, 13 insertions(+), 11 deletions(-)
Ad-hoc:
$ for goal in clean-all clean-all-async invalidate ng-killall; do PANTS_DEV=1 ./pants goal $goal -e 2>/dev/null | grep $goal | grep "\["; done
clean-all [ng-killall->NailgunKillall, invalidate->Invalidator, clean-all->Cleaner]
clean-all-async [ng-killall->NailgunKillall, invalidate->Invalidator, clean-all-async->AsyncCleaner]
invalidate [ng-killall->NailgunKillall, invalidate->Invalidator]
ng-killall [ng-killall->NailgunKillall]
  • 0
  • 0
  • 0
  • 1
  • 1
Description From Last Updated
BE
  1. LGTM, just a couple of incidental questions.
  2. Not immediately actionable, but at some point is the plan to get rid of the old execution engines? It seems silly to maintain the dependencies if we're never going to need them again.
    1. We need them for flags today.  Ity is working on this very thing.
  3. We don't technically need invalidate in clean_all's phase, as clean_all nukes all of .pants.d (including the build invalidator dir). But there's no harm in it, and it may be semantically more correct?
    1. I think it may be semantically more correct, but I'll let keep supposed existing behavior as the motivation winner here.
  4. In the old execution engine, won't ng-killall now run twice - once because its a dependency of clean-all and once because it's installed into clean-all's phase? Or does the de-duping change (that I haven't looked at yet) fix this? Or is it not an issue for some other reason?
    1. There is no old engine, there is just the Engine baseclass which does ~nothing except wrap up TaskError into graceful exit.  The confusing bit it the Engines ordering method for flag parsing, that today is a parallel graph needed to arrange all the right flags - Ity is attacking this.
  5. 
      
IT
  1. Ship It!
  2. 
      
JS
  1. Thanks folks - submitted @ https://github.com/pantsbuild/pants/commit/724d49074a305a8802db6d07a51827b0fb2ba83a
  2. 
      
JS
Review request changed

Status: Closed (submitted)

Loading...