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 4 commits
Commits
File filter

Filter by extension

Filter by extension

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

# NB: it appears that in Python 3, whenever __eq__ is overridden, __hash__() must also be
# explicitly implemented, otherwise Python will raise "unhashable type". It's not clear whether
# or where this behavior is documented.
Eric-Arellano marked this conversation as resolved.
Show resolved Hide resolved
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 in Python 3. This behavior appears to be undocumented.
Eric-Arellano marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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