[WIP 0/2] Adding support for shell arithmetics

[WIP 0/2] Adding support for shell arithmetics

[Olof Johansson]

Hi,

A week or so I found out that shell fragments in bitbake recipes
don't support shell arithmetics (`$((1+1))`). The reason for this
is the shell lexing throwing NotImplemented exceptions. I've made
some progress in adding support for shell arithmetics, but I have
a hard time getting support for corner cases.

It was suprising to see that currently, only a subset of posix
shell scripts is supported, even though they are sent through to
be executed by the system shell. I think I understand the reason
for having to do the shell lexing; knowing what functions and
variables to export right? But I wonder if it's possible to do
the shell lexing as a best effort, and ignore any errors.

What are the risks with this approach? Is it feasibile to do so?

This patch series is a work in progress, and does support the use
cases we had issues with, however, I currently skip one unit
test, as I can't get it to work. Comments and feedback much
appreciated!

The branch is also pushed to github at:

 https://github.com/olof/bitbake/tree/topic/shlex_shell_arith

Olof Johansson (2):
  pysh: Say what kind of token isn't implemented
  initial work on supporting shell arith

 bin/bitbake-selftest       |    1 +
 lib/bb/pysh/pyshlex.py     |   48 ++++++++++++++++--
 lib/bb/tests/codeparser.py |   22 ++++++++
 lib/bb/tests/pysh.py       |  119 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 186 insertions(+), 4 deletions(-)
 create mode 100644 lib/bb/tests/pysh.py

-- 
1.7.10.4

[WIP 1/2] pysh: Say what kind of token isn't implemented

[Olof Johansson]

When the shell lexer finds an unrecognized dollar token, the error
message should contain what kind of token it is having problems with.

Signed-off-by: Olof Johansson <olof.johansson@axis.com>
---
 lib/bb/pysh/pyshlex.py |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bb/pysh/pyshlex.py b/lib/bb/pysh/pyshlex.py
index b977b5e..b301236 100644
--- a/lib/bb/pysh/pyshlex.py
+++ b/lib/bb/pysh/pyshlex.py
@@ -292,7 +292,7 @@ class WordLexer:
         elif sep=='${':
             parsefunc = self._parse_parameter
         else:
-            raise NotImplementedError()
+            raise NotImplementedError(sep)
             
         pos, closed = parsefunc(buf, result, eof)
         return pos, closed
-- 
1.7.10.4

[WIP 2/2] initial work on supporting shell arith

[Olof Johansson]

Signed-off-by: Olof Johansson <olof.johansson@axis.com>
---
 bin/bitbake-selftest       |    1 +
 lib/bb/pysh/pyshlex.py     |   46 +++++++++++++++--
 lib/bb/tests/codeparser.py |   22 ++++++++
 lib/bb/tests/pysh.py       |  119 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 185 insertions(+), 3 deletions(-)
 create mode 100644 lib/bb/tests/pysh.py

diff --git a/bin/bitbake-selftest b/bin/bitbake-selftest
index 48a58fe..904a801 100755
--- a/bin/bitbake-selftest
+++ b/bin/bitbake-selftest
@@ -29,6 +29,7 @@ tests = ["bb.tests.codeparser",
          "bb.tests.cow",
          "bb.tests.data",
          "bb.tests.fetch",
+         "bb.tests.pysh",
          "bb.tests.utils"]
 
 for t in tests:
diff --git a/lib/bb/pysh/pyshlex.py b/lib/bb/pysh/pyshlex.py
index b301236..0c68e57 100644
--- a/lib/bb/pysh/pyshlex.py
+++ b/lib/bb/pysh/pyshlex.py
@@ -211,7 +211,45 @@ class WordLexer:
         else:
             #Keep everything until the separator and defer processing
             return pos, False
-            
+
+    def _find_balanced(self, buf, delim, escapes=True):
+        expr = 1
+        pos = 0
+
+        while expr > 0 and pos < len(buf):
+            # open delimiter
+            if ( delim[0] is not None and
+                 ''.join(buf[pos : pos+len(delim[0])]) == delim[0]):
+                expr += 1
+                pos += len(delim[0]) - 1
+            # closing delimiter
+            elif ''.join(buf[pos : pos+len(delim[1])]) == delim[1]:
+                expr -= 1
+                pos += len(delim[1]) - 1
+            elif escapes and buf[pos] == '\\':
+                pos += 1 # skip next, it's escaped!
+
+            pos += 1
+
+        if expr > 0:
+            return -1 # Could not find matching pair of delims
+        return pos
+
+    def _parse_arithmetic(self, buf, result, eof):
+        if not buf:
+            raise NeedMore()
+
+        pos = self._find_balanced(buf, ("$((", "))"))
+
+        if pos < 0:
+            raise NeedMore()
+
+        result[-1] += ''.join(buf[:pos-2])
+        result.append("))")
+
+        has_eof = len(buf) <= pos
+        return pos, has_eof
+
     def _parse_command(self, buf, result, eof):
         if not buf:
             raise NeedMore()
