[PATCH] [yocto-ab-helper] utils.py: Resolved unicode data expansion

[PATCH] [yocto-ab-helper] utils.py: Resolved unicode data expansion

[Aaron Chan]

Patch fix on utils:getconfig:expandresult function to handle the expansion
This patch is to add a condition to handle unicode entries as dict & list
have been handled during expandresult.

janitor/clobberdir: [line 46]: changes
from : trashdir = ourconfig["TRASH_DIR"]
to   : trashdir = utils.getconfig("TRASH_DIR", ourconfig)

scripts/utils.py:  [line 41-47]: added
getconfig invokes only unicode entries and handles the data expansions.
This allows ${BUILDDIR} to be expanded, to retain ${BUILDDIR} in ourconfig[c],
we should never invoke utils.getconfig("BUILDDIR", ourconfig) in our scripts
unless we intend to change the BUILDDIR paths.

Signed-off-by: Aaron Chan <aaron.chun.yew.chan@intel.com>
---
 janitor/clobberdir | 5 ++---
 scripts/utils.py   | 8 ++++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/janitor/clobberdir b/janitor/clobberdir
index 5dab5af..5e04ed7 100755
--- a/janitor/clobberdir
+++ b/janitor/clobberdir
@@ -43,11 +43,10 @@ if "TRASH_DIR" not in ourconfig:
     print("Please set TRASH_DIR in the configuration file")
     sys.exit(1)
 
-trashdir = ourconfig["TRASH_DIR"]
+trashdir = utils.getconfig("TRASH_DIR", ourconfig)
 
 for x in [clobberdir]:
     if os.path.exists(x):
         trashdest = trashdir + "/" + str(int(time.time())) + '-'  + str(random.randrange(100, 100000, 2))
         mkdir(trashdest)
-        subprocess.check_call(['mv', x, trashdest])
-
+        subprocess.check_call(['mv', x, trashdest])
\ No newline at end of file
diff --git a/scripts/utils.py b/scripts/utils.py
index db1e3c2..373f8de 100644
--- a/scripts/utils.py
+++ b/scripts/utils.py
@@ -26,6 +26,7 @@ def configtrue(name, config):
 # Handle variable expansion of return values, variables are of the form ${XXX}
 # need to handle expansion in list and dicts
 __expand_re__ = re.compile(r"\${[^{}@\n\t :]+}")
+__expansion__ = re.compile(r"\${(.+)}")
 def expandresult(entry, config):
     if isinstance(entry, list):
         ret = []
@@ -37,6 +38,13 @@ def expandresult(entry, config):
         for k in entry:
             ret[expandresult(k, config)] = expandresult(entry[k], config)
         return ret
+    if isinstance(entry, unicode):
+        entry = str(entry)
+        entryExpand = __expansion__.match(entry).group(1)
+        if entryExpand:
+            return entry.replace('${' + entryExpand + '}', config[entryExpand])
+        else:
+            return entry
     if not isinstance(entry, str):
         return entry
     class expander:
-- 
2.7.4

Re: [PATCH] [yocto-ab-helper] utils.py: Resolved unicode data expansion

[Chan, Aaron Chun Yew]

Hi Richard,

This is what I did on which you asked me to do

~/yocto-autobuilder-helper/janitor/clobberdir

1. Change to use python3 
    #!/usr/bin/env/python2 to #!/usr/bin/env/python3
    
    Retain and add print to debug message
    trashdir = ourconfig["TRASH_DIR"]
    print(trashdir)   

Results:
mv: cannot move '/home/pokybuild/yocto-worker/nightly-x86-64-bsp/' to a subdirectory of itself, '${BASE_HOMEDIR}/git/trash/1530767509-20564/nightly-x86-64-bsp'
${BASE_HOMEDIR}/git/trash   <----------------------- debug message
Traceback (most recent call last):
  File "/home/pokybuild/yocto-autobuilder-helper/janitor/clobberdir", line 54, in <module>
    subprocess.check_call(['mv', x, trashdest])
  File "/usr/lib/python3.5/subprocess.py", line 581, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['mv', '/home/pokybuild/yocto-worker/nightly-x86-64-bsp/', '${BASE_HOMEDIR}/git/trash/1530767509-20564']' returned non-zero exit status 1
program finished with exit code 1

2. Change to use python3
    #!/usr/bin/env python2 to #!/usr/bin/env/python3
    
    Change and print to debug message
    trashdir = utils.getconfig("TRASH_DIR", ourconfig)
    print(trashdir)

Results:
using PTY: False
/home/pokybuild/git/trash      <----------------------- debug message
program finished with exit code 0
elapsedTime=0.050781

I'll updated the patch on Item 2 in https://lists.yoctoproject.org/pipermail/yocto/2018-July/041694.html
Thanks,

