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

Fix nightly cron ctypes enum failure #7249

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/python/pants/util/objects.py
Expand Up @@ -132,13 +132,16 @@ def __eq__(self, other):
def __ne__(self, other):
return not (self == other)

# NB: in Python 3, whenever __eq__ is overridden, __hash__() must also be
# explicitly implemented, otherwise Python will raise "unhashable type". See
# https://docs.python.org/3/reference/datamodel.html#object.__hash__.
def __hash__(self):
return super(DataType, self).__hash__()

# NB: As datatype is not iterable, we need to override both __iter__ and all of the
# namedtuple methods that expect self to be iterable.
def __iter__(self):
raise TypeError("'{}' object is not iterable".format(type(self).__name__))
raise self.make_type_error("datatype object is not iterable")

def _super_iter(self):
return super(DataType, self).__iter__()
Expand Down Expand Up @@ -281,6 +284,20 @@ def __new__(cls, value):
"""
return cls.create(value)

# TODO: figure out if this will always trigger on primitives like strings, and what situations
# won't call this __eq__ (and therefore won't raise like we want).
def __eq__(self, other):
"""Redefine equality to raise to nudge people to use static pattern matching."""
raise self.make_type_error(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather only raise if the other isn't of the same type. I think it's totally valid to write toolchain_variant != ToolchainVariant.gnu, and that that is cleaner than the nested match you have in the ctypes test.

"enum equality is defined to be an error -- use .resolve_for_enum_variant() instead!")
# Redefine the canary so datatype __new__ doesn't raise.
__eq__._eq_override_canary = None

# NB: as noted in datatype(), __hash__ must be explicitly implemented whenever __eq__ is
# overridden. See https://docs.python.org/3/reference/datamodel.html#object.__hash__.
def __hash__(self):
return super(ChoiceDatatype, self).__hash__()

@classmethod
def create(cls, *args, **kwargs):
"""Create an instance of this enum, using the default value if specified.
Expand Down
Expand Up @@ -183,7 +183,10 @@ def test_ctypes_third_party_integration(self, toolchain_variant):
# TODO(#6848): this fails when run with gcc on osx as it requires gcc's libstdc++.so.6.dylib to
# be available on the runtime library path.
attempt_pants_run = Platform.create().resolve_for_enum_variant({
'darwin': toolchain_variant != 'gnu',
'darwin': toolchain_variant.resolve_for_enum_variant({
'gnu': False,
'llvm': True,
}),
'linux': True,
})
if attempt_pants_run:
Expand Down
20 changes: 20 additions & 0 deletions tests/python/pants_test/util/test_objects.py
Expand Up @@ -741,6 +741,26 @@ def test_enum_instance_creation_errors(self):
with self.assertRaisesRegexp(EnumVariantSelectionError, expected_rx_falsy_value):
SomeEnum('')

def test_enum_comparison_fails(self):
enum_instance = SomeEnum(1)
rx_str = re.escape("enum equality is defined to be an error")
with self.assertRaisesRegexp(TypeCheckError, rx_str):
enum_instance == enum_instance
with self.assertRaisesRegexp(TypeCheckError, rx_str):
enum_instance != enum_instance
# Test that comparison also fails against another type.
with self.assertRaisesRegexp(TypeCheckError, rx_str):
enum_instance == 1
with self.assertRaisesRegexp(TypeCheckError, rx_str):
1 == enum_instance

class StrEnum(enum(['a'])): pass
enum_instance = StrEnum('a')
with self.assertRaisesRegexp(TypeCheckError, rx_str):
enum_instance == 'a'
with self.assertRaisesRegexp(TypeCheckError, rx_str):
'a' == enum_instance

def test_enum_resolve_variant(self):
one_enum_instance = SomeEnum(1)
two_enum_instance = SomeEnum(2)
Expand Down