@@ -229,7 +267,7 @@ class WordLexer:
             return pos+1, True
         else:
             return pos, False
-            
+
     def _parse_parameter(self, buf, result, eof):
         if not buf:
             raise NeedMore()
@@ -289,6 +327,8 @@ class WordLexer:
         sep = result[0]    
         if sep=='$(':
             parsefunc = self._parse_command
+        elif sep=='$((':
+            parsefunc = self._parse_arithmetic
         elif sep=='${':
             parsefunc = self._parse_parameter
         else:
@@ -386,7 +426,7 @@ def make_wordtree(token, here_document=False):
         try:
             result, remaining = WordLexer(heredoc = here_document).add(remaining, True)
         except NeedMore:
-            raise ShellSyntaxError('Invalid token "%s"')
+            raise ShellSyntaxError('Invalid token "%s"' % remaining)
         tree.append(result)
         
                 
diff --git a/lib/bb/tests/codeparser.py b/lib/bb/tests/codeparser.py
index 938b04b..debb933 100644
--- a/lib/bb/tests/codeparser.py
+++ b/lib/bb/tests/codeparser.py
@@ -110,6 +110,28 @@ ${D}${libdir}/pkgconfig/*.pc
         self.parseExpression("foo=$(echo bar)")
         self.assertExecs(set(["echo"]))
 
+    def test_assign_arith(self):
+        self.parseExpression("python2.$((8-1))")
+        self.assertExecs(set(["python2.$((8-1))"]))
+
+    def test_assign_arith_only(self):
+        self.parseExpression("$((8-1))")
+        self.assertExecs(set(["$((8-1))"]))
+
+    @unittest.skip(
+        "FIXME: multiple shell arith expressions are not supported")
+    def test_assign_arith_dual(self):
+        self.parseExpression("$((1+1))to$((4-1))")
+        self.assertExecs(set(["$((1+1))to$((4-1))"]))
+
+    def test_assign_arith_nested1(self):
+        self.parseExpression("$((1+$((1))))")
+        self.assertExecs(set(["$((1+$((1))))"]))
+
+    def test_assign_arith_nested2(self):
+        self.parseExpression("$(( 1 + $(( 1 )) ))")
+        self.assertExecs(set(["$(( 1 + $(( 1 )) ))"]))
+
     def test_shell_unexpanded(self):
         self.setEmptyVars(["QT_BASE_NAME"])
         self.parseExpression('echo "${QT_BASE_NAME}"')
diff --git a/lib/bb/tests/pysh.py b/lib/bb/tests/pysh.py
new file mode 100644
index 0000000..3f64f14
--- /dev/null
+++ b/lib/bb/tests/pysh.py
@@ -0,0 +1,119 @@
+# ex:ts=4:sw=4:sts=4:et
+# -*- tab-width: 4; c-basic-offset: 4; indent-tabs-mode: nil -*-
+#
+# BitBake Tests for the pysh lexer (pysh/)
+#
+# Copyright (C) 2013 Olof Johansson <olof.johansson@axis.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+
+import unittest
+from bb.pysh.pyshlex import WordLexer, NeedMore
+
+orig_init = WordLexer.__init__
+
+class WordLexerParseTest(unittest.TestCase):
+    # mock out WordLexer's constructor
+    def SetUp(self):
+        WordLexer.__init__ = lambda: None
+
+    def TearDown(self):
+        WordLexer.__init__ = orig_init
+
+    # The arithmetic tests make sure that WordLexer's _parse_arithmetic
+    # method can parse out the expected shell arithmetic expression,
+    # including support for nested expressions.
+    def assertArithmeticParser(self, expr, expect=None, eof=True, length=None):
+        expr = expr[3:] # strip off $((
+        result = ["$((", ""]
+        buf = list(expr)
+
+        # Unless stated otherwise, let's assume the complete buf
+        # is an shell arithmetic expression.
+        if not expect:
+            expect = ["$((", expr[:-2], "))"]
+        if not length:
+            length = len(expr)
+
+        wl = WordLexer()
+        pos, got_eof = WordLexer._parse_arithmetic(wl, buf, result, True)
+
+        self.assertEqual(buf, list(expr))
+        self.assertEqual(result, expect)
+        self.assertEqual(pos, length)
+        self.assertEqual(got_eof, eof)
+
+    def test_parse_arithmetic_simple(self):
+        self.assertArithmeticParser("$((1+1))")
+
+    def test_parse_arithmetic_simple_trailing(self):
+        self.assertArithmeticParser(
+            "$((1+1)); uptime",
+            expect=["$((", "1+1", "))"],
+            length=5,
+            eof=False)
+
+    def test_parse_arithmetic_incomplete(self):
+        wl = WordLexer()
+        res = ["$((", ""]
+        buf = list("10+")
+
+        needs_more = False
+        try:
+            pos, got_eof = WordLexer._parse_arithmetic(wl, buf, res, True)
+        except NeedMore:
+            needs_more = True
+
+        self.assertTrue(needs_more)
+
+    def test_parse_arithmetic_empty(self):
+        wl = WordLexer()
+        res = ["$((", ""]
+        buf = list("")
+
+        needs_more = False
+        try:
+            pos, got_eof = WordLexer._parse_arithmetic(wl, buf, res, True)
+        except NeedMore:
+            needs_more = True
+
+        self.assertTrue(needs_more)
+
+    def test_parse_arithmetic_simple_whitespace(self):
+        self.assertArithmeticParser("$(( 1 + 1 ))")
+
+    def test_parse_arithmetic_nested1(self):
+        self.assertArithmeticParser("$((1+$((1+2))))")
+
+    def test_parse_arithmetic_nested2(self):
+        self.assertArithmeticParser("$(($((1+2))+$((1+2))))")
+
+    def assertCommandParser(self, command="", sep=("$(", ")"), eof=True):
+        result = [sep[0], ""]
+        buf = ''.join([command, sep[1]])
+
+        wl = WordLexer()
+        pos, got_eof = WordLexer._parse_command(wl, buf, result, True)
+
+        self.assertEqual(result, [sep[0], command, sep[1]])
+
+    def test_parse_command_simple(self):
+        self.assertCommandParser("uptime")
+
+    def test_parse_command_backticks(self):
+        self.assertCommandParser("echo bar", sep=("`", "`"))
+
+    #def test_parse_command_nested(self):
+    #    self.assertCommandParser("echo $(uptime)")
-- 
1.7.10.4

Re: [WIP 0/2] Adding support for shell arithmetics

[Olof Johansson]

On 2013-05-07 11:22, Olof Johansson wrote:
> Hi,
> 
> A week or so I found out that shell fragments in bitbake recipes
> don't support shell arithmetics (`$((1+1))`). The reason for this
> is the shell lexing throwing NotImplemented exceptions. I've made
> some progress in adding support for shell arithmetics, but I have
> a hard time getting support for corner cases.
> 
> It was suprising to see that currently, only a subset of posix
> shell scripts is supported, even though they are sent through to
> be executed by the system shell. I think I understand the reason
> for having to do the shell lexing; knowing what functions and
> variables to export right? But I wonder if it's possible to do
> the shell lexing as a best effort, and ignore any errors.
> 
> What are the risks with this approach? Is it feasibile to do so?
> 
> This patch series is a work in progress, and does support the use
> cases we had issues with, however, I currently skip one unit
> test, as I can't get it to work. Comments and feedback much
> appreciated!

Did anybody have any feedback on this? I have had a hard time
understanding the design of the lexing. I really could use some
pointers --- is there some documents somewhere that could make me
understand it better? I haven't had time to work on this lately,
but it would be great to get this fixed, so I figured I'll try to
take some time in the near future.

/Olof

PS. Saw that RP merged the exception message patch, thanks :).

Re: [WIP 0/2] Adding support for shell arithmetics

[Richard Purdie]

On Fri, 2013-06-14 at 15:55 +0200, Olof Johansson wrote:
> On 2013-05-07 11:22, Olof Johansson wrote:
> > Hi,
> > 
> > A week or so I found out that shell fragments in bitbake recipes
> > don't support shell arithmetics (`$((1+1))`). The reason for this
> > is the shell lexing throwing NotImplemented exceptions. I've made
> > some progress in adding support for shell arithmetics, but I have
> > a hard time getting support for corner cases.
> > 
> > It was suprising to see that currently, only a subset of posix
> > shell scripts is supported, even though they are sent through to
> > be executed by the system shell. I think I understand the reason
> > for having to do the shell lexing; knowing what functions and
> > variables to export right? But I wonder if it's possible to do
> > the shell lexing as a best effort, and ignore any errors.
> > 
> > What are the risks with this approach? Is it feasibile to do so?
> > 
> > This patch series is a work in progress, and does support the use
> > cases we had issues with, however, I currently skip one unit
> > test, as I can't get it to work. Comments and feedback much
> > appreciated!
> 
> Did anybody have any feedback on this? I have had a hard time
> understanding the design of the lexing. I really could use some
> pointers --- is there some documents somewhere that could make me
> understand it better? I haven't had time to work on this lately,
> but it would be great to get this fixed, so I figured I'll try to
> take some time in the near future.
> 
> /Olof
> 
> PS. Saw that RP merged the exception message patch, thanks :).

That one was easy :)

I'm afraid I don't have much knowledge of the sh lexing code, Chris
might have more knowledge than me...

Cheers,

Richard