Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230316220638.983743-1-dlatypov@google.com> <20230430181500.122664-1-sj@kernel.org>
In-Reply-To: <20230430181500.122664-1-sj@kernel.org>
From:   Daniel Latypov <dlatypov@google.com>
Date:   Sun, 30 Apr 2023 14:34:09 -0700
Message-ID: <CAGS_qxqmWYzTBkfmFoALoteaG303tJNh1K5N2=Pmykqk+2BeTg@mail.gmail.com>
Subject: Re: [PATCH v2 1/3] kunit: tool: add subscripts for type annotations
 where appropriate
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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, Apr 30, 2023 at 11:15=E2=80=AFAM SeongJae Park <sj@kernel.org> wrot=
e:
>
> Hi Daniel,
>
> On Thu, 16 Mar 2023 15:06:36 -0700 Daniel Latypov <dlatypov@google.com> w=
rote:
>
> > E.g. for subprocess.Popen, it can be opened in `text=3DTrue` mode where=
 it
> > returns strings, or `text=3DFalse` 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 kun=
it
> 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 currentl=
y).

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.p=
y.
Currently, it runs `mypy --strict` which will start failing if we
revert any part of this patch.

Thanks,
Daniel