Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use `type=` to register enum options #7304

Merged
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -4,6 +4,8 @@

from __future__ import absolute_import, division, print_function, unicode_literals

from builtins import str

from pants.backend.jvm.targets.jvm_target import JvmTarget
from pants.build_graph.target_scopes import Scope
from pants.subsystem.subsystem import Subsystem
@@ -118,9 +118,9 @@ def classify_target(cls, target):
if target.has_sources('.java') or \
isinstance(target, JUnitTests) or \
(isinstance(target, ScalaLibrary) and tuple(target.java_sources)):
target_type = _JvmCompileWorkflowType('zinc-only')
target_type = _JvmCompileWorkflowType.zinc_only
elif target.has_sources('.scala'):
target_type = _JvmCompileWorkflowType('rsc-then-zinc')
target_type = _JvmCompileWorkflowType.rsc_then_zinc
else:
target_type = None
return target_type
@@ -14,8 +14,7 @@
from pants.java.nailgun_executor import NailgunExecutor, NailgunProcessGroup
from pants.process.subprocess import Subprocess
from pants.task.task import Task, TaskBase
from pants.util.memo import memoized_property
from pants.util.objects import enum, register_enum_option
from pants.util.objects import enum


class NailgunTaskBase(JvmToolTaskMixin, TaskBase):
@@ -30,11 +29,11 @@ class ExecutionStrategy(enum([NAILGUN, SUBPROCESS, HERMETIC])): pass
@classmethod
def register_options(cls, register):
super(NailgunTaskBase, cls).register_options(register)
register_enum_option(
register, cls.ExecutionStrategy, '--execution-strategy', default=cls.NAILGUN,
help='If set to nailgun, nailgun will be enabled and repeated invocations of this '
'task will be quicker. If set to subprocess, then the task will be run without nailgun. '
'Hermetic execution is an experimental subprocess execution framework.')
register('--execution-strategy',
default=cls.ExecutionStrategy.nailgun, type=cls.ExecutionStrategy,
help='If set to nailgun, nailgun will be enabled and repeated invocations of this '
'task will be quicker. If set to subprocess, then the task will be run without '
'nailgun. Hermetic execution is an experimental subprocess execution framework.')
register('--nailgun-timeout-seconds', advanced=True, default=10, type=float,
help='Timeout (secs) for nailgun startup.')
register('--nailgun-connect-attempts', advanced=True, default=5, type=int,
@@ -47,12 +46,9 @@ def register_options(cls, register):
rev='0.9.1'),
])

@memoized_property
@property
def execution_strategy_enum(self):
# TODO: This .create() call can be removed when the enum interface is more stable as the option
# is converted into an instance of self.ExecutionStrategy via the `type` argument through
# register_enum_option().
return self.ExecutionStrategy(self.get_options().execution_strategy)
return self.get_options().execution_strategy

@classmethod
def subsystem_dependencies(cls):
@@ -12,7 +12,7 @@
from pants.subsystem.subsystem import Subsystem
from pants.util.memo import memoized_property
from pants.util.meta import classproperty
from pants.util.objects import enum, register_enum_option
from pants.util.objects import enum


class ToolchainVariant(enum(['gnu', 'llvm'])): pass
@@ -37,22 +37,16 @@ def register_options(cls, register):
help='The default for the "compiler_option_sets" argument '
'for targets of this language.')

register_enum_option(
register, ToolchainVariant, '--toolchain-variant', advanced=True, default='gnu',
help="Whether to use gcc (gnu) or clang (llvm) to compile C and C++. Currently all "
"linking is done with binutils ld on Linux, and the XCode CLI Tools on MacOS.")
register('--toolchain-variant', advanced=True,
default=ToolchainVariant.gnu, type=ToolchainVariant,
help="Whether to use gcc (gnu) or clang (llvm) to compile C and C++. Currently all "
"linking is done with binutils ld on Linux, and the XCode CLI Tools on MacOS.")

def get_compiler_option_sets_for_target(self, target):
return self.get_target_mirrored_option('compiler_option_sets', target)

