Re: [PATCH v2 1/3] kunit: tool: add subscripts for type annotations where appropriate

From: Daniel Latypov <dlatypov@google.com>
To: SeongJae Park <sj@kernel.org>
Cc: brendanhiggins@google.com, davidgow@google.com, rmoar@google.com,
	linux-kernel@vger.kernel.org, kunit-dev@googlegroups.com,
	linux-kselftest@vger.kernel.org, skhan@linuxfoundation.org,
	johannes@sipsolutions.net,
	Johannes Berg <johannes.berg@intel.com>,
	regressions@lists.linux.dev
Subject: Re: [PATCH v2 1/3] kunit: tool: add subscripts for type annotations where appropriate
Date: Sun, 30 Apr 2023 14:34:09 -0700	[thread overview]
Message-ID: <CAGS_qxqmWYzTBkfmFoALoteaG303tJNh1K5N2=Pmykqk+2BeTg@mail.gmail.com> (raw)
In-Reply-To: <20230430181500.122664-1-sj@kernel.org>

On Sun, Apr 30, 2023 at 11:15 AM SeongJae Park <sj@kernel.org> wrote:
>
> Hi Daniel,
>
> On Thu, 16 Mar 2023 15:06:36 -0700 Daniel Latypov <dlatypov@google.com> wrote:
>
> > E.g. for subprocess.Popen, it can be opened in `text=True` mode where it
> > returns strings, or `text=False` where it returns bytes.
> > To differentiate, you can annotate types as `Popen[str]` or
> > `Popen[bytes]`.
> >
> > This patch should add subscripts in all the places we were missing them.
>
> I just found this patch is in the latest mainline tree, and it causes kunit
> failure on my test machine like below.
>
>     $ python3 --version
>     Python 3.8.10
>     $
>     $ ./tools/testing/kunit/kunit.py run --build_dir ../kunit.out/
>     Traceback (most recent call last):
>       File "./tools/testing/kunit/kunit.py", line 24, in <module>
>         import kunit_kernel
>       File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 42, in <module>
>         class LinuxSourceTreeOperations:
>       File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 95, in LinuxSourceTreeOperations
>         def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]:
>     TypeError: 'type' object is not subscriptable
>     $
>
> I further confirmed reverting this patch makes it run again.  Do you have any
> idea?

It seems like support for the subscript wasn't added until Python 3.9.

I know support for subscripting other types like re.Pattern was added
in 3.9 per https://peps.python.org/pep-0585/ but it doesn't mention
Popen there...
This patch also added typing.IO[str] and concurrent.Future[None], so
those might be problematic too.

Can you check if the typing.IO and concurrent.Future[None] changes
cause problems?
(I don't have an easy way of testing against older Python versions currently).

If so, we should revert the patch.
If not, we can undo just the Popen changes.

And in either case, we'll need to update ./tools/testing/kunit/run_checks.py.
Currently, it runs `mypy --strict` which will start failing if we
revert any part of this patch.

Thanks,
Daniel