simplify install_tag handling according to the accepted API
There are two problems here: a typing problem, and an algorithm problem. We expect it to always be passed to CustomTarget() as a list, but we ran list() on it, which became horribly mangled if you violated the types and passed a string instead. This caused weird*er* errors and didn't even do anything. We want to do all validation in the interpreter, anyway, and make the build level dumb. Meanwhile we type it as accepting a T.Sequence, which technically permits... a string, actually. This isn't intentional; the point of using T.Sequence is out of a misguided idea that APIs are supposed to be "technically correct" by allowing "anything that fulfills an interface", which is a flawed concept because we aren't using interfaces here, and also because "technically string fulfills the same interface as a list, if we're talking sequences". Basically: - mypy is broken by design, because it typechecks "python", not "what we wish python to be" - we do not actually need to graciously permit passing tuples instead of lists As far as historic implementations of this logic go, we have formerly: - originally, typeslistified anything - switched to accepting list from the interpreter, redundantly ran list() on the list we got, and mishandling API violations passing a string (commit11f9638035
) - switched to accepting anything, stringlistifying it if it was not `None`, mishandling `[None]`, and invoking list(x) on a brand new list from stringlistify (commit157d438835
) - stopped stringlistify, just accept T.List[str | None] and re-cast to list, violates typing because we use/handle plain None too (commita8521fef70
) - break typing by declaring we accept a simple string, which still results in mishandling by converting 'foo' -> ['f', 'o', 'o'] (commitac576530c4
) All of this. ALL of it. Is because we tried to be fancy and say we accept T.Tuple; the only version of this logic that has ever worked correctly is the original untyped do-all-validation-in-the-build-phase typeslistified version. Let's just call it what it is. We want a list | None, and we handle it too.
This commit is contained in:
parent
248c1d9bd5
commit
cbf4496434
|
@ -117,15 +117,15 @@ known_shmod_kwargs = known_build_target_kwargs | {'vs_module_defs'}
|
|||
known_stlib_kwargs = known_build_target_kwargs | {'pic', 'prelink'}
|
||||
known_jar_kwargs = known_exe_kwargs | {'main_class', 'java_resources'}
|
||||
|
||||
def _process_install_tag(install_tag: T.Optional[T.Sequence[T.Optional[str]]],
|
||||
def _process_install_tag(install_tag: T.Optional[T.List[T.Optional[str]]],
|
||||
num_outputs: int) -> T.List[T.Optional[str]]:
|
||||
_install_tag: T.List[T.Optional[str]]
|
||||
if not install_tag:
|
||||
_install_tag = [None] * num_outputs
|
||||
elif len(install_tag) == 1:
|
||||
_install_tag = list(install_tag) * num_outputs
|
||||
_install_tag = install_tag * num_outputs
|
||||
else:
|
||||
_install_tag = list(install_tag)
|
||||
_install_tag = install_tag
|
||||
return _install_tag
|
||||
|
||||
|
||||
|
@ -2418,7 +2418,7 @@ class CustomTarget(Target, CommandBase):
|
|||
install: bool = False,
|
||||
install_dir: T.Optional[T.Sequence[T.Union[str, Literal[False]]]] = None,
|
||||
install_mode: T.Optional[FileMode] = None,
|
||||
install_tag: T.Optional[T.Sequence[T.Optional[str]]] = None,
|
||||
install_tag: T.Optional[T.List[T.Optional[str]]] = None,
|
||||
absolute_paths: bool = False,
|
||||
backend: T.Optional['Backend'] = None,
|
||||
):
|
||||
|
|
|
@ -304,7 +304,7 @@ class FSModule(ExtensionModule):
|
|||
install=kwargs['install'],
|
||||
install_dir=kwargs['install_dir'],
|
||||
install_mode=kwargs['install_mode'],
|
||||
install_tag=kwargs['install_tag'],
|
||||
install_tag=[kwargs['install_tag']],
|
||||
backend=state.backend,
|
||||
)
|
||||
|
||||
|
|
Loading…
Reference in New Issue