Hi Daniel, On Sun, 30 Apr 2023 14:34:09 -0700 Daniel Latypov <dlatypov@xxxxxxxxxx> wrote: > On Sun, Apr 30, 2023 at 11:15 AM SeongJae Park <sj@xxxxxxxxxx> wrote: > > > > Hi Daniel, > > > > On Thu, 16 Mar 2023 15:06:36 -0700 Daniel Latypov <dlatypov@xxxxxxxxxx> 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). Thank you for quick reply. Reverting Popen changes only as below fixed my issue. So seems typing.IO and concurrent.Future[None] chages doesn't cause problems at least for my use case. diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index f01f94106129..7f648802caf6 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -92,7 +92,7 @@ class LinuxSourceTreeOperations: if stderr: # likely only due to build warnings print(stderr.decode()) - def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]: + def start(self, params: List[str], build_dir: str) -> subprocess.Popen: raise RuntimeError('not implemented!') @@ -113,7 +113,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): kconfig.merge_in_entries(base_kunitconfig) return kconfig - def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]: + def start(self, params: List[str], build_dir: str) -> subprocess.Popen: kernel_path = os.path.join(build_dir, self._kernel_path) qemu_command = ['qemu-system-' + self._qemu_arch, '-nodefaults', @@ -142,7 +142,7 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations): kconfig.merge_in_entries(base_kunitconfig) return kconfig - def start(self, params: List[str], build_dir: str) -> subprocess.Popen[str]: + def start(self, params: List[str], build_dir: str) -> subprocess.Popen: """Runs the Linux UML binary. Must be named 'linux'.""" linux_bin = os.path.join(build_dir, 'linux') params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt']) > > 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. Those make sense. Thanks, SJ > > Thanks, > Daniel
