Get rid of the Command class completely.

Review Request #1465 — Created Dec. 10, 2014 and submitted

benjyw
pants
805ba3b...
pants-reviews
ity, jsirois, patricklaw, zundel

GoalRunner now just extends object.

The functionality previously in Command is now in GoalRunner.

Most of the functionality in pants_exe.py is now in GoalRunner too.

RIP old pants...

All integration and unit tests pass.

ZU
  1. 
      
  2. before cleanup() was only called after builds that were interrupted. I think the reason was that out-of-date or nailguns with memory leaks could be timing out or hogging memory on the machine, so cleaning up the nailguns was done to make the next invocation of pants more likely to succeed.

    1. It's true that cleanup() was only called on KeyboardInterrupt, and GoalRunner's cleanup() killed nailguns. However nailguns were also killed in the finally: block in pants_exe.py (if the kill_nailguns was set). It's that always-run nailgun-killing that this code replaces. I repurposed the name "cleanup()" for this, since that is no longer a lifecycle method of Command.

      However there is now the following difference: previously we would unconditionally kill nailguns on KeyboardInterrupt, whereas now we do so only if the kill_nailguns option tells us to. Arguably this is more consistent than the previous behavior, but I could be convinced otherwise. Opinions?

    2. I'm not sure how many problems with nailgun this masks. I'd be inclined to keep the nailgun-killing behavior on CTRL-C unless we prove otherwise that these kinds of issues are behind us.

  3. 
      
BE
PA
  1. Ship It!

  2. 
      
JS
  1. 
      
  2. src/python/pants/bin/BUILD (Diff revision 2)
     
     

    This dep should be moved into :pants_exe - pants_exe.py depends directly on the GoalRunner with an import. At that point this target is a 1-1 alias so it can be killed and instead :pants_exe inlined in the python_binarys below.

    1. Yep. I have a followup change that moves goal_running into bin.

  3. Can you expand on this? Afaict you can actually delete it - there are no local .output and neither GoalRunner.output in the existing codebase.

    1. I have no idea what that is, and noticed the same thing - that it looks like I can kill it. I just copied it over from the old code, and the XXX was to remind me to look into it. So yeah, killed.

  4. 
      
IT
  1. epic!

    1. Truly. The work to add a pure python backend side-by-side with the original ant backend started in 2011. From then forward we've have the command/goal split. This has been a very long time coming, but it marks the true death of the original ant backend.
      Thank you Benjy!

  2. 
      
BE
JS
  1. Ship It!

  2. 
      
BE
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as 6a30e22fa06ec2801136bb59d5cc8b639e8e7394.
Loading...