Skip to content

Commit

Permalink
Switch all remaining accesses of 'context.options' to new options.
Browse files Browse the repository at this point in the history
Killed IvyUtils's dependency on options. It only needed it for
two options that no one is using in practice and this made things
much much simpler.

Also made some methods in IvyUtil classmethods, so that call sites
don't need to instantiate it and pass it config.

Testing Done:
All task unittests passed.

CI passed.

Reviewed at https://rbcommons.com/s/twitter/r/1289/
  • Loading branch information
benjyw authored and Benjy committed Nov 6, 2014
1 parent efbf2a8 commit 55b7316
Show file tree
Hide file tree
Showing 11 changed files with 32 additions and 91 deletions.
10 changes: 4 additions & 6 deletions src/python/pants/backend/codegen/tasks/thrift_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,12 @@
from __future__ import (nested_scopes, generators, division, absolute_import, with_statement,
print_function, unicode_literals)

from pants.backend.codegen.targets.java_thrift_library import JavaThriftLibrary

from pants.base.exceptions import TaskError
from pants.base.workunit import WorkUnit
from pants.backend.core.tasks.console_task import ConsoleTask
from pants.backend.jvm.tasks.jvm_tool_task_mixin import JvmToolTaskMixin
from pants.backend.jvm.tasks.nailgun_task import NailgunTask


class ThriftLinter(NailgunTask, JvmToolTaskMixin):
"""Print linter warnings for thrift files.
"""
Expand Down Expand Up @@ -68,12 +66,12 @@ def is_strict(self, target):
# 2. java_thrift_library target in BUILD file, thrift_linter_strict = False,
# 3. pants.ini, [scrooge-linter] section, strict field.
# 4. default = False
cmdline_strict = self.context.options.thrift_linter_strict
cmdline_strict = self.get_options().strict

if cmdline_strict != None:
if cmdline_strict is not None:
return self._to_bool(cmdline_strict)

if target.thrift_linter_strict != None:
if target.thrift_linter_strict is not None:
return self._to_bool(target.thrift_linter_strict)

