linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kunit: tool: Assert version requirement
@ 2021-06-16  9:40 SeongJae Park
  2021-06-16 21:14 ` Daniel Latypov
  0 siblings, 1 reply; 12+ messages in thread
From: SeongJae Park @ 2021-06-16  9:40 UTC (permalink / raw)
  To: brendanhiggins; +Cc: linux-kselftest, kunit-dev, linux-kernel, SeongJae Park

From: SeongJae Park <sjpark@amazon.de>

Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
Because it is supported on only >=3.7 Python, people using older Python
will get below error:

    Traceback (most recent call last):
      File "./tools/testing/kunit/kunit.py", line 20, in <module>
        import kunit_kernel
      File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
        from __future__ import annotations
        ^
    SyntaxError: future feature annotations is not defined

This commit adds a version assertion in 'kunit.py', so that people get
more explicit error message like below:

   Traceback (most recent call last):
      File "./tools/testing/kunit/kunit.py", line 15, in <module>
        assert sys.version_info >= (3, 7)
    AssertionError

Signed-off-by: SeongJae Park <sjpark@amazon.de>
---
 tools/testing/kunit/kunit.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index be8d8d4a4e08..748d88178506 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -12,6 +12,8 @@ import sys
 import os
 import time
 
+assert sys.version_info >= (3, 7)
+
 from collections import namedtuple
 from enum import Enum, auto
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] kunit: tool: Assert version requirement
  2021-06-16  9:40 [PATCH] kunit: tool: Assert version requirement SeongJae Park
@ 2021-06-16 21:14 ` Daniel Latypov
  2021-06-17  7:39   ` SeongJae Park
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Latypov @ 2021-06-16 21:14 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Brendan Higgins, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, Linux Kernel Mailing List, SeongJae Park

On Wed, Jun 16, 2021 at 2:40 AM SeongJae Park <sj38.park@gmail.com> wrote:
>
> From: SeongJae Park <sjpark@amazon.de>
>
> Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
> tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
> Because it is supported on only >=3.7 Python, people using older Python
> will get below error:

Ah, we had been fine with just using 3.6 before this (and could have
dropped down to 3.5 with a few lines changed).

But 3.7 came out initially in 2018*, so I assume we're probably fine
to rely on that in kunit tool.
*https://www.python.org/downloads/release/python-370/

>
>     Traceback (most recent call last):
>       File "./tools/testing/kunit/kunit.py", line 20, in <module>
>         import kunit_kernel
>       File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
>         from __future__ import annotations
>         ^
>     SyntaxError: future feature annotations is not defined
>
> This commit adds a version assertion in 'kunit.py', so that people get
> more explicit error message like below:
>
>    Traceback (most recent call last):
>       File "./tools/testing/kunit/kunit.py", line 15, in <module>
>         assert sys.version_info >= (3, 7)
>     AssertionError
>
> Signed-off-by: SeongJae Park <sjpark@amazon.de>

Acked-by: Daniel Latypov <dlatypov@google.com>

> ---
>  tools/testing/kunit/kunit.py | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index be8d8d4a4e08..748d88178506 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -12,6 +12,8 @@ import sys
>  import os
>  import time
>
> +assert sys.version_info >= (3, 7)

Do we perhaps want
  assert sys.version_info >= (3, 7), "Python version is too old"

Then the error message would be
  Traceback (most recent call last):
    File "./tools/testing/kunit/kunit.py", line 15, in <module>
      assert sys.version_info >= (3, 7), "Python version is too old"
  AssertionError: Python version is too old

I assume kernel devs know some Python, but not necessarily that
sys.version_info == "my python version"

> +
>  from collections import namedtuple
>  from enum import Enum, auto
>
> --
> 2.17.1
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20210616094033.18246-1-sj38.park%40gmail.com.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] kunit: tool: Assert version requirement
  2021-06-16 21:14 ` Daniel Latypov
