* [PATCH v2 1/2] codeparser/utils: clean up more deprecated AST usages
@ 2023-10-23 16:46 chris.laplante
2023-10-23 16:46 ` [PATCH v2 2/2] codegen: cleanup " chris.laplante
2023-10-24 10:44 ` [bitbake-devel] [PATCH v2 1/2] codeparser/utils: clean up more " Richard Purdie
0 siblings, 2 replies; 4+ messages in thread
From: chris.laplante @ 2023-10-23 16:46 UTC (permalink / raw)
To: bitbake-devel; +Cc: Chris Laplante
From: Chris Laplante <chris.laplante@agilent.com>
Also introduces bb.utils.ast_node_str_value method to simplify handling
of AST str nodes.
Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
lib/bb/codeparser.py | 22 ++++++++++------------
lib/bb/utils.py | 10 ++++++++++
2 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/lib/bb/codeparser.py b/lib/bb/codeparser.py
index cd39409434..92ed31fa5f 100644
--- a/lib/bb/codeparser.py
+++ b/lib/bb/codeparser.py
@@ -256,19 +256,18 @@ class PythonParser():
def visit_Call(self, node):
name = self.called_node_name(node.func)
if name and (name.endswith(self.getvars) or name.endswith(self.getvarflags) or name in self.containsfuncs or name in self.containsanyfuncs):
- if isinstance(node.args[0], ast.Constant) and isinstance(node.args[0].value, str):
- varname = node.args[0].value
- if name in self.containsfuncs and isinstance(node.args[1], ast.Str):
+ if (varname := bb.utils.ast_node_str_value(node.args[0])) is not None:
+ if name in self.containsfuncs and bb.utils.ast_node_str_value(node.args[1]) is not None:
if varname not in self.contains:
self.contains[varname] = set()
- self.contains[varname].add(node.args[1].s)
- elif name in self.containsanyfuncs and isinstance(node.args[1], ast.Str):
+ self.contains[varname].add(node.args[1].value)
+ elif name in self.containsanyfuncs and bb.utils.ast_node_str_value(node.args[1]) is not None:
if varname not in self.contains:
self.contains[varname] = set()
- self.contains[varname].update(node.args[1].s.split())
+ self.contains[varname].update(node.args[1].value.split())
elif name.endswith(self.getvarflags):
- if isinstance(node.args[1], ast.Str):
- self.references.add('%s[%s]' % (varname, node.args[1].s))
+ if bb.utils.ast_node_str_value(node.args[1]) is not None:
+ self.references.add('%s[%s]' % (varname, node.args[1].value))
else:
self.warn(node.func, node.args[1])
else:
@@ -276,8 +275,7 @@ class PythonParser():
else:
self.warn(node.func, node.args[0])
elif name and name.endswith(".expand"):
- if isinstance(node.args[0], ast.Str):
- value = node.args[0].s
+ if (value := bb.utils.ast_node_str_value(node.args[0])) is not None:
d = bb.data.init()
parser = d.expandWithRefs(value, self.name)
self.references |= parser.references
@@ -287,8 +285,8 @@ class PythonParser():
self.contains[varname] = set()
self.contains[varname] |= parser.contains[varname]
elif name in self.execfuncs:
- if isinstance(node.args[0], ast.Str):
- self.var_execs.add(node.args[0].s)
+ if bb.utils.ast_node_str_value(node.args[0]) is not None:
+ self.var_execs.add(node.args[0].value)
else:
self.warn(node.func, node.args[0])
elif name and isinstance(node.func, (ast.Name, ast.Attribute)):
diff --git a/lib/bb/utils.py b/lib/bb/utils.py
index b401fa5ec7..0ac304163c 100644
--- a/lib/bb/utils.py
+++ b/lib/bb/utils.py
@@ -11,6 +11,7 @@ import re, fcntl, os, string, stat, shutil, time
import sys
import errno
import logging
+import ast
import bb
import bb.msg
import locale
@@ -33,6 +34,7 @@ import random
import socket
import struct
import tempfile
+from typing import Optional
from subprocess import getstatusoutput
from contextlib import contextmanager
from ctypes import cdll
@@ -1863,3 +1865,11 @@ def lock_timeout(lock):
yield held
finally:
lock.release()
+
+def ast_node_str_value(node: ast.AST) -> Optional[str]:
+ """
+ Returns node value if it is an `ast.Constant` str; None otherwise
+ """
+ if isinstance(node, ast.Constant) and isinstance(node.value, str):
+ return node.value
+ return None
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] codegen: cleanup deprecated AST usages
2023-10-23 16:46 [PATCH v2 1/2] codeparser/utils: clean up more deprecated AST usages chris.laplante
@ 2023-10-23 16:46 ` chris.laplante
2023-10-24 10:44 ` [bitbake-devel] [PATCH v2 1/2] codeparser/utils: clean up more " Richard Purdie
1 sibling, 0 replies; 4+ messages in thread
From: chris.laplante @ 2023-10-23 16:46 UTC (permalink / raw)
To: bitbake-devel; +Cc: Chris Laplante
From: Chris Laplante <chris.laplante@agilent.com>
This code is just completely dead as of Python 3.8, like the comment
says.
Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
lib/codegen.py | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/lib/codegen.py b/lib/codegen.py
index 6955a7ada5..018b283177 100644
--- a/lib/codegen.py
+++ b/lib/codegen.py
@@ -392,19 +392,7 @@ class SourceGenerator(NodeVisitor):
def visit_Name(self, node):
self.write(node.id)
- def visit_Str(self, node):
- self.write(repr(node.s))
-
- def visit_Bytes(self, node):
- self.write(repr(node.s))
-
- def visit_Num(self, node):
- self.write(repr(node.n))
-
def visit_Constant(self, node):
- # Python 3.8 deprecated visit_Num(), visit_Str(), visit_Bytes(),
- # visit_NameConstant() and visit_Ellipsis(). They can be removed once we
- # require 3.8+.
self.write(repr(node.value))
def visit_Tuple(self, node):
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [bitbake-devel] [PATCH v2 1/2] codeparser/utils: clean up more deprecated AST usages
2023-10-23 16:46 [PATCH v2 1/2] codeparser/utils: clean up more deprecated AST usages chris.laplante
2023-10-23 16:46 ` [PATCH v2 2/2] codegen: cleanup " chris.laplante
@ 2023-10-24 10:44 ` Richard Purdie
2023-10-24 14:20 ` chris.laplante
1 sibling, 1 reply; 4+ messages in thread
From: Richard Purdie @ 2023-10-24 10:44 UTC (permalink / raw)
To: chris.laplante, bitbake-devel
On Mon, 2023-10-23 at 12:46 -0400, Chris Laplante via
lists.openembedded.org wrote:
> From: Chris Laplante <chris.laplante@agilent.com>
>
> Also introduces bb.utils.ast_node_str_value method to simplify handling
> of AST str nodes.
>
> Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
> ---
> lib/bb/codeparser.py | 22 ++++++++++------------
> lib/bb/utils.py | 10 ++++++++++
> 2 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/lib/bb/codeparser.py b/lib/bb/codeparser.py
> index cd39409434..92ed31fa5f 100644
> --- a/lib/bb/codeparser.py
> +++ b/lib/bb/codeparser.py
> @@ -256,19 +256,18 @@ class PythonParser():
> def visit_Call(self, node):
> name = self.called_node_name(node.func)
> if name and (name.endswith(self.getvars) or name.endswith(self.getvarflags) or name in self.containsfuncs or name in self.containsanyfuncs):
> - if isinstance(node.args[0], ast.Constant) and isinstance(node.args[0].value, str):
> - varname = node.args[0].value
> - if name in self.containsfuncs and isinstance(node.args[1], ast.Str):
> + if (varname := bb.utils.ast_node_str_value(node.args[0])) is not None:
> + if name in self.containsfuncs and bb.utils.ast_node_str_value(node.args[1]) is not None:
Personally, I really don't like condition tests modifying variables as
it can be easy to miss when reading the code. I therefore have a bit of
a preference for not doing that.
Perhaps it just takes time to get used to the new syntax, perhaps I'm
just old fashioned.
> if varname not in self.contains:
> self.contains[varname] = set()
> - self.contains[varname].add(node.args[1].s)
> - elif name in self.containsanyfuncs and isinstance(node.args[1], ast.Str):
> + self.contains[varname].add(node.args[1].value)
> + elif name in self.containsanyfuncs and bb.utils.ast_node_str_value(node.args[1]) is not None:
> if varname not in self.contains:
> self.contains[varname] = set()
> - self.contains[varname].update(node.args[1].s.split())
> + self.contains[varname].update(node.args[1].value.split())
> elif name.endswith(self.getvarflags):
> - if isinstance(node.args[1], ast.Str):
> - self.references.add('%s[%s]' % (varname, node.args[1].s))
> + if bb.utils.ast_node_str_value(node.args[1]) is not None:
> + self.references.add('%s[%s]' % (varname, node.args[1].value))
> else:
> self.warn(node.func, node.args[1])
> else:
> @@ -276,8 +275,7 @@ class PythonParser():
> else:
> self.warn(node.func, node.args[0])
> elif name and name.endswith(".expand"):
> - if isinstance(node.args[0], ast.Str):
> - value = node.args[0].s
> + if (value := bb.utils.ast_node_str_value(node.args[0])) is not None:
> d = bb.data.init()
> parser = d.expandWithRefs(value, self.name)
> self.references |= parser.references
> @@ -287,8 +285,8 @@ class PythonParser():
> self.contains[varname] = set()
> self.contains[varname] |= parser.contains[varname]
> elif name in self.execfuncs:
> - if isinstance(node.args[0], ast.Str):
> - self.var_execs.add(node.args[0].s)
> + if bb.utils.ast_node_str_value(node.args[0]) is not None:
> + self.var_execs.add(node.args[0].value)
> else:
> self.warn(node.func, node.args[0])
> elif name and isinstance(node.func, (ast.Name, ast.Attribute)):
> diff --git a/lib/bb/utils.py b/lib/bb/utils.py
> index b401fa5ec7..0ac304163c 100644
> --- a/lib/bb/utils.py
> +++ b/lib/bb/utils.py
> @@ -11,6 +11,7 @@ import re, fcntl, os, string, stat, shutil, time
> import sys
> import errno
> import logging
> +import ast
> import bb
> import bb.msg
> import locale
> @@ -33,6 +34,7 @@ import random
> import socket
> import struct
> import tempfile
> +from typing import Optional
> from subprocess import getstatusoutput
> from contextlib import contextmanager
> from ctypes import cdll
> @@ -1863,3 +1865,11 @@ def lock_timeout(lock):
> yield held
> finally:
> lock.release()
> +
> +def ast_node_str_value(node: ast.AST) -> Optional[str]:
> + """
> + Returns node value if it is an `ast.Constant` str; None otherwise
> + """
> + if isinstance(node, ast.Constant) and isinstance(node.value, str):
> + return node.value
> + return None
We were previously asked about adding typing and again I'm a little
unsure we want to do that. Personally, I think it looks horribly ugly
and I'm not convinced it buys us that much. I know others have other
opinions.
Whilst I am in theory bitbake's maintainer and in theory I can say no,
I know I'll get asked when we're converting everything time and time
again so again I'm really torn on this :/.
Cheers,
Richard
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [bitbake-devel] [PATCH v2 1/2] codeparser/utils: clean up more deprecated AST usages
2023-10-24 10:44 ` [bitbake-devel] [PATCH v2 1/2] codeparser/utils: clean up more " Richard Purdie
@ 2023-10-24 14:20 ` chris.laplante
0 siblings, 0 replies; 4+ messages in thread
From: chris.laplante @ 2023-10-24 14:20 UTC (permalink / raw)
To: Richard Purdie, bitbake-devel
Hi Richard,
> > def visit_Call(self, node):
> > name = self.called_node_name(node.func)
> > if name and (name.endswith(self.getvars) or
> name.endswith(self.getvarflags) or name in self.containsfuncs or name in
> self.containsanyfuncs):
> > - if isinstance(node.args[0], ast.Constant) and
> isinstance(node.args[0].value, str):
> > - varname = node.args[0].value
> > - if name in self.containsfuncs and isinstance(node.args[1], ast.Str):
> > + if (varname := bb.utils.ast_node_str_value(node.args[0])) is not
> None:
> > + if name in self.containsfuncs and
> bb.utils.ast_node_str_value(node.args[1]) is not None:
>
> Personally, I really don't like condition tests modifying variables as it can be
> easy to miss when reading the code. I therefore have a bit of a preference for
> not doing that.
>
> Perhaps it just takes time to get used to the new syntax, perhaps I'm just old
> fashioned.
I'm happy to submit a v3 without the walrus operator if you prefer :).
> > +
> > +def ast_node_str_value(node: ast.AST) -> Optional[str]:
> > + """
> > + Returns node value if it is an `ast.Constant` str; None otherwise
> > + """
> > + if isinstance(node, ast.Constant) and isinstance(node.value, str):
> > + return node.value
> > + return None
>
> We were previously asked about adding typing and again I'm a little unsure we
> want to do that. Personally, I think it looks horribly ugly and I'm not convinced it
> buys us that much. I know others have other opinions.
>
> Whilst I am in theory bitbake's maintainer and in theory I can say no, I know I'll
> get asked when we're converting everything time and time again so again I'm
> really torn on this :/.
IMHO BitBake is so large and complicated that it would benefit from it. I'm a bit biased, being a heavy user of statically-typed languages and an IDE (IntelliJ) that moans loudly about perceived type mismatches. So as with the walrus, I can resubmit if the answer is 'no' :).
Thanks,
Chris
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-24 14:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-23 16:46 [PATCH v2 1/2] codeparser/utils: clean up more deprecated AST usages chris.laplante
2023-10-23 16:46 ` [PATCH v2 2/2] codegen: cleanup " chris.laplante
2023-10-24 10:44 ` [bitbake-devel] [PATCH v2 1/2] codeparser/utils: clean up more " Richard Purdie
2023-10-24 14:20 ` chris.laplante
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.