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

Drop old deprecated options / conditionals #7332

Merged
merged 7 commits into from Mar 13, 2019
@@ -9,7 +9,6 @@
import os
import shutil

from pants.base.deprecated import deprecated_conditional
from pants.base.exceptions import TaskError
from pants.binaries.binary_tool import NativeTool
from pants.option.custom_types import dir_option, file_option
@@ -83,25 +82,6 @@ def get_package_manager(self, package_manager=None):
))
return package_manager_obj

@memoized_method
def version(self, context=None):
# The versions reported by node and embedded in distribution package names are 'vX.Y.Z'.
# TODO: After the deprecation cycle is over we'll expect the values of the version option
# to already include the 'v' prefix, so there will be no need to normalize, and we can
# delete this entire method override.
version = super(NodeDistribution, self).version(context)
deprecated_conditional(
lambda: not version.startswith('v'), entity_description='', removal_version='1.7.0.dev0',
hint_message='value of --version in scope {} must be of the form '
'vX.Y.Z'.format(self.options_scope))
return version if version.startswith('v') else 'v' + version

@classmethod
def _normalize_version(cls, version):

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 7, 2019

Author Contributor

This was removed from the __init__ here, AFAICT it's not used elsewhere either so opted to remove it as well.

Update - Some more context, from what I can tell the last use of it was in def yarnpkg_version, removed here.

# The versions reported by node and embedded in distribution package names are 'vX.Y.Z' and not
# 'X.Y.Z'.
return version if version.startswith('v') else 'v' + version

@memoized_property
def eslint_setupdir(self):
return self.get_options().eslint_setupdir
@@ -9,7 +9,7 @@
from builtins import object, open, str
from multiprocessing import cpu_count

from future.utils import string_types, text_type
from future.utils import text_type
from twitter.common.collections import OrderedSet

from pants.backend.jvm.subsystems.dependency_context import DependencyContext
@@ -30,7 +30,6 @@
from pants.backend.jvm.tasks.jvm_dependency_analyzer import JvmDependencyAnalyzer
from pants.backend.jvm.tasks.nailgun_task import NailgunTaskBase
from pants.base.build_environment import get_buildroot
from pants.base.deprecated import deprecated_conditional
from pants.base.exceptions import TaskError
from pants.base.worker_pool import WorkerPool
from pants.base.workunit import WorkUnitLabel
@@ -810,14 +809,6 @@ def _extra_compile_time_classpath(self):
def extra_compile_classpath_iter():
for conf in self._confs:
for jar in self.extra_compile_time_classpath_elements():
if isinstance(jar, string_types):
# Backwards compatibility
deprecated_conditional(
lambda: True,
"1.12.0.dev0",
"Extra compile classpath auto-promotion from string to ClasspathEntry",
)
jar = ClasspathEntry(jar)
yield (conf, jar)

return list(extra_compile_classpath_iter())
@@ -66,22 +66,6 @@ def register_options(cls, register):
'"<PATH>" (the contents of the PATH env var), '
'"<PEXRC>" (paths in the PEX_PYTHON_PATH variable in a pexrc file), '
'"<PYENV>" (all python versions under $(pyenv root)/versions).')
register('--resolver-blacklist', advanced=True, type=dict, default={},
removal_version='1.13.0.dev2',
removal_hint='Now unused. Pants, via PEX, handles blacklisting automatically via '
'PEP-508 environment markers anywhere Python requirements are specified '
'(e.g. `requirements.txt` and `python_requirement(...)` in BUILD files): '
'https://www.python.org/dev/peps/pep-0508/#environment-markers',
metavar='<blacklist>',
help='A blacklist dict (str->str) that maps package name to an interpreter '
'constraint. If a package name is in the blacklist and its interpreter '
'constraint matches the target interpreter, skip the requirement. This is needed '
'to ensure that universal requirement resolves for a target interpreter version do '
'not error out on interpreter specific requirements such as backport libs like '
'`functools32`. For example, a valid blacklist is {"functools32": "CPython>3"}. '
'NOTE: this keyword is a temporary fix and will be reverted per: '
'https://github.com/pantsbuild/pants/issues/5696. The long term '
'solution is tracked by: https://github.com/pantsbuild/pex/issues/456.')
register('--resolver-use-manylinux', advanced=True, type=bool, default=True, fingerprint=True,
help='Whether to consider manylinux wheels when resolving requirements for linux '
'platforms.')
@@ -12,7 +12,6 @@
import six

from pants.base.build_file_target_factory import BuildFileTargetFactory
from pants.base.deprecated import deprecated_conditional
from pants.build_graph.target import Target
from pants.util.memo import memoized_property