@ 2021-06-17  7:39   ` SeongJae Park
  2021-06-17  7:46     ` [PATCH v2] kunit: tool: Assert the " SeongJae Park
  0 siblings, 1 reply; 12+ messages in thread
From: SeongJae Park @ 2021-06-17  7:39 UTC (permalink / raw)
  To: Daniel Latypov
  Cc: SeongJae Park, Brendan Higgins,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, SeongJae Park

From: SeongJae Park <sjpark@amazon.de>

Hello Daniel,

On Wed, 16 Jun 2021 14:14:40 -0700 Daniel Latypov <dlatypov@google.com> wrote:

> On Wed, Jun 16, 2021 at 2:40 AM SeongJae Park <sj38.park@gmail.com> wrote:
> >
> > From: SeongJae Park <sjpark@amazon.de>
> >
> > Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
> > tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
> > Because it is supported on only >=3.7 Python, people using older Python
> > will get below error:
> 
> Ah, we had been fine with just using 3.6 before this (and could have
> dropped down to 3.5 with a few lines changed).
> 
> But 3.7 came out initially in 2018*, so I assume we're probably fine
> to rely on that in kunit tool.
> *https://www.python.org/downloads/release/python-370/

Agreed.

> 
> >
> >     Traceback (most recent call last):
> >       File "./tools/testing/kunit/kunit.py", line 20, in <module>
> >         import kunit_kernel
> >       File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
> >         from __future__ import annotations
> >         ^
> >     SyntaxError: future feature annotations is not defined
> >
> > This commit adds a version assertion in 'kunit.py', so that people get
> > more explicit error message like below:
> >
> >    Traceback (most recent call last):
> >       File "./tools/testing/kunit/kunit.py", line 15, in <module>
> >         assert sys.version_info >= (3, 7)
> >     AssertionError
> >
> > Signed-off-by: SeongJae Park <sjpark@amazon.de>
> 
> Acked-by: Daniel Latypov <dlatypov@google.com>

Thank you! :)

> 
> > ---
> >  tools/testing/kunit/kunit.py | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> > index be8d8d4a4e08..748d88178506 100755
> > --- a/tools/testing/kunit/kunit.py
> > +++ b/tools/testing/kunit/kunit.py
> > @@ -12,6 +12,8 @@ import sys
> >  import os
> >  import time
> >
> > +assert sys.version_info >= (3, 7)
> 
> Do we perhaps want
>   assert sys.version_info >= (3, 7), "Python version is too old"
> 
> Then the error message would be
>   Traceback (most recent call last):
>     File "./tools/testing/kunit/kunit.py", line 15, in <module>
>       assert sys.version_info >= (3, 7), "Python version is too old"
>   AssertionError: Python version is too old

That looks easier to understand.  I will post v2 in reply to this.


Thanks,
SeongJae Park

> 
> I assume kernel devs know some Python, but not necessarily that
> sys.version_info == "my python version"
> 
> > +
> >  from collections import namedtuple
> >  from enum import Enum, auto
> >
> > --
> > 2.17.1
> >
> > --
> > You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20210616094033.18246-1-sj38.park%40gmail.com.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] kunit: tool: Assert the version requirement
  2021-06-17  7:39   ` SeongJae Park
@ 2021-06-17  7:46     ` SeongJae Park
  2021-06-22 23:28       ` Daniel Latypov
  2021-06-28 19:41       ` Brendan Higgins
  0 siblings, 2 replies; 12+ messages in thread
From: SeongJae Park @ 2021-06-17  7:46 UTC (permalink / raw)
  To: brendanhiggins
  Cc: dlatypov, linux-kselftest, kunit-dev, linux-kernel, SeongJae Park

Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
Because it is supported on only >=3.7 Python, people using older Python
will get below error:

    Traceback (most recent call last):
      File "./tools/testing/kunit/kunit.py", line 20, in <module>
        import kunit_kernel
      File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
        from __future__ import annotations
        ^
    SyntaxError: future feature annotations is not defined