return self._to_bool(self.context.config.get(self._CONFIG_SECTION, 'strict',
Expand Down
66 changes: 12 additions & 54 deletions src/python/pants/backend/jvm/ivy_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import errno
import os
import pkgutil
import re
import threading
import xml

Expand Down Expand Up @@ -53,21 +52,14 @@ def get_jars_for_ivy_module(self, jar):


class IvyUtils(object):
"""Useful methods related to interaction with ivy."""

IVY_TEMPLATE_PACKAGE_NAME = __name__
IVY_TEMPLATE_PATH = os.path.join('tasks', 'templates', 'ivy_resolve', 'ivy.mustache')

"""Useful methods related to interaction with ivy."""
def __init__(self, config, options, log):
def __init__(self, config, log):
self._log = log

# TODO(pl): This is super awful, but options doesn't have a nice way to get out
# attributes that might not be there, and even then the attribute value might be
# None, which we still want to override
# Benjy thinks we should probably hoist these options to the global set of options,
# rather than just keeping them within IvyResolve.setup_parser
self._mutable_pattern = (getattr(options, 'ivy_mutable_pattern', None) or
config.get('ivy-resolve', 'mutable_pattern', default=None))

self._transitive = config.getbool('ivy-resolve', 'transitive', default=True)
self._args = config.getlist('ivy-resolve', 'args', default=[])
self._jvm_options = config.getlist('ivy-resolve', 'jvm_args', default=[])
Expand All @@ -76,40 +68,6 @@ def __init__(self, config, options, log):
self._workdir = os.path.join(config.getdefault('pants_workdir'), 'ivy')
self._template_path = self.IVY_TEMPLATE_PATH

if self._mutable_pattern:
try:
self._mutable_pattern = re.compile(self._mutable_pattern)
except re.error as e:
raise TaskError('Invalid mutable pattern specified: %s %s' % (self._mutable_pattern, e))

def parse_override(override):
match = re.match(r'^([^#]+)#([^=]+)=([^\s]+)$', override)
if not match:
raise TaskError('Invalid dependency override: %s' % override)

org, name, rev_or_url = match.groups()

def fmt_message(message, template):
return message % dict(
overridden='%s#%s;%s' % (template.org, template.module, template.version),
rev=rev_or_url,
url=rev_or_url)

def replace_rev(template):
self._log.info(fmt_message('Overrode %(overridden)s with rev %(rev)s', template))
return template.extend(version=rev_or_url, url=None, force=True)

def replace_url(template):
self._log.info(fmt_message('Overrode %(overridden)s with snapshot at %(url)s', template))
return template.extend(version='SNAPSHOT', url=rev_or_url, force=True)

replace = replace_url if re.match(r'^\w+://.+', rev_or_url) else replace_rev
return (org, name), replace
self._overrides = {}
# TODO(pl): See above comment wrt options
if hasattr(options, 'ivy_resolve_overrides') and options.ivy_resolve_overrides:
self._overrides.update(parse_override(o) for o in options.ivy_resolve_overrides)

@staticmethod
def _generate_exclude_template(exclude):
return TemplateData(org=exclude.org, name=exclude.name)
Expand Down Expand Up @@ -173,22 +131,25 @@ def symlink_cachepath(ivy_home, inpath, symlink_dir, outpath):
symlink_map = dict(zip(paths, new_paths))
return symlink_map

def identify(self, targets):
@staticmethod
def identify(targets):
targets = list(targets)
if len(targets) == 1 and targets[0].is_jvm and getattr(targets[0], 'provides', None):
return targets[0].provides.org, targets[0].provides.name
else:
return 'internal', Target.maybe_readable_identify(targets)

def xml_report_path(self, targets, conf):
@classmethod
def xml_report_path(cls, targets, conf):
"""The path to the xml report ivy creates after a retrieve."""
org, name = self.identify(targets)
org, name = cls.identify(targets)
cachedir = Bootstrapper.instance().ivy_cache_dir
return os.path.join(cachedir, '%s-%s-%s.xml' % (org, name, conf))

def parse_xml_report(self, targets, conf):
@classmethod
def parse_xml_report(cls, targets, conf):
"""Returns the IvyInfo representing the info in the xml report, or None if no report exists."""
path = self.xml_report_path(targets, conf)
path = cls.xml_report_path(targets, conf)
if not os.path.exists(path):
return None

Expand Down Expand Up @@ -323,8 +284,6 @@ def _resolve_conflict(self, existing, proposed):
def _is_mutable(self, jar):
if jar.mutable is not None:
return jar.mutable
if self._mutable_pattern:
return self._mutable_pattern.match(jar.rev)
return False

def _generate_jar_template(self, jar, confs):
Expand All @@ -338,8 +297,7 @@ def _generate_jar_template(self, jar, confs):
transitive=jar.transitive,
artifacts=jar.artifacts,
configurations=[conf for conf in jar.configurations if conf in confs])
override = self._overrides.get((jar.org, jar.name))
return override(template) if override else template
return template

def mapto_dir(self):
"""Subclasses can override to establish an isolated jar mapping directory."""
Expand Down
5 changes: 1 addition & 4 deletions src/python/pants/backend/jvm/tasks/depmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,6 @@ def __init__(self, *args, **kwargs):
self.separator = self.get_options().separator
self.project_info = self.get_options().project_info
self.format = self.get_options().project_info_formatted
self._ivy_utils = IvyUtils(config=self.context.config,
options=self.context.options,
log=self.context.log)

def console_output(self, targets):
if len(self.context.target_roots) == 0:
Expand Down Expand Up @@ -231,7 +228,7 @@ def output_deps(outputted, dep, parent=None):
def project_info_output(self, targets):
targets_map = {}
resource_target_map = {}
ivy_info = self._ivy_utils.parse_xml_report(targets, 'default')
ivy_info = IvyUtils.parse_xml_report(targets, 'default')

def process_target(current_target):
"""
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/jvm/tasks/ivy_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

class ImportsUtil(IvyUtils):
def __init__(self, context):
super(ImportsUtil, self).__init__(context.config, context.options, context.log)
super(ImportsUtil, self).__init__(context.config, context.log)

def is_mappable_artifact(self, org, name, path):
return path.endswith('.jar') and super(ImportsUtil, self).is_mappable_artifact(org, name, path)
Expand Down
12 changes: 6 additions & 6 deletions src/python/pants/backend/jvm/tasks/ivy_resolve.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ def __init__(self, *args, **kwargs):
ini_key='bootstrap-tools',
default=['//:xalan'])

self._ivy_utils = IvyUtils(config=self.context.config,
options=self.context.options,
log=self.context.log)
self._ivy_utils = IvyUtils(config=self.context.config, log=self.context.log)

# Typically this should be a local cache only, since classpaths aren't portable.
self.setup_artifact_cache_from_config(config_section=self._CONFIG_SECTION)
Expand Down Expand Up @@ -137,6 +135,8 @@ def execute(self):
if create_jardeps_for:
genmap = self.context.products.get('jar_dependencies')
for target in filter(create_jardeps_for, targets):
# TODO: Add mapjars to IvyTaskMixin? Or get rid of the mixin? It's weird that we use
# self.ivy_resolve for some ivy invocations but this for others.
self._ivy_utils.mapjars(genmap, target, executor=executor,
workunit_factory=self.context.new_workunit)

Expand All @@ -150,7 +150,7 @@ def _populate_ivy_jar_products(self, targets):
"""Populate the build products with an IvyInfo object for each generated ivy report."""
ivy_products = self.context.products.get_data('ivy_jar_products') or defaultdict(list)
for conf in self._confs:
ivyinfo = self._ivy_utils.parse_xml_report(targets, conf)
ivyinfo = IvyUtils.parse_xml_report(targets, conf)
if ivyinfo:
# Value is a list, to accommodate multiple exclusives groups.
ivy_products[conf].append(ivyinfo)
Expand Down Expand Up @@ -181,7 +181,7 @@ def make_empty_report(report, organisation, module, conf):
classpath = self.tool_classpath(self._ivy_bootstrap_key, self.create_java_executor())

reports = []
org, name = self._ivy_utils.identify(targets)
org, name = IvyUtils.identify(targets)
xsl = os.path.join(self._cachedir, 'ivy-report.xsl')

# Xalan needs this dir to exist - ensure that, but do no more - we have no clue where this
Expand All @@ -190,7 +190,7 @@ def make_empty_report(report, organisation, module, conf):

for conf in self._confs:
params = dict(org=org, name=name, conf=conf)
xml = self._ivy_utils.xml_report_path(targets, conf)
xml = IvyUtils.xml_report_path(targets, conf)
if not os.path.exists(xml):
make_empty_report(xml, org, name, conf)
out = os.path.join(self._outdir, '%(org)s-%(name)s-%(conf)s.html' % params)
Expand Down
4 changes: 1 addition & 3 deletions src/python/pants/backend/jvm/tasks/ivy_task_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ def ivy_resolve(self,
return []

ivy_workdir = os.path.join(self.context.config.getdefault('pants_workdir'), 'ivy')
ivy_utils = IvyUtils(config=self.context.config,
options=self.context.options,
log=self.context.log)
ivy_utils = IvyUtils(config=self.context.config, log=self.context.log)

fingerprint_strategy = IvyResolveFingerprintStrategy()

Expand Down
9 changes: 3 additions & 6 deletions src/python/pants/backend/jvm/tasks/provides.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ def register_options(cls, register):

def __init__(self, *args, **kwargs):
super(Provides, self).__init__(*args, **kwargs)
self.ivy_utils = IvyUtils(config=self.context.config,
options=self.context.options,
log=self.context.log)
self.confs = self.context.config.getlist('ivy', 'confs', default=['default'])
self.target_roots = self.context.target_roots
self.transitive = self.get_options().transitive
Expand All @@ -53,11 +50,11 @@ def execute(self):
safe_mkdir(self.workdir)
targets = self.context.targets()
for conf in self.confs:
outpath = os.path.join(self.workdir, '%s.%s.provides' %
(self.ivy_utils.identify(targets)[1], conf))
outpath = os.path.join(self.workdir,
'{0}.{1}.provides'.format(IvyUtils.identify(targets)[1], conf))
if self.transitive:
outpath += '.transitive'
ivyinfo = self.ivy_utils.parse_xml_report(self.context.target_roots, conf)
ivyinfo = IvyUtils.parse_xml_report(self.context.target_roots, conf)
jar_paths = OrderedSet()
for root in self.target_roots:
jar_paths.update(self.get_jar_paths(ivyinfo, root, conf))
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/engine/round_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def attempt(self, context, goals):
execution_goals = ' -> '.join(e.goal.name for e in goal_executors)
context.log.info('Executing tasks in goals: {goals}'.format(goals=execution_goals))

explain = getattr(context.options, 'explain', False)
explain = context.new_options.for_global_scope().explain
if explain:
print('Goal Execution Order:\n\n%s\n' % execution_goals)
print('Goal [TaskRegistrar->Task] Order:\n')
Expand Down
1 change: 1 addition & 0 deletions tests/python/pants_test/engine/test_round_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class RoundEngineTest(EngineTestBase, BaseTest):
def setUp(self):
super(RoundEngineTest, self).setUp()

self.set_new_options_for_scope('', explain=False)
self._context = self.context()
self.assertTrue(self._context.is_unlocked())

Expand Down
1 change: 0 additions & 1 deletion tests/python/pants_test/jvm/jvm_tool_task_test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ def alias_groups(self):

def setUp(self):
real_config = self.config()

super(JvmToolTaskTestBase, self).setUp()

def link(path, optional=False, force=False):
Expand Down
11 changes: 2 additions & 9 deletions tests/python/pants_test/tasks/test_ivy_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,9 @@
from pants.backend.jvm.register import build_file_aliases as register_jvm
from pants.util.contextutil import temporary_file_path
from pants_test.base_test import BaseTest
from pants_test.base.context_utils import create_config


class IvyUtilsTestBase(BaseTest):
@staticmethod
def create_options(**kwargs):
options = dict(ivy_mutable_pattern=None,
ivy_resolve_overrides=None)
options.update(**kwargs)
return options

@property
def alias_groups(self):
return register_core().merge(register_jvm())
Expand Down Expand Up @@ -55,7 +47,8 @@ def setUp(self):
"""))

self.simple = self.target('src/java/targets:simple')
self.ivy_utils = IvyUtils(create_config(), self.create_options(), logging.Logger('test'))
context = self.context()
self.ivy_utils = IvyUtils(context.config, logging.Logger('test'))

def test_force_override(self):
jars = list(self.simple.payload.jars)
Expand Down

0 comments on commit 55b7316

Please sign in to comment.