[PATCH] image.bbclass: support TMPDIR/DATETIME inside complex variables

[PATCH] image.bbclass: support TMPDIR/DATETIME inside complex variables

[Patrick Ohly]

Not replacing TMPDIR at all (from in OE-core:a8c377beadb85b0ff50)
led to an expansion error when INITRD needs to passed to the image command:
   ERROR: ExpansionError during parsing ....bb: Failure expanding variable INITRD, expression was ${@bb.utils.contains('MACHINE_FEATURES', 'intel-ucode', '${TMPDIR}/deploy/images/intel-corei7-64/microcode.cpio ', '', d)}

That's because the replacement of ${@ stops at the curly brackets after
TMPDIR, leading to an incomplete Python expression. The right solution
would be to enhance bitbake's data_smart.py such that it does not rely
on a regular expression to find the matching closing bracket.

But that is a performance sensitive area that will need good testing,
so as a temporary workaround for image creation, variable references
to DATETIME and TMPDIR get expanded to ___DATETIME___
resp. ___TMPDIR___ (special strings which are unlikely to be used but
are valid when inserted into a shell command; <<TMPDIR>> for example
did not work because the temporary image command was getting parsed as
shell script) and then turned back into the original variable
references, which is how the code worked before this change.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/image.bbclass | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 3870516..e5d1eb4 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -348,6 +348,8 @@ python () {
 
     maskedtypes = (d.getVar('IMAGE_TYPES_MASKED', True) or "").split()
 
+    import re
+    undovarset = re.compile(r'___(DATETIME|TMPDIR)___')
     for t in basetypes:
         vardeps = set()
         cmds = []
@@ -366,12 +368,18 @@ python () {
         localdata.setVar('OVERRIDES', '%s:%s' % (realt, old_overrides))
         bb.data.update_data(localdata)
         localdata.setVar('type', realt)
-        # Delete DATETIME so we don't expand any references to it now
+        # Expand DATETIME to a fixed value that we will replace with the
+        # real value later. In particular we have to get rid of the brackets,
+        # because keeping ${DATETIME} and ${TMPDIR} unchanged leads to parse
+        # errors when expanding the outer expression (like ${@ .... '${TMPDIR}/...' })
+        # variables are used inside more complex strings. The temporary
+        # replacement must be a valid path name, otherwise parsing
+        # the shell script where it gets embedded fails.
         # This means the task's hash can be stable rather than having hardcoded
         # date/time values. It will get expanded at execution time.
         # Similarly TMPDIR since otherwise we see QA stamp comparision problems
-        localdata.delVar('DATETIME')
-        localdata.delVar('TMPDIR')
+        localdata.setVar('DATETIME', '___DATETIME___')
+        localdata.setVar('TMPDIR', '___TMPDIR___')
 
         image_cmd = localdata.getVar("IMAGE_CMD", True)
         vardeps.add('IMAGE_CMD_' + realt)
@@ -399,7 +407,10 @@ python () {
 
         t = t.replace("-", "_")
 
-        d.setVar('do_image_%s' % t, '\n'.join(cmds))
+        # Undo the temporary replacement and insert the variable references again.
+        doimage = '\n'.join(cmds)
+        doimage = undovarset.sub(r'${\1}', doimage)
+        d.setVar('do_image_%s' % t, doimage)
         d.setVarFlag('do_image_%s' % t, 'func', '1')
         d.setVarFlag('do_image_%s' % t, 'fakeroot', '1')
         d.setVarFlag('do_image_%s' % t, 'prefuncs', debug + 'set_image_size')
-- 
2.1.4

Re: [PATCH] image.bbclass: support TMPDIR/DATETIME inside complex variables

[Patrick Ohly]

On Thu, 2016-01-21 at 14:23 +0100, Patrick Ohly wrote:
> Not replacing TMPDIR at all (from in OE-core:a8c377beadb85b0ff50)
> led to an expansion error when INITRD needs to passed to the image command:
>    ERROR: ExpansionError during parsing ....bb: Failure expanding variable INITRD, expression was ${@bb.utils.contains('MACHINE_FEATURES', 'intel-ucode', '${TMPDIR}/deploy/images/intel-corei7-64/microcode.cpio ', '', d)}
> 
> That's because the replacement of ${@ stops at the curly brackets after
> TMPDIR, leading to an incomplete Python expression. The right solution
> would be to enhance bitbake's data_smart.py such that it does not rely
> on a regular expression to find the matching closing bracket.

Richard proposed a different solution for this, which is an enhancement
for bitbake which stops expanding Python expressions when the problem
strikes:
http://lists.openembedded.org/pipermail/bitbake-devel/2016-January/006932.html

I'm fine with dropping my proposed patch and using the bitbake
enhancement instead. I checked that it also addresses the specific
problem that I ran into.

However, after thinking about it some more, I suspect that it leads to
situations where sstate hashes do not properly track variable
dependencies.

Suppose an image command is passed a variable like this:
  FOO = "${@ os.join.paths('${TMPDIR}', (bb.utils.contains('BAR', 'a', 'x', 'y', d)}"

If the entire expression was expanded, the result would change depending
on whether BAR contains a or not. But because expanding the Python
expression stops, FOO is essentially constant when checking sstate
hashes and only expands later.

So ultimately, a proper solution for matching brackets for Python
expressions in data_smart.py will be needed to cover all cases.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.