This commit adds a version assertion in 'kunit.py', so that people get
more explicit error message like below:

    Traceback (most recent call last):
      File "./tools/testing/kunit/kunit.py", line 15, in <module>
        assert sys.version_info >= (3, 7), "Python version is too old"
    AssertionError: Python version is too old

Signed-off-by: SeongJae Park <sjpark@amazon.de>
Acked-by: Daniel Latypov <dlatypov@google.com>
---

Changes from v1
- Add assertion failure message (Daniel Latypov)
- Add Acked-by: Daniel Latypov <dlatypov@google.com>

 tools/testing/kunit/kunit.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index be8d8d4a4e08..6276ce0c0196 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -12,6 +12,8 @@ import sys
 import os
 import time
 
+assert sys.version_info >= (3, 7), "Python version is too old"
+
 from collections import namedtuple
 from enum import Enum, auto
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kunit: tool: Assert the version requirement
  2021-06-17  7:46     ` [PATCH v2] kunit: tool: Assert the " SeongJae Park
@ 2021-06-22 23:28       ` Daniel Latypov
  2021-06-22 23:55         ` Daniel Latypov
  2021-06-28 19:41       ` Brendan Higgins
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel Latypov @ 2021-06-22 23:28 UTC (permalink / raw)
  To: SeongJae Park
  Cc: brendanhiggins, linux-kselftest, kunit-dev, linux-kernel, SeongJae Park

On Thu, Jun 17, 2021 at 12:46 AM SeongJae Park <sj38.park@gmail.com> wrote:
>
> Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
> tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
> Because it is supported on only >=3.7 Python, people using older Python
> will get below error:
>
>     Traceback (most recent call last):
>       File "./tools/testing/kunit/kunit.py", line 20, in <module>
>         import kunit_kernel
>       File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
>         from __future__ import annotations

Chatted offline with David about this.
He was thinking if we could instead drop the minimal version back to 3.6.

I think we can do so, see below.
Perhaps we should drop the import and then chain this patch on top of
that, specifying a minimum version of 3.6?

Checking out https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/?h=kunit-fixes

The offending "annotations" import is related to type annotations.
Specifically https://www.python.org/dev/peps/pep-0563/

So let's see how the two most popular typecheckers fare.

pytype is happy with or without import.
mypy has the same issues with or without the import.

