Skip to content

Commit

Permalink
Remove all global config state.
Browse files Browse the repository at this point in the history
- No more config cache.
- No more global default bootstrap options, so also no dynamic
  resetting of those...
- Removed many superfluous BUILD dependencies on the config library.
- test_gen_tasks_options_reference_data no longer broken.

There are now only 4 deps on config in the entire codebase:
1. The options system depends on it, of course.
2. The migrate config script.
3. Android keystore code, which uses it to reads its own config
   (which has nothing to do with the main pants config).
4. Its unittest.

So this commit completes the process of switching everything over
to new-style options. It should now be possible (with maybe some
tweaking of defaults) to run pants with an empty pants.ini.
I have not tried this... :)

Actually, the above statement is not quite true: backends are still read
directly from config.

Testing Done:
CI, amazingly, passed on the first attempt: https://travis-ci.org/pantsbuild/pants/builds/62586666

Bugs closed: 1544

Reviewed at https://rbcommons.com/s/twitter/r/2222/
  • Loading branch information
benjyw authored and Benjy committed May 14, 2015
1 parent 653d786 commit 9bd9f7f
Show file tree
Hide file tree
Showing 20 changed files with 84 additions and 161 deletions.
1 change: 0 additions & 1 deletion src/python/pants/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ python_library(
dependencies=[
'3rdparty/python/twitter/commons:twitter.common.collections',
'3rdparty/python:six',
'src/python/pants/base:config',
'src/python/pants/base:exceptions',
'src/python/pants/util:contextutil',
'src/python/pants/util:dirutil',
Expand Down
4 changes: 0 additions & 4 deletions src/python/pants/backend/codegen/targets/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ python_library(
'jaxb_library.py',
],
dependencies = [
'3rdparty/python/twitter/commons:twitter.common.collections',
'src/python/pants/backend/jvm/targets:jvm',
'src/python/pants/base:build_environment',
'src/python/pants/base:build_manual',
'src/python/pants/base:config',
'src/python/pants/base:exceptions',
'src/python/pants/base:payload',
'src/python/pants/base:payload_field',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
unicode_literals, with_statement)

from pants.backend.jvm.targets.jvm_target import JvmTarget
from pants.base.build_manual import manual
from pants.base.config import Config
from pants.base.exceptions import TargetDefinitionException


class JavaRagelLibrary(JvmTarget):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
unicode_literals, with_statement)

from pants.backend.jvm.targets.jvm_target import JvmTarget
from pants.base.config import Config
from pants.base.exceptions import TargetDefinitionException


Expand Down
7 changes: 0 additions & 7 deletions src/python/pants/backend/codegen/targets/jaxb_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,7 @@
from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

import sys
from hashlib import sha1
from types import GeneratorType

from twitter.common.collections import OrderedSet

from pants.backend.jvm.targets.jvm_target import JvmTarget
from pants.base.build_environment import get_buildroot
from pants.base.payload import Payload
from pants.base.payload_field import PrimitiveField

Expand Down
2 changes: 0 additions & 2 deletions src/python/pants/backend/core/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ python_library(
'src/python/pants/base:build_environment',
'src/python/pants/base:build_file_parser',
'src/python/pants/base:build_manual',
'src/python/pants/base:config',
'src/python/pants/base:exceptions',
'src/python/pants/base:generator',
'src/python/pants/base:target',
Expand Down Expand Up @@ -253,7 +252,6 @@ python_library(
'3rdparty/python:docutils',
'3rdparty/python:six',
'src/python/pants/base:build_manual',
'src/python/pants/base:config',
'src/python/pants/base:generator',
'src/python/pants/base:target',
'src/python/pants/goal:goal',
Expand Down
11 changes: 1 addition & 10 deletions src/python/pants/backend/core/tasks/reflect.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from six.moves import range

from pants.base.build_manual import get_builddict_info
from pants.base.config import Config
from pants.base.exceptions import TaskError
from pants.base.generator import TemplateData
from pants.base.target import Target
Expand Down Expand Up @@ -418,15 +417,7 @@ def map_symbols(symbols):


def bootstrap_option_values():
try:
return OptionsBootstrapper(buildroot='<buildroot>').get_bootstrap_options().for_global_scope()
finally:
# Today, the OptionsBootstrapper mutates global state upon construction in the form of:
# Config.reset_default_bootstrap_option_values(...)
# As such bootstrap options that use the buildroot get contaminated globally here. We only
# need the contaminated values locally though for doc display, thus the reset of global state.
# TODO(John Sirois): remove this hack when mutable Config._defaults is killed.
Config.reset_default_bootstrap_option_values()
return OptionsBootstrapper(buildroot='<buildroot>').get_bootstrap_options().for_global_scope()


def gen_glopts_reference_data():
Expand Down
110 changes: 34 additions & 76 deletions src/python/pants/base/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,47 +21,6 @@
import configparser as ConfigParser


def reset_default_bootstrap_option_values(defaults, values=None, buildroot=None):
"""Reset the bootstrap options' default values.
:param defaults: The dict to set the values on.
:param values: A namespace containing the values to set. If unspecified, uses
buildroot-based defaults.
:param buildroot: If values is None, use this buildroot to generate the hard-coded defaults.
If unspecified, uses the detected buildroot.
The bootstrapping code will use this to set the bootstrapped values. Code that doesn't trigger
bootstrapping (i.e., the one remaining old-style command) will get the hard-coded defaults, as
it did before.
It's a code smell to update nominally static data dynamically, but this is temporary,
and saves us having to plumb things through to all the Config.from_cache() call sites.
This method is also called in tests of this code, to reset state for unrelated tests.
TODO: Remove after all direct config reads have been subsumed into the options system,
which can pass these into Config.load() itself after bootstrapping them.
"""

buildroot = buildroot or get_buildroot()
defaults.update({
'buildroot': buildroot
})

if values:
defaults.update({
'pants_workdir': values.pants_workdir,
'pants_supportdir': values.pants_supportdir,
'pants_distdir': values.pants_distdir
})
else:
defaults.update({
'pants_workdir': os.path.join(buildroot, '.pants.d'),
'pants_supportdir': os.path.join(buildroot, 'build-support'),
'pants_distdir': os.path.join(buildroot, 'dist')
})


class Config(object):
"""Encapsulates ini-style config file loading and access.
Expand All @@ -70,23 +29,9 @@ class Config(object):
"""
DEFAULT_SECTION = ConfigParser.DEFAULTSECT

_defaults = {
'homedir': os.path.expanduser('~'),
'user': getpass.getuser(),
'pants_bootstrapdir': get_pants_cachedir(),
'pants_configdir': get_pants_configdir()
}
reset_default_bootstrap_option_values(_defaults)

class ConfigError(Exception):
pass

@classmethod
def reset_default_bootstrap_option_values(cls, values=None, buildroot=None):
reset_default_bootstrap_option_values(cls._defaults, values, buildroot)

_cached_config = None

@classmethod
def _munge_configpaths_arg(cls, configpaths):
"""Converts a string or iterable-of-strings argument into a tuple of strings.
Expand All @@ -98,43 +43,56 @@ def _munge_configpaths_arg(cls, configpaths):
return tuple(configpaths) if configpaths else (os.path.join(get_buildroot(), 'pants.ini'),)

@classmethod
def from_cache(cls):
if not cls._cached_config:
raise cls.ConfigError('No config cached.')
return cls._cached_config

@classmethod
def cache(cls, config):
cls._cached_config = config

@classmethod
def load(cls, configpaths=None):
def load(cls, configpaths=None, seed_values=None):
"""Loads config from the given paths.
By default this is the path to the pants.ini file in the current build root directory.
Callers may specify a single path, or a list of the paths of configs to be chained, with
later instances taking precedence over eariler ones.
A handful of seed values will be set to act as if specified in the loaded config file's DEFAULT
section, and be available for use in substitutions. The caller may override some of these
seed values.
Any defaults supplied will act as if specified in the loaded config file's DEFAULT section.
The 'buildroot', invoking 'user' and invoking user's 'homedir' are automatically defaulted.
:param configpaths: Load from these paths. Later instances take precedence over earlier ones.
If unspecified, loads from pants.ini in the current build root directory.
:param seed_values: A dict with optional override seed values for buildroot, pants_workdir,
pants_supportdir and pants_distdir.
"""
configpaths = cls._munge_configpaths_arg(configpaths)
single_file_configs = []
for configpath in configpaths:
parser = cls.create_parser()
parser = cls.create_parser(seed_values)
with open(configpath, 'r') as ini:
parser.readfp(ini)
single_file_configs.append(SingleFileConfig(configpath, parser))
return ChainedConfig(single_file_configs)

@classmethod
def create_parser(cls):
def create_parser(cls, seed_values=None):
"""Creates a config parser that supports %([key-name])s value substitution.
Any defaults supplied will act as if specified in the loaded config file's DEFAULT section and
be available for substitutions, along with all the standard defaults defined above.
A handful of seed values will be set to act as if specified in the loaded config file's DEFAULT
section, and be available for use in substitutions. The caller may override some of these
seed values.
:param seed_values: A dict with optional override seed values for buildroot, pants_workdir,
pants_supportdir and pants_distdir.
"""
return ConfigParser.SafeConfigParser(cls._defaults)
seed_values = seed_values or {}
buildroot = seed_values.get('buildroot', get_buildroot())

all_seed_values = {
'buildroot': buildroot,
'homedir': os.path.expanduser('~'),
'user': getpass.getuser(),
'pants_bootstrapdir': get_pants_cachedir(),
'pants_configdir': get_pants_configdir(),
}

def update_dir_from_seed_values(key, default):
all_seed_values[key] = seed_values.get(key, os.path.join(buildroot, default))
update_dir_from_seed_values('pants_workdir', '.pants.d')
update_dir_from_seed_values('pants_supportdir', 'build-support')
update_dir_from_seed_values('pants_distdir', 'dist')

return ConfigParser.SafeConfigParser(all_seed_values)

# TODO(John Sirois): s/type/type_/

Expand Down
1 change: 0 additions & 1 deletion src/python/pants/bin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ python_library(
'src/python/pants/base:build_file_parser',
'src/python/pants/base:build_graph',
'src/python/pants/base:cmd_line_spec_parser',
'src/python/pants/base:config',
'src/python/pants/base:extension_loader',
'src/python/pants/base:scm_build_file',
'src/python/pants/base:workunit',
Expand Down
14 changes: 5 additions & 9 deletions src/python/pants/bin/goal_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from pants.base.build_file_parser import BuildFileParser
from pants.base.build_graph import BuildGraph
from pants.base.cmd_line_spec_parser import CmdLineSpecParser
from pants.base.config import Config
from pants.base.extension_loader import load_plugins_and_backends
from pants.base.scm_build_file import ScmBuildFile
from pants.base.workunit import WorkUnit
Expand Down Expand Up @@ -53,11 +52,8 @@ def __init__(self, root_dir):

def setup(self):
options_bootstrapper = OptionsBootstrapper()

# Force config into the cache so we (and plugin/backend loading code) can use it.
# TODO: Plumb options in explicitly.
bootstrap_options = options_bootstrapper.get_bootstrap_options()
self.config = Config.from_cache()
config = options_bootstrapper.post_bootstrap_config()

# Get logging setup prior to loading backends so that they can log as needed.
self._setup_logging(bootstrap_options.for_global_scope())
Expand All @@ -68,8 +64,8 @@ def setup(self):
pkg_resources.fixup_namespace_packages(path)

# Load plugins and backends.
backend_packages = self.config.getlist('backends', 'packages', [])
plugins = self.config.getlist('backends', 'plugins', [])
backend_packages = config.getlist('backends', 'packages', [])
plugins = config.getlist('backends', 'plugins', [])
build_configuration = load_plugins_and_backends(plugins, backend_packages)

# Now that plugins and backends are loaded, we can gather the known scopes.
Expand Down Expand Up @@ -98,7 +94,7 @@ def setup(self):

# Now that we have options we can instantiate subsystems.
self.run_tracker = RunTracker.global_instance()
report = initial_reporting(self.config, self.run_tracker)
report = initial_reporting(config, self.run_tracker)
self.run_tracker.start(report)
url = self.run_tracker.run_info.get_info('report_url')
if url:
Expand All @@ -123,7 +119,7 @@ def setup(self):

with self.run_tracker.new_workunit(name='bootstrap', labels=[WorkUnit.SETUP]):
# construct base parameters to be filled in for BuildGraph
for path in self.config.getlist('goals', 'bootstrap_buildfiles', default=[]):
for path in config.getlist('goals', 'bootstrap_buildfiles', default=[]):
build_file = self.address_mapper.from_cache(root_dir=self.root_dir, relpath=path)
# TODO(pl): This is an unfortunate interface leak, but I don't think
# in the long run that we should be relying on "bootstrap" BUILD files
Expand Down
14 changes: 12 additions & 2 deletions src/python/pants/option/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,21 @@
unicode_literals, with_statement)


class RegistrationError(Exception):
class OptionsError(Exception):
"""An options system-related error."""
pass


class BootstrapError(OptionsError):
"""An error at options bootstrapping time."""
pass


class RegistrationError(OptionsError):
"""An error at option registration time."""
pass


class ParseError(Exception):
class ParseError(OptionsError):
"""An error at flag parsing time."""
pass
7 changes: 7 additions & 0 deletions src/python/pants/option/option_value_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ def update(self, attrs):
for k, v in attrs.items():
setattr(self, k, v)

def get(self, key, default=None):
# Support dict-like dynamic access. See also __getitem__ below.
if hasattr(self, key):
return getattr(self, key)
else:
return default

def __setattr__(self, key, value):
if key == '_forwardings':
return super(OptionValueContainer, self).__setattr__(key, value)
Expand Down
Loading

0 comments on commit 9bd9f7f

Please sign in to comment.