From cbf4496434ef78f695c2ceca4e9e093ea14ab784 Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Thu, 9 Jun 2022 20:42:50 -0400 Subject: 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 (commit 11f96380351a88059ec55f1070fdebc1b1033117) - switched to accepting anything, stringlistifying it if it was not `None`, mishandling `[None]`, and invoking list(x) on a brand new list from stringlistify (commit 157d43883515507f42618b065a64fb26501734a0) - stopped stringlistify, just accept T.List[str | None] and re-cast to list, violates typing because we use/handle plain None too (commit a8521fef70ef76fb742d80aceb2e9ed634bd6a70) - break typing by declaring we accept a simple string, which still results in mishandling by converting 'foo' -> ['f', 'o', 'o'] (commit ac576530c43734495815f22456596772a8f6a8cc) 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. --- mesonbuild/modules/fs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'mesonbuild/modules/fs.py') diff --git a/mesonbuild/modules/fs.py b/mesonbuild/modules/fs.py index 7e3c75a11..eb19ec4ff 100644 --- a/mesonbuild/modules/fs.py +++ b/mesonbuild/modules/fs.py @@ -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, ) -- cgit v1.2.3