$ mypy tools/testing/kunit/*.py
tools/testing/kunit/kunit_kernel.py:227: error: Item "_Loader" of
"Optional[_Loader]" has no attribute "exec_module"
tools/testing/kunit/kunit_kernel.py:227: error: Item "None" of
"Optional[_Loader]" has no attribute "exec_module"
tools/testing/kunit/kunit_kernel.py:228: error: Module has no
attribute "QEMU_ARCH"
tools/testing/kunit/kunit_kernel.py:229: error: Module has no
attribute "QEMU_ARCH"

So clearly it's not doing anything for them.

Taking a look over 87c9c1631788 ("kunit: tool: add support for QEMU")
next then...
I don't see anything that would warrant the import, so we should
probably drop it.

In that case, the minimum supported version should drop back down to 3.6.
We use enum.auto, which is from 3.6
https://docs.python.org/3/library/enum.html#enum.auto

We could consider stopping using that, and I think we might be then
3.5-compatible.
Maybe we have a chain of 3 patches then, drop the import, drop auto,
and then add in a >=3.5 version check?

>         ^
>     SyntaxError: future feature annotations is not defined
>
> This commit adds a version assertion in 'kunit.py', so that people get
> more explicit error message like below:
>
>     Traceback (most recent call last):
>       File "./tools/testing/kunit/kunit.py", line 15, in <module>
>         assert sys.version_info >= (3, 7), "Python version is too old"
>     AssertionError: Python version is too old
>
> Signed-off-by: SeongJae Park <sjpark@amazon.de>
> Acked-by: Daniel Latypov <dlatypov@google.com>
> ---
>
> Changes from v1
> - Add assertion failure message (Daniel Latypov)
> - Add Acked-by: Daniel Latypov <dlatypov@google.com>
>
>  tools/testing/kunit/kunit.py | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index be8d8d4a4e08..6276ce0c0196 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -12,6 +12,8 @@ import sys
>  import os
>  import time
>
> +assert sys.version_info >= (3, 7), "Python version is too old"
> +
>  from collections import namedtuple
>  from enum import Enum, auto
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kunit: tool: Assert the version requirement
  2021-06-22 23:28       ` Daniel Latypov
@ 2021-06-22 23:55         ` Daniel Latypov
  2021-06-28 13:37           ` SeongJae Park
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Latypov @ 2021-06-22 23:55 UTC (permalink / raw)
  To: SeongJae Park
  Cc: brendanhiggins, linux-kselftest, kunit-dev, linux-kernel, SeongJae Park

On Tue, Jun 22, 2021 at 4:28 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> On Thu, Jun 17, 2021 at 12:46 AM SeongJae Park <sj38.park@gmail.com> wrote:
> >
> > Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
> > tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
> > Because it is supported on only >=3.7 Python, people using older Python
> > will get below error:
> >
> >     Traceback (most recent call last):
> >       File "./tools/testing/kunit/kunit.py", line 20, in <module>
> >         import kunit_kernel
> >       File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
> >         from __future__ import annotations
>
> Chatted offline with David about this.
> He was thinking if we could instead drop the minimal version back to 3.6.
>
> I think we can do so, see below.
> Perhaps we should drop the import and then chain this patch on top of
> that, specifying a minimum version of 3.6?

Actually, now I've gotten python3.6 installed on my machine, I see we
have another issue.

We pass text=true to subprocess.
That didn't exist back in 3.6, see
https://docs.python.org/3.6/library/subprocess.html

We can workaround that, but there's more chance of subtle bugs that
I'd rather we don't touch it.

>
> Checking out https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/?h=kunit-fixes
>
> The offending "annotations" import is related to type annotations.
> Specifically https://www.python.org/dev/peps/pep-0563/
>
> So let's see how the two most popular typecheckers fare.
>
> pytype is happy with or without import.
> mypy has the same issues with or without the import.
>
> $ mypy tools/testing/kunit/*.py
> tools/testing/kunit/kunit_kernel.py:227: error: Item "_Loader" of
> "Optional[_Loader]" has no attribute "exec_module"
> tools/testing/kunit/kunit_kernel.py:227: error: Item "None" of
> "Optional[_Loader]" has no attribute "exec_module"
> tools/testing/kunit/kunit_kernel.py:228: error: Module has no
> attribute "QEMU_ARCH"
> tools/testing/kunit/kunit_kernel.py:229: error: Module has no
> attribute "QEMU_ARCH"
>
> So clearly it's not doing anything for them.
>
> Taking a look over 87c9c1631788 ("kunit: tool: add support for QEMU")
> next then...
> I don't see anything that would warrant the import, so we should
> probably drop it.

Also, using 3.6 now I have it installed, I found what it was added for.
But it doesn't need to be there.

This patch drops it and makes things work, afaict:
diff --git a/tools/testing/kunit/kunit_kernel.py
b/tools/testing/kunit/kunit_kernel.py
index e1951fa60027..5987d5b1b874 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -6,15 +6,13 @@
 # Author: Felix Guo <felixguoxiuping@gmail.com>
 # Author: Brendan Higgins <brendanhiggins@google.com>

-from __future__ import annotations
 import importlib.util
 import logging
 import subprocess
 import os
 import shutil
 import signal
-from typing import Iterator
-from typing import Optional
+from typing import Iterator, Optional, Tuple

 from contextlib import ExitStack

@@ -208,7 +206,7 @@ def get_source_tree_ops(arch: str, cross_compile:
Optional[str]) -> LinuxSourceT
                raise ConfigError(arch + ' is not a valid arch')

 def get_source_tree_ops_from_qemu_config(config_path: str,
-                                        cross_compile: Optional[str]) -> tuple[
+                                        cross_compile: Optional[str]) -> Tuple[
                                                         str,
LinuxSourceTreeOperations]:
        # The module name/path has very little to do with where the actual file
        # exists (I learned this through experimentation and could not find it

>
> In that case, the minimum supported version should drop back down to 3.6.
> We use enum.auto, which is from 3.6
> https://docs.python.org/3/library/enum.html#enum.auto
>
> We could consider stopping using that, and I think we might be then
> 3.5-compatible.
> Maybe we have a chain of 3 patches then, drop the import, drop auto,
> and then add in a >=3.5 version check?
>
> >         ^
> >     SyntaxError: future feature annotations is not defined
> >
> > This commit adds a version assertion in 'kunit.py', so that people get
> > more explicit error message like below:
> >
> >     Traceback (most recent call last):
> >       File "./tools/testing/kunit/kunit.py", line 15, in <module>
> >         assert sys.version_info >= (3, 7), "Python version is too old"
> >     AssertionError: Python version is too old
> >
> > Signed-off-by: SeongJae Park <sjpark@amazon.de>
> > Acked-by: Daniel Latypov <dlatypov@google.com>

Reviewed-by: Daniel Latypov <dlatypov@google.com>

As mentioned above, we do actually need 3.7, and not just for the extra import.
Now I know that, I feel more strongly that this patch should go in, as-is.

> > ---
> >
> > Changes from v1
> > - Add assertion failure message (Daniel Latypov)
> > - Add Acked-by: Daniel Latypov <dlatypov@google.com>
> >
> >  tools/testing/kunit/kunit.py | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> > index be8d8d4a4e08..6276ce0c0196 100755
> > --- a/tools/testing/kunit/kunit.py
> > +++ b/tools/testing/kunit/kunit.py
> > @@ -12,6 +12,8 @@ import sys
> >  import os
> >  import time
> >
> > +assert sys.version_info >= (3, 7), "Python version is too old"
> > +
> >  from collections import namedtuple
> >  from enum import Enum, auto
> >
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kunit: tool: Assert the version requirement
  2021-06-22 23:55         ` Daniel Latypov
@ 2021-06-28 13:37           ` SeongJae Park
  2021-06-28 19:51             ` Brendan Higgins
  0 siblings, 1 reply; 12+ messages in thread
From: SeongJae Park @ 2021-06-28 13:37 UTC (permalink / raw)
  Cc: SeongJae Park, brendanhiggins, linux-kselftest, kunit-dev,
	linux-kernel, SeongJae Park

Hello Brendan, could I ask you opinion for this patch?


Thanks,
SeongJae Park

[...]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kunit: tool: Assert the version requirement
  2021-06-17  7:46     ` [PATCH v2] kunit: tool: Assert the " SeongJae Park
  2021-06-22 23:28       ` Daniel Latypov
@ 2021-06-28 19:41       ` Brendan Higgins
  2021-07-12 19:42         ` Shuah Khan
  1 sibling, 1 reply; 12+ messages in thread
From: Brendan Higgins @ 2021-06-28 19:41 UTC (permalink / raw)
  To: SeongJae Park
  Cc: dlatypov, linux-kselftest, kunit-dev, linux-kernel, SeongJae Park

On Thu, Jun 17, 2021 at 12:46 AM SeongJae Park <sj38.park@gmail.com> wrote:
>
> Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
> tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
> Because it is supported on only >=3.7 Python, people using older Python
> will get below error:
>
>     Traceback (most recent call last):
>       File "./tools/testing/kunit/kunit.py", line 20, in <module>
>         import kunit_kernel
>       File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
>         from __future__ import annotations
>         ^
>     SyntaxError: future feature annotations is not defined
>
> This commit adds a version assertion in 'kunit.py', so that people get
> more explicit error message like below:
>
>     Traceback (most recent call last):
>       File "./tools/testing/kunit/kunit.py", line 15, in <module>
>         assert sys.version_info >= (3, 7), "Python version is too old"
>     AssertionError: Python version is too old
>
> Signed-off-by: SeongJae Park <sjpark@amazon.de>
> Acked-by: Daniel Latypov <dlatypov@google.com>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kunit: tool: Assert the version requirement
  2021-06-28 13:37           ` SeongJae Park
@ 2021-06-28 19:51             ` Brendan Higgins
  0 siblings, 0 replies; 12+ messages in thread
From: Brendan Higgins @ 2021-06-28 19:51 UTC (permalink / raw)
  To: SeongJae Park; +Cc: linux-kselftest, kunit-dev, linux-kernel, SeongJae Park

On Mon, Jun 28, 2021 at 6:38 AM SeongJae Park <sj38.park@gmail.com> wrote:
>
> Hello Brendan, could I ask you opinion for this patch?

Yep, sorry, I just wanted to make sure everything else (there were a
lot of things in the queue) was in order for the 5.14 merge window
first.

Looks good!

Thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kunit: tool: Assert the version requirement
  2021-06-28 19:41       ` Brendan Higgins
@ 2021-07-12 19:42         ` Shuah Khan
  2021-07-12 19:47           ` SeongJae Park
  2021-07-12 19:52           ` [PATCH v3] " SeongJae Park
  0 siblings, 2 replies; 12+ messages in thread
From: Shuah Khan @ 2021-07-12 19:42 UTC (permalink / raw)
  To: Brendan Higgins, SeongJae Park
  Cc: dlatypov, linux-kselftest, kunit-dev, linux-kernel,
	SeongJae Park, Shuah Khan

On 6/28/21 1:41 PM, Brendan Higgins wrote:
> On Thu, Jun 17, 2021 at 12:46 AM SeongJae Park <sj38.park@gmail.com> wrote:
>>
>> Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
>> tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
>> Because it is supported on only >=3.7 Python, people using older Python
>> will get below error:
>>
>>      Traceback (most recent call last):
>>        File "./tools/testing/kunit/kunit.py", line 20, in <module>
>>          import kunit_kernel
>>        File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
>>          from __future__ import annotations
>>          ^
>>      SyntaxError: future feature annotations is not defined
>>
>> This commit adds a version assertion in 'kunit.py', so that people get
>> more explicit error message like below:
>>
>>      Traceback (most recent call last):
>>        File "./tools/testing/kunit/kunit.py", line 15, in <module>
>>          assert sys.version_info >= (3, 7), "Python version is too old"
>>      AssertionError: Python version is too old
>>
>> Signed-off-by: SeongJae Park <sjpark@amazon.de>

Your from and Signed-off-by email addresses don't match.

Please resend the patch with the correction.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kunit: tool: Assert the version requirement
  2021-07-12 19:42         ` Shuah Khan
@ 2021-07-12 19:47           ` SeongJae Park
  2021-07-12 19:52           ` [PATCH v3] " SeongJae Park
  1 sibling, 0 replies; 12+ messages in thread
From: SeongJae Park @ 2021-07-12 19:47 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Brendan Higgins, SeongJae Park, dlatypov, linux-kselftest,
	kunit-dev, linux-kernel, SeongJae Park

From: SeongJae Park <sjpark@amazon.de>

On Mon, 12 Jul 2021 13:42:34 -0600 Shuah Khan <skhan@linuxfoundation.org> wrote:

> On 6/28/21 1:41 PM, Brendan Higgins wrote:
> > On Thu, Jun 17, 2021 at 12:46 AM SeongJae Park <sj38.park@gmail.com> wrote:
> >>
> >> Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
> >> tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
> >> Because it is supported on only >=3.7 Python, people using older Python
> >> will get below error:
> >>
> >>      Traceback (most recent call last):
> >>        File "./tools/testing/kunit/kunit.py", line 20, in <module>
> >>          import kunit_kernel
> >>        File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
> >>          from __future__ import annotations
> >>          ^
> >>      SyntaxError: future feature annotations is not defined
> >>
> >> This commit adds a version assertion in 'kunit.py', so that people get
> >> more explicit error message like below:
> >>
> >>      Traceback (most recent call last):
> >>        File "./tools/testing/kunit/kunit.py", line 15, in <module>
> >>          assert sys.version_info >= (3, 7), "Python version is too old"
> >>      AssertionError: Python version is too old
> >>
> >> Signed-off-by: SeongJae Park <sjpark@amazon.de>
> 
> Your from and Signed-off-by email addresses don't match.
> 
> Please resend the patch with the correction.

Sorry, my fault.  Will send fixed one as a reply to this.


Thanks,
SeongJae Park

> 
> thanks,
> -- Shuah

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] kunit: tool: Assert the version requirement
  2021-07-12 19:42         ` Shuah Khan
  2021-07-12 19:47           ` SeongJae Park
@ 2021-07-12 19:52           ` SeongJae Park
  1 sibling, 0 replies; 12+ messages in thread
From: SeongJae Park @ 2021-07-12 19:52 UTC (permalink / raw)
  To: skhan
  Cc: brendanhiggins, dlatypov, linux-kselftest, kunit-dev,
	linux-kernel, SeongJae Park

From: SeongJae Park <sjpark@amazon.de>

Commit 87c9c1631788 ("kunit: tool: add support for QEMU") on the 'next'
tree adds 'from __future__ import annotations' in 'kunit_kernel.py'.
Because it is supported on only >=3.7 Python, people using older Python
will get below error:

    Traceback (most recent call last):
      File "./tools/testing/kunit/kunit.py", line 20, in <module>
        import kunit_kernel
      File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 9
        from __future__ import annotations
        ^
    SyntaxError: future feature annotations is not defined

This commit adds a version assertion in 'kunit.py', so that people get
more explicit error message like below:

    Traceback (most recent call last):
      File "./tools/testing/kunit/kunit.py", line 15, in <module>
        assert sys.version_info >= (3, 7), "Python version is too old"
    AssertionError: Python version is too old

Signed-off-by: SeongJae Park <sjpark@amazon.de>
Acked-by: Daniel Latypov <dlatypov@google.com>
---

Changes from v2
- Fix mismatch of Signed-off and from

Changes from v1
- Add assertion failure message (Daniel Latypov)
- Add Acked-by: Daniel Latypov <dlatypov@google.com>

 tools/testing/kunit/kunit.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index be8d8d4a4e08..6276ce0c0196 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -12,6 +12,8 @@ import sys
 import os
 import time
 
+assert sys.version_info >= (3, 7), "Python version is too old"
+
 from collections import namedtuple
 from enum import Enum, auto
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-07-12 19:53 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-16  9:40 [PATCH] kunit: tool: Assert version requirement SeongJae Park
2021-06-16 21:14 ` Daniel Latypov
2021-06-17  7:39   ` SeongJae Park
2021-06-17  7:46     ` [PATCH v2] kunit: tool: Assert the " SeongJae Park
2021-06-22 23:28       ` Daniel Latypov
2021-06-22 23:55         ` Daniel Latypov
2021-06-28 13:37           ` SeongJae Park
2021-06-28 19:51             ` Brendan Higgins
2021-06-28 19:41       ` Brendan Higgins
2021-07-12 19:42         ` Shuah Khan
2021-07-12 19:47           ` SeongJae Park
2021-07-12 19:52           ` [PATCH v3] " SeongJae Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).