Best wishes,
Aaron
________________________________________
From: richard.purdie@linuxfoundation.org [richard.purdie@linuxfoundation.org]
Sent: Wednesday, July 04, 2018 3:51 PM
To: Chan, Aaron Chun Yew; yocto@yoctoproject.org
Subject: Re: [PATCH] [yocto-ab-helper] utils.py: Resolved unicode data expansion

Hi Aaron,

On Wed, 2018-07-04 at 06:42 +0000, Chan, Aaron Chun Yew wrote:
> This morning I had the new autobuilder setup from scratch with the
> latest patch you just check out, thanks for that and rollout the fix
> below manually. Everything else is clean and I did what you asked me
> to.
> However this did not resolved the data expansion when called
> utils.getconfig("REPO_STASH_DIR, ourconfig), this applies when you
> invoke ourconfig["REPO_STASH_DIR"]. Both yields the same errors.
> We assume the  JSON dumps are properly handled in ourconfig[c] when
> we handles config[c] but that is not the case. I do see there is a
> growing issues as your strategy to use nested JSON, however
> we wont be able to handle all of these conditions needed when nested
> JSON becomes complex. Anyway, i'll leave it you decide what will be
> best course of action.

I had another look at this as we shouldn't be seeing unicode items in
this in python3. I've realised that clobberdir is using python2. If you
change clobberdir to use python3 does this mean the unicode part of the
patch is no longer needed? I think that may be the real problem!

Cheers,

Richard

Re: [PATCH] [yocto-ab-helper] utils.py: Resolved unicode data expansion

[Richard Purdie]

Hi Aaron,

On Wed, 2018-07-04 at 06:42 +0000, Chan, Aaron Chun Yew wrote:
> This morning I had the new autobuilder setup from scratch with the
> latest patch you just check out, thanks for that and rollout the fix
> below manually. Everything else is clean and I did what you asked me
> to.
> However this did not resolved the data expansion when called
> utils.getconfig("REPO_STASH_DIR, ourconfig), this applies when you
> invoke ourconfig["REPO_STASH_DIR"]. Both yields the same errors.
> We assume the  JSON dumps are properly handled in ourconfig[c] when
> we handles config[c] but that is not the case. I do see there is a
> growing issues as your strategy to use nested JSON, however 
> we wont be able to handle all of these conditions needed when nested
> JSON becomes complex. Anyway, i'll leave it you decide what will be
> best course of action.

I had another look at this as we shouldn't be seeing unicode items in
this in python3. I've realised that clobberdir is using python2. If you
change clobberdir to use python3 does this mean the unicode part of the
patch is no longer needed? I think that may be the real problem!

Cheers,

Richard

Re: [PATCH] [yocto-ab-helper] utils.py: Resolved unicode data expansion

[Chan, Aaron Chun Yew]

Hello Richard,

This morning I had the new autobuilder setup from scratch with the latest patch you just check out, thanks for that and rollout the fix below manually. Everything else is clean and I did what you asked me to.
However this did not resolved the data expansion when called utils.getconfig("REPO_STASH_DIR, ourconfig), this applies when you invoke ourconfig["REPO_STASH_DIR"]. Both yields the same errors.
We assume the  JSON dumps are properly handled in ourconfig[c] when we handles config[c] but that is not the case. I do see there is a growing issues as your strategy to use nested JSON, however 
we wont be able to handle all of these conditions needed when nested JSON becomes complex. Anyway, i'll leave it you decide what will be best course of action.

STDERR logs on autobuilder: (poky-tiny)
--------------------------------------
mv: cannot move '/home/pokybuild/yocto-worker/poky-tiny/' to a subdirectory of itself, '${BASE_HOMEDIR}/git/mirror/1530669213-56172/poky-tiny'
Traceback (most recent call last):
  File "/home/pokybuild/yocto-autobuilder-helper/janitor/clobberdir", line 52, in <module>
    subprocess.check_call(['mv', x, trashdest])
  File "/usr/lib/python2.7/subprocess.py", line 541, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['mv', '/home/pokybuild/yocto-worker/poky-tiny/', u'${BASE_HOMEDIR}/git/mirror/1530669213-56172']' returned non-zero exit status 1

also, this action causes directory named ${BASE_HOMEDIR} to be created under ~/yocto-worker/poky-tiny/build

This patch [https://lists.yoctoproject.org/pipermail/yocto/2018-July/041685.html] which submitted today resolves the Step 1: Clobber build dir on autobuilder. 

Best wishes,
Aaron
________________________________________
From: richard.purdie@linuxfoundation.org [richard.purdie@linuxfoundation.org]
Sent: Tuesday, July 03, 2018 9:25 PM
To: Chan, Aaron Chun Yew; yocto@yoctoproject.org
Subject: Re: [PATCH] [yocto-ab-helper] utils.py: Resolved unicode data expansion

On Tue, 2018-07-03 at 09:44 +0800, Aaron Chan wrote:
> Updated patch to trigger handlestr() when unicode string is found
> during iteration json.loads(config.json). Unicode and list with data
> expansion were not handled hence adding this patch to handle
> conversion.
> Added a debug message to dump pretty json data populated to
> ourconfig[c].
>
> e.g "REPO_STASH_DIR" read as ${BASE_HOMEDIR}/git/mirror, where it
> should be
>     "REPO_STASH_DIR" as /home/pokybuild/git/mirror
>
> Signed-off-by: Aaron Chan <aaron.chun.yew.chan@intel.com>
> ---
>  scripts/utils.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

It took me a while to figure out why you were doing this.

We can't expand the data half way through loading the json file as
other pieces of data may later override the values. We therefore have
to defer expansion of variables until the file is completely loaded.

We therefore have to expand the variables later on, when we read them.

I pointed you at this commit:

http://git.yoctoproject.org/cgit/cgit.cgi/yocto-autobuilder-helper/commit/?id=d6253df2bc21752bc0b53202e491140b0994ff63

which changes direct accesses into ourconfig, e.g.:

ourconfig["REPO_STASH_DIR"]

into accesses using a function:

utils.getconfig("REPO_STASH_DIR", ourconfig)

and that function handles the expansion.

You should therefore be able to fix the clobberdir issue by using the
getconfig() method instead of direct access?

Cheers,

Richard

Re: [PATCH] [yocto-ab-helper] utils.py: Resolved unicode data expansion

[Richard Purdie]

On Tue, 2018-07-03 at 09:44 +0800, Aaron Chan wrote:
> Updated patch to trigger handlestr() when unicode string is found
> during iteration json.loads(config.json). Unicode and list with data
> expansion were not handled hence adding this patch to handle
> conversion.
> Added a debug message to dump pretty json data populated to
> ourconfig[c].
> 
> e.g "REPO_STASH_DIR" read as ${BASE_HOMEDIR}/git/mirror, where it
> should be
>     "REPO_STASH_DIR" as /home/pokybuild/git/mirror
> 
> Signed-off-by: Aaron Chan <aaron.chun.yew.chan@intel.com>
> ---
>  scripts/utils.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

It took me a while to figure out why you were doing this.

We can't expand the data half way through loading the json file as
other pieces of data may later override the values. We therefore have
to defer expansion of variables until the file is completely loaded.

We therefore have to expand the variables later on, when we read them.

I pointed you at this commit:

http://git.yoctoproject.org/cgit/cgit.cgi/yocto-autobuilder-helper/commit/?id=d6253df2bc21752bc0b53202e491140b0994ff63

which changes direct accesses into ourconfig, e.g.:

ourconfig["REPO_STASH_DIR"]

into accesses using a function:

utils.getconfig("REPO_STASH_DIR", ourconfig)

and that function handles the expansion.

You should therefore be able to fix the clobberdir issue by using the
getconfig() method instead of direct access?

Cheers,

Richard

[PATCH] [yocto-ab-helper] utils.py: Resolved unicode data expansion

[Aaron Chan]

Updated patch to trigger handlestr() when unicode string is found
during iteration json.loads(config.json). Unicode and list with data
expansion were not handled hence adding this patch to handle conversion.
Added a debug message to dump pretty json data populated to ourconfig[c].

e.g "REPO_STASH_DIR" read as ${BASE_HOMEDIR}/git/mirror, where it should be
    "REPO_STASH_DIR" as /home/pokybuild/git/mirror

Signed-off-by: Aaron Chan <aaron.chun.yew.chan@intel.com>
---
 scripts/utils.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/utils.py b/scripts/utils.py
index d26cd0c..32caa4f 100644
--- a/scripts/utils.py
+++ b/scripts/utils.py
@@ -152,11 +152,13 @@ def loadconfig():
         except:
             pass
 
-    def handlestr(config, ourconfig, c):
+    def handlestr(config, ourconfig, c, debug=False):
         if not c in ourconfig:
             ourconfig[c] = config[c]
         if isinstance(config[c], str):
             resolvexp(r"(\${(.+)})", config, c)
+        if debug:
+            print(json.dumps(ourconfig[c], indent=4, sort_keys=True))
         
     ourconfig = {}
     for f in files.split():
@@ -168,6 +170,8 @@ def loadconfig():
             for c in config:
                 if isinstance(config[c], dict):
                     handledict(config, ourconfig, c)
+                elif isinstance(config[c], str):
+                    handlestr(config, ourconfig, c)
                 else:
                     ourconfig[c] = config[c]
 
-- 
2.7.4