@@ -39,15 +38,8 @@ def wrap(cls, context_aware_object_factory, *target_types):
:rtype: :class:`TargetMacro.Factory`
"""
if not target_types:
raise ValueError('The given `context_aware_object_factory` {} must expand at least 1 '
'produced type; none were registered'.format(context_aware_object_factory))
deprecated_conditional(
lambda: len(target_types) > 1,
'1.12.0.dev0',
'TargetMacro.Factory instances that construct more than one type are no longer supported. '
'Consider using a `context_aware_object_factory, which can construct any number of '
'different objects.'
)
raise ValueError('The given `context_aware_object_factory` {} must expand 1 produced '
'type; none were registered'.format(context_aware_object_factory))

class Factory(cls):
@property
@@ -13,7 +13,6 @@
from twitter.common.collections import OrderedSet

from pants.base.build_environment import get_buildroot
from pants.base.deprecated import deprecated_conditional
from pants.base.exceptions import TargetDefinitionException
from pants.base.fingerprint_strategy import DefaultFingerprintStrategy
from pants.base.hash_utils import hash_all
@@ -26,7 +25,7 @@
from pants.build_graph.target_scopes import Scope
from pants.fs.fs import safe_filename
from pants.source.payload_fields import SourcesField
from pants.source.wrapped_globs import EagerFilesetWithSpec, FilesetWithSpec
from pants.source.wrapped_globs import FilesetWithSpec
from pants.subsystem.subsystem import Subsystem
from pants.util.memo import memoized_method, memoized_property

@@ -113,17 +112,11 @@ def check_unknown(self, target, kwargs, payload):
"""
ignore_params = set((self.get_options().ignored or {}).get(target.type_alias, ()))
unknown_args = {arg: value for arg, value in kwargs.items() if arg not in ignore_params}
if 'source' in unknown_args and 'sources' in payload.as_dict():
unknown_args.pop('source')
if 'sources' in unknown_args:
if 'sources' in payload.as_dict():
deprecated_conditional(
lambda: True,
'1.11.0.dev0',
('The source argument is deprecated - it gets automatically promoted to sources.'
'Target {} should just use a sources argument. No BUILD files need changing. '
'The source argument will stop being populated -').format(target.type_alias),
)
# TODO(#7357) Remove these checks once test issues are resolved.
if 'sources' in payload.as_dict():

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 8, 2019

Author Contributor

I noticed this was last touched in 450de70 - it feels like we should be able to drop these checks / pops and allow an error to be raised if they're included (or move them to ignored_args?), @illicitonion @cosmicexplorer might ya'll know on if this can be removed, moved to ignored_args, or should be left as-is?

This comment has been minimized.

Copy link
@illicitonion

illicitonion Mar 8, 2019

Contributor

Yeah, I think you should be able to remove this entire block, and just treat sources like any other arg :) Good spot!

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 11, 2019

Author Contributor

Getting rid of this caused quite a few test failures, most (maybe all? from what I've spot checked so far at least) of which are from having source= within BUILD files across pants. Would it be safe to assume those should be updated to sources=[...] accordingly, or might it be preferable to revert c09f9cb for now?

This comment has been minimized.

Copy link
@illicitonion

illicitonion Mar 11, 2019

Contributor

Oh dear - please revert that change and file a ticket assigned to me :) Sorry about that!

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 11, 2019

Author Contributor

No worries! Will revert and get a ticket open for that.

Edit: #7357

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 11, 2019

Contributor

Consider adding a TODO comment linking to 7357 here.

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 11, 2019

Author Contributor
if 'source' in unknown_args:
unknown_args.pop('source')
if 'sources' in unknown_args:
unknown_args.pop('sources')
kwargs.pop('sources')
ignored_args = {arg: value for arg, value in kwargs.items() if arg in ignore_params}
@@ -874,13 +867,5 @@ def create_sources_field(self, sources, sources_rel_path, key_arg=None):
key_arg_section = "'{}' to be ".format(key_arg) if key_arg else ""
raise TargetDefinitionException(self, "Expected {}a glob, an address or a list, but was {}"
.format(key_arg_section, type(sources)))
elif not isinstance(sources, EagerFilesetWithSpec):
deprecated_conditional(
lambda: True,
'1.12.0.dev0',
('FilesetWithSpec sources values are deprecated except for EagerFilesetWithSpec values. '
'Saw value of type {}').format(type(sources))
)


return SourcesField(sources=sources)
@@ -254,10 +254,6 @@ def register_bootstrap_options(cls, register):
help='The port to bind the pants nailgun server to. Defaults to a random port.')
register('--pantsd-log-dir', advanced=True, default=None,
help='The directory to log pantsd output to.')
register('--pantsd-fs-event-workers', advanced=True, type=int, default=4,
removal_version='1.14.0.dev2',
removal_hint='Filesystem events are now handled by a single dedicated thread.',
help='The number of workers to use for the filesystem event service executor pool.')
register('--pantsd-invalidation-globs', advanced=True, type=list, fromfile=True, default=[],
help='Filesystem events matching any of these globs will trigger a daemon restart.')

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.