Add FrozenDict as an immutable wrapper around native dictionaries

[Eric-Arellano]

Problem

We have no ergonomic way to work with dictionaries with the engine, as dict is mutable and not hashable. We've resorted to using custom constructors in dataclasses that allow the user to pass a Dict, and then, in the constructor, converting it to a Tuple[Tuple[K, V], ...], which is still clunky to use.

pants/src/python/pants/engine/isolated_process.py

Lines 24 to 62 in eeb9d74

	@frozen_after_init
	@dataclass(unsafe_hash=True)
	class ExecuteProcessRequest:
	"""Request for execution with args and snapshots to extract."""
	# TODO: add a method to hack together a `process_executor` invocation command line which
	# reproduces this process execution request to make debugging remote executions effortless!
	argv: Tuple[str, ...]
	input_files: Digest
	description: str
	working_directory: Optional[str]
	env: Tuple[str, ...]
	output_files: Tuple[str, ...]
	output_directories: Tuple[str, ...]
	timeout_seconds: Union[int, float]
	unsafe_local_only_files_because_we_favor_speed_over_correctness_for_this_rule: Digest
	jdk_home: Optional[str]
	is_nailgunnable: bool
	def __init__(
	self,
	argv: Tuple[str, ...],
	*,
	input_files: Digest,
	description: str,
	working_directory: Optional[str] = None,
	env: Optional[Dict[str, str]] = None,
	output_files: Optional[Tuple[str, ...]] = None,
	output_directories: Optional[Tuple[str, ...]] = None,
	timeout_seconds: Union[int, float] = _default_timeout_seconds,
	unsafe_local_only_files_because_we_favor_speed_over_correctness_for_this_rule: Digest = EMPTY_DIRECTORY_DIGEST,
	jdk_home: Optional[str] = None,
	is_nailgunnable: bool = False,
	) -> None:
	self.argv = argv
	self.input_files = input_files
	self.description = description
	self.working_directory = working_directory
	self.env = tuple(itertools.chain.from_iterable((env or {}).items()))

Solution

Add FrozenDict as a very lightweight wrapper around native dict, with the only differences that it removes all entry points for mutation (e.g. d.update()) and it implements __hash__().

Rejected alternative: proactive casting of unhashable elements

Toolchain has a FrozenDict implementation that proactively attempts to convert hashable elements to hashable, and has additional guarantees to ensure the dict truly is frozen. For example, it will convert {'a': [1]} to {'a': (1,)}.

Instead, we take a much simpler approach to validate and give a nice error message when using unhashable elements, so that the rule author may fix their code. This is simpler, more performant, and more transparent.

Does not use typing.Hashable

This is a way in MyPy to express that something must must be hashable. However, it does not work as well as expected, e.g. complaining about typing.Type, such as str, not being hashable, even though hash(str) works.

Result

In a followup, we will audit the current engine code to see where we can clean things up by using FrozenDict.

[jsirois]

Well, you can perfectly well have a mutable object that's hashable - the mutable part just shouldn't (but could be :/) part of the hash. It might be best to just drop immutable here - you're not guaranteeing that.