Seems OK to me. This has a few conflicts with the sources/javadoc patch I have outstanding but I don't think they will be difficult to resolve.
Some refactoring of IvyUtils.
Review Request #1626 — Created Jan. 19, 2015 and submitted
|ity, jsirois, patricklaw, stuhood, zundel|
- IvyUtils is now just a repository of classmethods, and is not instantiated.
This is a better idiom for something called "Utils", and most of the
instance methods didn't need to be anyway.
- The relevant dynamic code has been moved to IvyTaskMixin, where it can
directly use the task's context, without dipping directly into pants.ini.
- The ImportUtils subclass is no longer needed: The differentiation of
workdirs now comes simply because IvyResolve and IvyImports are two
distinct tasks, with two distinct workdirs. IvyUtils no longer reads
its workdir out of config, but uses the task's workdir.
- The mixin now registers a --jvm-options option, to allow its subtypes
(currently IvyResolve and BootstrapJvmTools) to tweak the JVM when
running Ivy. To get this registration to work, the mixin was moved
before the Task superclass in the mro.
- Got rid of the ivy.xml symlink hack.
The purpose, and result, of these changes, was to remove some direct
accesses to pants.ini. However this change also has the nice effect of
getting rid of the weird IvyUtils/IvyTaskMixin dichotomy and making
things more uniform. Ivy code is now quite a bit easier to reason about.
CI passes: https://travis-ci.org/pantsbuild/pants/builds/47480451
LGTM - but please add a Twitter reviewer if even just for a post-facto heads up. I'm almost positive this will give them a minor break to mend on an internal Task (subclass of IvyUtils will need to instead subclass IvyTaskMixin).