def get_toolchain_variant_for_target(self, target):
# TODO(#7233): convert this option into an enum instance using the `type` argument in option
# registration!
enum_or_value = self.get_target_mirrored_option('toolchain_variant', target)
if isinstance(enum_or_value, ToolchainVariant):
return enum_or_value
else:
return ToolchainVariant(enum_or_value)
return self.get_target_mirrored_option('toolchain_variant', target)

@classproperty
def get_compiler_option_sets_enabled_default_value(cls):
@@ -9,7 +9,7 @@
from future.utils import text_type

from pants.base.deprecated import warn_or_error
from pants.base.hash_utils import stable_json_hash
from pants.base.hash_utils import stable_json_sha1
from pants.base.payload import Payload
from pants.base.payload_field import PayloadField
from pants.base.validation import assert_list
@@ -22,7 +22,7 @@
class ConanRequirementSetField(tuple, PayloadField):

def _compute_fingerprint(self):
return stable_json_hash(tuple(hash(req) for req in self))
return stable_json_sha1(tuple(hash(req) for req in self))


class ConanRequirement(datatype([
@@ -13,6 +13,7 @@

from pants.base.deprecated import deprecated
from pants.util.collections_abc_backport import Iterable, Mapping, OrderedDict, Set
from pants.util.objects import DatatypeMixin
from pants.util.strutil import ensure_binary


@@ -89,6 +90,13 @@ def default(self, o):
.format(cls=type(self).__name__, val=o))
# Set order is arbitrary in python 3.6 and 3.7, so we need to keep this sorted() call.
return sorted(self.default(i) for i in o)
elif isinstance(o, DatatypeMixin):
# datatype objects will intentionally raise in the __iter__ method, but the Iterable abstract
# base class will match any class with any superclass that has the attribute __iter__ in the
# __dict__ (see https://docs.python.org/2/library/abc.html), so we need to check for it
# specially here.
# TODO: determine if the __repr__ should be some abstractmethod on DatatypeMixin!
return self.default(repr(o))
elif isinstance(o, Iterable) and not isinstance(o, (bytes, list, str)):
return list(self.default(i) for i in o)
return o
@@ -11,6 +11,7 @@
from packaging.version import Version

from pants.option.arg_splitter import GLOBAL_SCOPE
from pants.option.options_fingerprinter import CoercingOptionEncoder
from pants.option.ranked_value import RankedValue
from pants.task.console_task import ConsoleTask
from pants.version import PANTS_SEMVER
@@ -174,4 +175,4 @@ def console_output(self, targets):
if self.is_json():
inner_map["history"] = history_list
if self.is_json():
yield json.dumps(output_map, indent=2, sort_keys=True)
yield json.dumps(output_map, indent=2, sort_keys=True, cls=CoercingOptionEncoder)
@@ -232,8 +232,7 @@ def _get_starting_indent(source):
def _make_rule(output_type, input_selectors, for_goal=None, cacheable=True):
"""A @decorator that declares that a particular static function may be used as a TaskRule.
:param Constraint output_type: The return/output type for the Rule. This may be either a
concrete Python type, or an instance of `Exactly` representing a union of multiple types.
:param type output_type: The return/output type for the Rule. This must be a concrete Python type.
:param list input_selectors: A list of Selector instances that matches the number of arguments
to the @decorated function.
:param str for_goal: If this is a @console_rule, which goal string it's called for.
@@ -348,8 +347,8 @@ def get_some_union_type(x):

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 6, 2019

Author Contributor

This started erroring out here when running locally and in CI without these (necessary) changes. I'm not sure why this is the case, but that is why this file is changed in this PR.


class UnionRule(datatype([
('union_base', type),
('union_member', type),
('union_base', _type_field),
('union_member', _type_field),
])):
"""Specify that an instance of `union_member` can be substituted wherever `union_base` is used."""

@@ -487,8 +486,8 @@ def add_type_transition_rule(union_rule):
add_rule(rule)
else:
raise TypeError("""\
Unexpected rule type: {}. Rules either extend Rule or UnionRule, or are static functions decorated \
with @rule.""".format(type(entry)))
Rule entry {} had an unexpected type: {}. Rules either extend Rule or UnionRule, or are static \
functions decorated with @rule.""".format(entry, type(entry)))

return cls(serializable_rules, serializable_roots, union_rules)

@@ -28,12 +28,27 @@
from pants.goal.artifact_cache_stats import ArtifactCacheStats
from pants.goal.pantsd_stats import PantsDaemonStats
from pants.option.config import Config
from pants.option.options_fingerprinter import CoercingOptionEncoder
from pants.reporting.report import Report
from pants.stats.statsdb import StatsDBFactory
from pants.subsystem.subsystem import Subsystem
from pants.util.collections_abc_backport import OrderedDict
from pants.util.dirutil import relative_symlink, safe_file_dump


class RunTrackerOptionEncoder(CoercingOptionEncoder):
"""Use the json encoder we use for making options hashable to support datatypes.
This encoder also explicitly allows OrderedDict inputs, as we accept more than just option values
when encoding stats to json.
"""

def default(self, o):
if isinstance(o, OrderedDict):
return o
return super(RunTrackerOptionEncoder, self).default(o)


class RunTracker(Subsystem):
"""Tracks and times the execution of a pants run.
@@ -341,7 +356,7 @@ def error(msg):
# TODO(benjy): The upload protocol currently requires separate top-level params, with JSON
# values. Probably better for there to be one top-level JSON value, namely json.dumps(stats).
# But this will first require changing the upload receiver at every shop that uses this.
params = {k: json.dumps(v) for (k, v) in stats.items()}
params = {k: cls._json_dump_options(v) for (k, v) in stats.items()}
cookies = Cookies.global_instance()
auth_provider = auth_provider or '<provider>'

@@ -371,13 +386,17 @@ def do_post(url, num_redirects_allowed):
except Exception as e: # Broad catch - we don't want to fail the build over upload errors.
return error('Error: {}'.format(e))

@classmethod
def _json_dump_options(cls, stats):
return json.dumps(stats, cls=RunTrackerOptionEncoder)

@classmethod
def write_stats_to_json(cls, file_name, stats):
"""Write stats to a local json file.
:return: True if successfully written, False otherwise.
"""
params = json.dumps(stats)
params = cls._json_dump_options(stats)
if PY2:
params = params.decode('utf-8')
try:
@@ -412,7 +431,7 @@ def store_stats(self):
stats_file = os.path.join(get_pants_cachedir(), 'stats',
'{}.json'.format(self.run_info.get_info('id')))
mode = 'w' if PY3 else 'wb'
safe_file_dump(stats_file, json.dumps(stats), mode=mode)
safe_file_dump(stats_file, self._json_dump_options(stats), mode=mode)

# Add to local stats db.
StatsDBFactory.global_instance().get_db().insert_stats(stats)
@@ -263,9 +263,7 @@ def setup_legacy_graph(native, options_bootstrapper, build_configuration):
options_bootstrapper,
build_configuration,
native=native,
# TODO(#7233): convert this into an enum instance using the `type` argument in option
# registration!
glob_match_error_behavior=GlobMatchErrorBehavior(bootstrap_options.glob_expansion_failure),
glob_match_error_behavior=bootstrap_options.glob_expansion_failure,
build_ignore_patterns=bootstrap_options.build_ignore,
exclude_target_regexps=bootstrap_options.exclude_target_regexp,
subproject_roots=bootstrap_options.subproject_roots,
@@ -10,7 +10,6 @@
import site
from builtins import object, open

from future.utils import PY3
from pex import resolver
from pex.base import requirement_is_exact
from pkg_resources import Requirement
@@ -22,6 +21,7 @@
from pants.util.contextutil import temporary_dir
from pants.util.dirutil import safe_mkdir, safe_open
from pants.util.memo import memoized_property
from pants.util.strutil import ensure_text
from pants.version import PANTS_SEMVER

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 6, 2019

Author Contributor

This was breaking with newstr does not support decode, which kind of makes sense.


@@ -99,7 +99,7 @@ def _resolve_exact_plugin_locations(self):
tmp_plugins_list = resolved_plugins_list + '~'
with safe_open(tmp_plugins_list, 'w') as fp:
for plugin in self._resolve_plugins():
fp.write(plugin.location if PY3 else plugin.location.decode('utf-8'))
fp.write(ensure_text(plugin.location))
fp.write('\n')
os.rename(tmp_plugins_list, resolved_plugins_list)
with open(resolved_plugins_list, 'r') as fp:
@@ -17,6 +17,7 @@
from pants.java.nailgun_protocol import ChunkType, NailgunProtocol
from pants.util.osutil import safe_kill
from pants.util.socket import RecvBufferedSocket
from pants.util.strutil import ensure_binary


This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 6, 2019

Author Contributor

This was failing when running with ./pants2, and I'm not sure why, but it seems like a correct edit to make regardless.

logger = logging.getLogger(__name__)
@@ -62,7 +63,7 @@ def _write_flush(self, fd, payload=None):
"""Write a payload to a given fd (if provided) and flush the fd."""
try:
if payload:
fd.write(payload)
fd.write(ensure_binary(payload))
fd.flush()
except (IOError, OSError) as e:
# If a `Broken Pipe` is encountered during a stdio fd write, we're headless - bail.
@@ -16,7 +16,7 @@
from pants.option.optionable import Optionable
from pants.option.scope import ScopeInfo
from pants.subsystem.subsystem_client_mixin import SubsystemClientMixin
from pants.util.objects import datatype, enum, register_enum_option
from pants.util.objects import datatype, enum


class GlobMatchErrorBehavior(enum(['ignore', 'warn', 'error'])):
@@ -195,12 +195,10 @@ def register_bootstrap_options(cls, register):
help='Paths to ignore for all filesystem operations performed by pants '
'(e.g. BUILD file scanning, glob matching, etc). '
'Patterns use the gitignore syntax (https://git-scm.com/docs/gitignore).')
register_enum_option(
# TODO: allow using the attribute `GlobMatchErrorBehavior.warn` for more safety!
register, GlobMatchErrorBehavior, '--glob-expansion-failure', default='warn',
advanced=True,
help="Raise an exception if any targets declaring source files "
"fail to match any glob provided in the 'sources' argument.")
register('--glob-expansion-failure', advanced=True,
default=GlobMatchErrorBehavior.warn, type=GlobMatchErrorBehavior,
help="Raise an exception if any targets declaring source files "
"fail to match any glob provided in the 'sources' argument.")

register('--exclude-target-regexp', advanced=True, type=list, default=[], daemon=False,
metavar='<regexp>', help='Exclude target roots that match these regexes.')
@@ -8,7 +8,7 @@
import os
import re
import traceback
from builtins import next, object, open
from builtins import next, object, open, str
from collections import defaultdict

import six
@@ -442,7 +442,13 @@ def to_value_type(val_str):
elif kwargs.get('type') == bool:
return self._ensure_bool(val_str)
else:
return self._wrap_type(kwargs.get('type', str))(val_str)
type_arg = kwargs.get('type', str)
This conversation was marked as resolved by cosmicexplorer

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 2, 2019

Contributor

File needs a from builtins import str import now.

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 2, 2019

Author Contributor

Done!

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 3, 2019

Author Contributor

Done!

try:
return self._wrap_type(type_arg)(val_str)
except TypeError as e:
raise ParseError(
"Error applying type '{}' to option value '{}', for option '--{}' in {}: {}"
.format(type_arg.__name__, val_str, dest, self._scope_str(), e))

# Helper function to expand a fromfile=True value string, if needed.
def expand(val_str):
@@ -551,6 +557,13 @@ def record_option(value, rank, option_details=None):
def check(val):
if val is not None:
choices = kwargs.get('choices')
# If the `type` argument has an `all_variants` attribute, use that as `choices` if not
# already set. Using an attribute instead of checking a subclass allows `type` arguments
# which are functions to have an implicit fallback `choices` set as well.
if choices is None and 'type' in kwargs:
type_arg = kwargs.get('type')
if hasattr(type_arg, 'all_variants'):
choices = list(type_arg.all_variants)
if choices is not None and val not in choices:
raise ParseError('`{}` is not an allowed value for option {} in {}. '
'Must be one of: {}'.format(val, dest, self._scope_str(), choices))
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.