Use NailgunTask's Java distribution consistently.

Review Request #3793 — Created April 30, 2016 and submitted

benjyw
pants
pants-reviews
jsirois, nhoward_tw, stuhood

A few NailgunTasks need the JDK's tools.jar, and were fetching
its path by going to DistributionLocator, instead of using
the distribution it will later execute with.

In practice, today, these likely end up being the same distribution.
But in the future, if a particular task restricts its choice of
distribution then we could end up running in one JDK with the
tools.jar of another JDK on our classpath. This change prevents
that from happening.

CI passes: https://travis-ci.org/pantsbuild/pants/builds/126825440

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
JS
  1. 
      
  2. find_libs raises Distribution.Error.
    1. Right you are. self.set_distribution() catches Distribution.error and reraises a TaskError, so I think that threw me off.

  3. 
      
BE
ST
  1. 
      
  2. I don't think this was accurate before... if there isn't a JDK available at all, then there will be no javac.

    In theory zinc might be using a better search path to locate the JDK and still succeed by forking javac, but it might be best to just allow the error to propagate here.

    1. Good point. I assume this was there for a reason though? Also, does scalac work without a JDK? If so then I don't know if we want to error out here, because we'd be imposing an unnecessary constraint on scala-only repos. Although in that case we also don't really need to log this message...

    2. Looks like the answer to "does scalac work without a JDK" is "no". So I've made this change.

  3. Definitely. Subclasses are also forking without a workunit, which makes its errors hard to find later.

  4. 
      
BE
BE
Review request changed

Status: Closed (submitted)

Change Summary:

e415a5ad6c7b84eee989e719eeab981c632c8017

BE
  1. Submitted. Thanks!

  2. 
      
Loading...