A PythonRun task.

Review Request #510 — Created June 13, 2014 and submitted

benjyw
pants
pants-reviews
jsirois
Brings python running under goal.

Comitted as 05a7d786f7bc7e432576c9db61b75a2e61a3e984.
Running various binaries. All unittests. 

CI passes.
  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
JS
  1. Nice to see the verbs unite!
    
    It would be great to get an integration test - you already have the binary from your last foray that you can run and test the output of.
    1. Yes, keeping me honest as usual... Done.
      
      ci is baking again.
  2. JvmRun just noops: https://github.com/pantsbuild/pants/blob/master/src/python/pants/backend/jvm/tasks/jvm_run.py#L69
    
    This behavior is less surprising though, but at the end of the day non-uniform behavior is also surprising again.  The answer may be to fix JvmRun to act like this.
    1. Yeah, I will fix JvmRun to match this. Note that I changed the behavior here to be much stricter: it now requires exactly one python_binary specified on the cmd line (rather than "anywhere in the target closure").
  3. This is another builder chroot leak.  It may be useful to add a contextmanager in PythonTask to handle this in 1 spot correctly.
    1. @John, what does this comment mean? Can you point me somewhere?
    2. Benjy has been ripping through command -> goal for python and the prior RB for 'binary' had this issue: https://rbcommons.com/s/twitter/r/470/diff/1/?file=1166370#file1166370line60
    3. Good catch, I got misled by the tempfile.mkdtmp() into thinking that was already being cleaned up somewhere. 
      
      Done.
  4. 
      
ZU
  1. 
      
  2. add a warning if binaries.len > 1? Maybe the command line doesn't work they way the user thinks it does.
  3. 
      
BE
JS
  1. 
      
  2. tests/python/pants_test/python/BUILD (Diff revision 2)
     
     
    dedent this line and the next 2
  3. 
      
BE
  1. Committed as 05a7d786f7bc7e432576c9db61b75a2e61a3e984.
  2. 
      
BE
BE
Review request changed

Status: Closed (submitted)

Loading...