[Buildroot] [PATCH] pycompile: fix .pyc original source file paths

[Buildroot] [PATCH] pycompile: fix .pyc original source file paths

[Julien Floret]

When generating a .pyc file, the original .py source file path is
encoded in it. It is used for various purposes: traceback generation,
.pyc file comparison with its .py source, and code inspection.

By default, the source path used when invoking compileall is encoded in
the .pyc file. Since we use paths relative to TARGET_DIR, we end up with
paths that are only valid when relative to '/' encoded in the installed
.pyc files on the target.

This breaks code inspection at runtime since the original source path
will be invalid unless the code is executed from '/'.

Rework the script to call py_compile.compile() with pertinent options.

Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
Signed-off-by: Julien Floret <julien.floret@6wind.com>
---
 package/python/python.mk     |  1 -
 package/python3/python3.mk   |  1 -
 support/scripts/pycompile.py | 74 ++++++++++++++----------------------
 3 files changed, 29 insertions(+), 47 deletions(-)

diff --git a/package/python/python.mk b/package/python/python.mk
index ccaaadd012a5..67a7814a2d15 100644
--- a/package/python/python.mk
+++ b/package/python/python.mk
@@ -262,7 +262,6 @@ define PYTHON_CREATE_PYC_FILES
 	PYTHONPATH="$(PYTHON_PATH)" \
 	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
-		$(if $(BR2_REPRODUCIBLE),--force) \
 		usr/lib/python$(PYTHON_VERSION_MAJOR)
 endef
 
diff --git a/package/python3/python3.mk b/package/python3/python3.mk
index 31e7ca3d3af3..0749bfbe7ebf 100644
--- a/package/python3/python3.mk
+++ b/package/python3/python3.mk
@@ -279,7 +279,6 @@ define PYTHON3_CREATE_PYC_FILES
 	PYTHONPATH="$(PYTHON3_PATH)" \
 	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
-		$(if $(BR2_REPRODUCIBLE),--force) \
 		usr/lib/python$(PYTHON3_VERSION_MAJOR)
 endef
 
diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index 9192a7016a78..bfb82eac2750 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -9,61 +9,45 @@ Inspired from:
 from __future__ import print_function
 import sys
 import py_compile
-import compileall
 import argparse
+import os
+import re
 
 
-def check_for_errors(comparison):
-    '''Wrap comparison operator with code checking for PyCompileError.
-    If PyCompileError was raised, re-raise it again to abort execution,
-    otherwise perform comparison as expected.
-    '''
-    def operator(self, other):
-        exc_type, value, traceback = sys.exc_info()
-        if exc_type is not None and issubclass(exc_type,
-                                               py_compile.PyCompileError):
-            print("Cannot compile %s" % value.file)
-            raise value
-
-        return comparison(self, other)
-
-    return operator
+def is_importable_py_file(fpath):
+    if not fpath.endswith('.py'):
+        return False
+    if not (fpath.startswith('/usr/lib') or fpath.startswith('/usr/share')):
+        return False
+    return re.match(r'^[_A-Za-z][_A-Za-z0-9]+\.py$', os.path.basename(fpath))
 
 
-class ReportProblem(int):
-    '''Class that pretends to be an int() object but implements all of its
-    comparison operators such that it'd detect being called in
-    PyCompileError handling context and abort execution
-    '''
-    VALUE = 1
+def main():
+    parser = argparse.ArgumentParser(description='Compile Python source files in a directory tree.')
+    parser.add_argument("target", metavar='DIRECTORY',
+                        help='Directory to scan')
 
-    def __new__(cls, *args, **kwargs):
-        return int.__new__(cls, ReportProblem.VALUE, **kwargs)
+    args = parser.parse_args()
 
-    @check_for_errors
-    def __lt__(self, other):
-        return ReportProblem.VALUE < other
+    try:
 
-    @check_for_errors
-    def __eq__(self, other):
-        return ReportProblem.VALUE == other
+        for root, _, files in os.walk(args.target):
+            for f in files:
+                f = os.path.join(root, f)
+                if os.path.islink(f) or os.access(f, os.X_OK):
+                    continue
+                target = os.path.join('/', os.path.relpath(f, args.target))
+                if not is_importable_py_file(target):
+                    continue
 
-    def __ge__(self, other):
-        return not self < other
+                py_compile.compile(f, cfile=f + 'c', dfile=target, doraise=True)
 
-    def __gt__(self, other):
-        return not self < other and not self == other
+    except Exception as e:
+        print('error: %s' % e, file=sys.stderr)
+        return 1
 
-    def __ne__(self, other):
-        return not self == other
+    return 0
 
 
-parser = argparse.ArgumentParser(description='Compile Python source files in a directory tree.')
-parser.add_argument("target", metavar='DIRECTORY',
-                    help='Directory to scan')
-parser.add_argument("--force", action='store_true',
-                    help="Force compilation even if alread compiled")
-
-args = parser.parse_args()
-
-compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
+if __name__ == '__main__':
+    sys.exit(main())
-- 
2.25.1

[Buildroot] [PATCH] pycompile: fix .pyc original source file paths

[Yann E. MORIN]

Julien, All,

On 2020-09-04 13:29 +0200, Julien Floret spake thusly:
> When generating a .pyc file, the original .py source file path is
> encoded in it. It is used for various purposes: traceback generation,
> .pyc file comparison with its .py source, and code inspection.
> 
> By default, the source path used when invoking compileall is encoded in
> the .pyc file. Since we use paths relative to TARGET_DIR, we end up with
> paths that are only valid when relative to '/' encoded in the installed
> .pyc files on the target.
> 
> This breaks code inspection at runtime since the original source path
> will be invalid unless the code is executed from '/'.

Thanks for the good description of the problem, I think I mostly understood
it, so let me rephrase just in case:

  - we have e.g.: /path/to/Buildroot/output/target/usr/lib/python3.5/foo.py

  - when pre-compiled it generates foo.pyc, which contains a reference
    to 'usr/lib/python3.5/foo.py' verbatim.

Right? Or did I forget a leading '/' missing in the verbatim?

> Rework the script to call py_compile.compile() with pertinent options.

You did provide a good description of the problem, so you should not
stop there. It would have been even btter if you had also provided a
good explanations of how you are fixing it! ;-)

Do not just _describe_ the patch, _explain_ it. For example:

    The issues stems from the fact that py_compile.compile() will default
    to store the path as we pass it, so we override that by explicitly
    passing the path at it is on the target.

    We can't do that with compileall.compile_dir(), so instead we
    do the recursion and call py_compile.compile() on fitting files
    ourselves.

(this is a mostly made up explanations; adapt or rewrite appropriately)

Also, see some comments in-line below.

> Signed-off-by: Julien Floret <julien.floret@6wind.com>
> ---
[--SNIP--]
> diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
> index 9192a7016a78..bfb82eac2750 100644
> --- a/support/scripts/pycompile.py
> +++ b/support/scripts/pycompile.py
> @@ -9,61 +9,45 @@ Inspired from:
>  from __future__ import print_function
>  import sys
>  import py_compile
> -import compileall
>  import argparse
> +import os
> +import re
>  
>  
> -def check_for_errors(comparison):
> -    '''Wrap comparison operator with code checking for PyCompileError.
> -    If PyCompileError was raised, re-raise it again to abort execution,
> -    otherwise perform comparison as expected.
> -    '''
> -    def operator(self, other):
> -        exc_type, value, traceback = sys.exc_info()
> -        if exc_type is not None and issubclass(exc_type,
> -                                               py_compile.PyCompileError):
> -            print("Cannot compile %s" % value.file)
> -            raise value
> -
> -        return comparison(self, other)
> -
> -    return operator
> +def is_importable_py_file(fpath):
> +    if not fpath.endswith('.py'):
> +        return False
> +    if not (fpath.startswith('/usr/lib') or fpath.startswith('/usr/share')):

Actually, some people are building out of /usr/share/buildroot for
example, so this would effectively ignore all .py files that were
installed, and none would be compiled.

However, you know that the CWD is $(TARGET_DIR) because that's were we
cd into before running the script. So maybe you want to change this
condition to (to be tested):

    if not fpath.startswith(os.getcwd()):
        return False

> +        return False
> +    return re.match(r'^[_A-Za-z][_A-Za-z0-9]+\.py$', os.path.basename(fpath))
>  
>  
> -class ReportProblem(int):
> -    '''Class that pretends to be an int() object but implements all of its
> -    comparison operators such that it'd detect being called in
> -    PyCompileError handling context and abort execution
> -    '''
> -    VALUE = 1

You should also explain why you get rid of the ReportProblem class, and
all the error handling that is currently in place.

> +def main():

I like that you introduce a main(), thanks! :-)

However, this is mixing code-style reformatting and actual code fixups,
which makes it a bit more difficult to analyse and review.

Could you please split it into two patches;

  - code cleanups to introduce main()

  - code fixups to properly store the .py path

Otherwise, I think this looks nice, thanks!

Regards,
Yann E. MORIN.

> +    parser = argparse.ArgumentParser(description='Compile Python source files in a directory tree.')
> +    parser.add_argument("target", metavar='DIRECTORY',
> +                        help='Directory to scan')
>  
> -    def __new__(cls, *args, **kwargs):
> -        return int.__new__(cls, ReportProblem.VALUE, **kwargs)
> +    args = parser.parse_args()
>  
> -    @check_for_errors
> -    def __lt__(self, other):
> -        return ReportProblem.VALUE < other
> +    try:
>  
> -    @check_for_errors
> -    def __eq__(self, other):
> -        return ReportProblem.VALUE == other
> +        for root, _, files in os.walk(args.target):
> +            for f in files:
> +                f = os.path.join(root, f)
> +                if os.path.islink(f) or os.access(f, os.X_OK):
> +                    continue
> +                target = os.path.join('/', os.path.relpath(f, args.target))
> +                if not is_importable_py_file(target):
> +                    continue
>  
> -    def __ge__(self, other):
> -        return not self < other
> +                py_compile.compile(f, cfile=f + 'c', dfile=target, doraise=True)
>  
> -    def __gt__(self, other):
> -        return not self < other and not self == other
> +    except Exception as e:
> +        print('error: %s' % e, file=sys.stderr)
> +        return 1
>  
> -    def __ne__(self, other):
> -        return not self == other
> +    return 0
>  
>  
> -parser = argparse.ArgumentParser(description='Compile Python source files in a directory tree.')
> -parser.add_argument("target", metavar='DIRECTORY',
> -                    help='Directory to scan')
> -parser.add_argument("--force", action='store_true',
> -                    help="Force compilation even if alread compiled")
> -
> -args = parser.parse_args()
> -
> -compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
> +if __name__ == '__main__':
> +    sys.exit(main())
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] pycompile: fix .pyc original source file paths

[Yann E. MORIN]

Julien, All,

On 2020-09-04 23:26 +0200, Yann E. MORIN spake thusly:
> On 2020-09-04 13:29 +0200, Julien Floret spake thusly:
> > When generating a .pyc file, the original .py source file path is
> > encoded in it. It is used for various purposes: traceback generation,
> > .pyc file comparison with its .py source, and code inspection.

One more comment I forgot to add:

> > +def is_importable_py_file(fpath):
> > +    if not fpath.endswith('.py'):
> > +        return False

Why do you have this condiiton here...

> > +    return re.match(r'^[_A-Za-z][_A-Za-z0-9]+\.py$', os.path.basename(fpath))

When you already test here that file ends with '.py' ?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 0/4] pycompile: fix .pyc source paths + improvements

[Robin Jarry]

Hi all,

This series is about fixing the original .py source paths encoded into
.pyc files. It comes with small improvements on pycompile.py as well.

The first version of the patch was actually not working at all... No
.pyc file was compiled. We did not see it because
BR2_PACKAGE_PYTHON*_PYC_ONLY were not selected and therefore, the .pyc
files were generated automatically at runtime.

This v2 should work properly.

Robin Jarry (4):
  pycompile: add main entry point
  pycompile: sort imports
  pycompile: fix .pyc original source file paths
  pycompile: add --verbose option

 package/python/python.mk     |   6 +-
 package/python3/python3.mk   |   6 +-
 support/scripts/pycompile.py | 158 +++++++++++++++++++++--------------
 3 files changed, 104 insertions(+), 66 deletions(-)

Differences with v1:

  * split in multiple commits
  * fixed the script itself
  * sorted imports at the top of the file
  * added new --verbose option

-- 
2.28.0

[Buildroot] [PATCH v2 1/4] pycompile: add main entry point

[Robin Jarry]

Only run code when the script is executed directly (not imported).
Factorize command description by using the script's __doc__ variable.
Fix typo in --force help message.

Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
---
 support/scripts/pycompile.py | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index 9192a7016a78..fb1a12b2b782 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -58,12 +58,19 @@ class ReportProblem(int):
         return not self == other
 
 
-parser = argparse.ArgumentParser(description='Compile Python source files in a directory tree.')
-parser.add_argument("target", metavar='DIRECTORY',
-                    help='Directory to scan')
-parser.add_argument("--force", action='store_true',
-                    help="Force compilation even if alread compiled")
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("target", metavar="TARGET",
+                        help="Directory to scan")
+    parser.add_argument("--force", action="store_true",
+                        help="Force compilation even if already compiled")
 
-args = parser.parse_args()
+    args = parser.parse_args()
 
-compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
+    compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
+
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())
-- 
2.28.0

[Buildroot] [PATCH v2 2/4] pycompile: sort imports

[Robin Jarry]

Sort imports alphabetically.

Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
---
 support/scripts/pycompile.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index fb1a12b2b782..b713fe19323c 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -6,11 +6,13 @@ when a python byte code generation failed.
 Inspired from:
    http://stackoverflow.com/questions/615632/how-to-detect-errors-from-compileall-compile-dir
 '''
+
 from __future__ import print_function
-import sys
-import py_compile
-import compileall
+
 import argparse
+import compileall
+import py_compile
+import sys
 
 
 def check_for_errors(comparison):
-- 
2.28.0

[Buildroot] [PATCH v2 3/4] pycompile: fix .pyc original source file paths

[Robin Jarry]

When generating a .pyc file, the original .py source file path is
encoded in it. It is used for various purposes: traceback generation,
.pyc file comparison with its .py source, and code inspection.

By default, the source path used when invoking compileall is encoded in
the .pyc file. Since we use paths relative to TARGET_DIR, we end up with
paths that are only valid when relative to '/' encoded in the installed
.pyc files on the target.

This breaks code inspection at runtime since the original source path
will be invalid unless the code is executed from '/'.

Unfortunately, compileall cannot be forced to use the proper path. It
was not written with cross-compilation usage in mind.

Rework the script to call py_compile.compile() with pertinent options:

- The script is now called with $(TARGET_DIR) as first argument. This is
  the future runtime /.
- All other (non-optional) arguments are folders in which all
  "importable" .py files will be compiled to .pyc.
- Using the $(TARGET_DIR), the future runtime path of each .py file is
  computed and encoded into the compiled .pyc.

No need to change directory before running the script anymore.

The trickery used to handle error reporting was only applicable with
compileall. Since we implement our own "compileall", error reporting
becomes trivial.

Signed-off-by: Julien Floret <julien.floret@6wind.com>
Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
---
 package/python/python.mk     |   5 +-
 package/python3/python3.mk   |   5 +-
 support/scripts/pycompile.py | 122 ++++++++++++++++++++---------------
 3 files changed, 76 insertions(+), 56 deletions(-)

diff --git a/package/python/python.mk b/package/python/python.mk
index ccaaadd012a5..7000658330e8 100644
--- a/package/python/python.mk
+++ b/package/python/python.mk
@@ -260,10 +260,11 @@ endif
 define PYTHON_CREATE_PYC_FILES
 	$(PYTHON_FIX_TIME)
 	PYTHONPATH="$(PYTHON_PATH)" \
-	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
+	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
-		usr/lib/python$(PYTHON_VERSION_MAJOR)
+		$(TARGET_DIR) \
+		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
 endef
 
 ifeq ($(BR2_PACKAGE_PYTHON_PYC_ONLY)$(BR2_PACKAGE_PYTHON_PY_PYC),y)
diff --git a/package/python3/python3.mk b/package/python3/python3.mk
index 31e7ca3d3af3..e8c200b2081e 100644
--- a/package/python3/python3.mk
+++ b/package/python3/python3.mk
@@ -277,10 +277,11 @@ endif
 define PYTHON3_CREATE_PYC_FILES
 	$(PYTHON3_FIX_TIME)
 	PYTHONPATH="$(PYTHON3_PATH)" \
-	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
+	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
-		usr/lib/python$(PYTHON3_VERSION_MAJOR)
+		$(TARGET_DIR) \
+		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)
 endef
 
 ifeq ($(BR2_PACKAGE_PYTHON3_PYC_ONLY)$(BR2_PACKAGE_PYTHON3_PY_PYC),y)
diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index b713fe19323c..c60d4e13f3b4 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -1,75 +1,93 @@
 #!/usr/bin/env python
 
-'''Wrapper for python2 and python3 around compileall to raise exception
-when a python byte code generation failed.
-
-Inspired from:
-   http://stackoverflow.com/questions/615632/how-to-detect-errors-from-compileall-compile-dir
-'''
+"""
+Byte compile all .py files from a target dir and encode a proper source runtime
+path into them.
+"""
 
 from __future__ import print_function
 
 import argparse
-import compileall
+import importlib
+import os
 import py_compile
+import re
+import struct
 import sys
 
 
-def check_for_errors(comparison):
-    '''Wrap comparison operator with code checking for PyCompileError.
-    If PyCompileError was raised, re-raise it again to abort execution,
-    otherwise perform comparison as expected.
-    '''
-    def operator(self, other):
-        exc_type, value, traceback = sys.exc_info()
-        if exc_type is not None and issubclass(exc_type,
-                                               py_compile.PyCompileError):
-            print("Cannot compile %s" % value.file)
-            raise value
-
-        return comparison(self, other)
-
-    return operator
-
-
-class ReportProblem(int):
-    '''Class that pretends to be an int() object but implements all of its
-    comparison operators such that it'd detect being called in
-    PyCompileError handling context and abort execution
-    '''
-    VALUE = 1
-
-    def __new__(cls, *args, **kwargs):
-        return int.__new__(cls, ReportProblem.VALUE, **kwargs)
-
-    @check_for_errors
-    def __lt__(self, other):
-        return ReportProblem.VALUE < other
-
-    @check_for_errors
-    def __eq__(self, other):
-        return ReportProblem.VALUE == other
-
-    def __ge__(self, other):
-        return not self < other
-
-    def __gt__(self, other):
-        return not self < other and not self == other
-
-    def __ne__(self, other):
-        return not self == other
+def compile_one(host_path, target_root, force=False):
+    """
+    Compile a .py file into a .pyc file located next to it.
+
+    :arg host_path:
+        Absolute path to the file to compile on the host running the build.
+    :arg target_root:
+        Absolute path of the target dir (i.e. the future /).
+    :arg force:
+        Recompile even if already up-to-date.
+    """
+    if os.path.islink(host_path):
+        return  # only compile real files
+
+    if not re.match(r"^[_A-Za-z][_A-Za-z0-9]+\.py$",
+                    os.path.basename(host_path)):
+        return  # only compile "importable" python modules
+
+    if not force:
+        # inspired from compileall.compile_file in the standard library
+        try:
+            with open(host_path + "c", "rb") as f:
+                header = f.read(12)
+            expect = struct.pack("<4sll", importlib.util.MAGIC_NUMBER,
+                                 0, int(os.stat(host_path).st_mtime))
+            if header == expect:
+                return  # .pyc file already up to date.
+        except OSError:
+            pass  # .pyc file does not exist
+
+    # determine the runtime path of the file (i.e.: relative path to target
+    # root dir prepended with "/").
+    runtime_path = os.path.join("/", os.path.relpath(host_path, target_root))
+
+    # will raise an error if the file cannot be compiled
+    py_compile.compile(host_path, cfile=host_path + "c",
+                       dfile=runtime_path, doraise=True)
 
 
 def main():
     parser = argparse.ArgumentParser(description=__doc__)
     parser.add_argument("target", metavar="TARGET",
-                        help="Directory to scan")
+                        help="Path on the build host that will be /@runtime")
+    parser.add_argument("dirs", metavar="PATH", nargs="+",
+                        help="Folder located inside TARGET to scan and compile")
     parser.add_argument("--force", action="store_true",
                         help="Force compilation even if already compiled")
 
     args = parser.parse_args()
 
-    compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
+    if not os.path.isdir(args.target):
+        parser.error("TARGET: no such directory: %r" % args.target)
+
+    # only work with absolute paths
+    args.target = os.path.abspath(args.target)
+
+    try:
+        # Make sure that all scanned dirs do exist and that they are located
+        # *inside* the target dir.
+        for d in args.dirs:
+            if not os.path.isdir(d):
+                parser.error("PATH: no such directory: %r" % d)
+            d = os.path.abspath(d)
+            if ".." in os.path.relpath(d, args.target):
+                parser.error("PATH: not inside TARGET dir: %r" % d)
+            for parent, _, files in os.walk(d):
+                for f in files:
+                    compile_one(os.path.join(parent, f), args.target, args.force)
+
+    except Exception as e:
+        print("error: %s" % e, file=sys.stderr)
+        return 1
 
     return 0
 
-- 
2.28.0

[Buildroot] [PATCH v2 3/4] pycompile: fix .pyc original source file paths

[Yann E. MORIN]

Robin, All,

On 2020-09-08 10:10 +0200, Robin Jarry spake thusly:
> When generating a .pyc file, the original .py source file path is
> encoded in it. It is used for various purposes: traceback generation,
> .pyc file comparison with its .py source, and code inspection.
> 
> By default, the source path used when invoking compileall is encoded in
> the .pyc file. Since we use paths relative to TARGET_DIR, we end up with
> paths that are only valid when relative to '/' encoded in the installed
> .pyc files on the target.
> 
> This breaks code inspection at runtime since the original source path
> will be invalid unless the code is executed from '/'.
> 
> Unfortunately, compileall cannot be forced to use the proper path. It
> was not written with cross-compilation usage in mind.
> 
> Rework the script to call py_compile.compile() with pertinent options:
> 
> - The script is now called with $(TARGET_DIR) as first argument. This is
>   the future runtime /.
> - All other (non-optional) arguments are folders in which all
>   "importable" .py files will be compiled to .pyc.
> - Using the $(TARGET_DIR), the future runtime path of each .py file is
>   computed and encoded into the compiled .pyc.
> 
> No need to change directory before running the script anymore.
> 
> The trickery used to handle error reporting was only applicable with
> compileall. Since we implement our own "compileall", error reporting
> becomes trivial.

Thanks very much for this extended commit log; good for me now.

However, I have now some more feedback to provide, now that I can more
clearly see the delta. ;-)

See below...

> Signed-off-by: Julien Floret <julien.floret@6wind.com>
> Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
> ---
>  package/python/python.mk     |   5 +-
>  package/python3/python3.mk   |   5 +-
>  support/scripts/pycompile.py | 122 ++++++++++++++++++++---------------
>  3 files changed, 76 insertions(+), 56 deletions(-)
> 
> diff --git a/package/python/python.mk b/package/python/python.mk
> index ccaaadd012a5..7000658330e8 100644
> --- a/package/python/python.mk
> +++ b/package/python/python.mk
> @@ -260,10 +260,11 @@ endif
>  define PYTHON_CREATE_PYC_FILES
>  	$(PYTHON_FIX_TIME)
>  	PYTHONPATH="$(PYTHON_PATH)" \
> -	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
> +	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
>  		$(TOPDIR)/support/scripts/pycompile.py \
>  		$(if $(BR2_REPRODUCIBLE),--force) \
> -		usr/lib/python$(PYTHON_VERSION_MAJOR)
> +		$(TARGET_DIR) \
> +		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
>  endef
>  
>  ifeq ($(BR2_PACKAGE_PYTHON_PYC_ONLY)$(BR2_PACKAGE_PYTHON_PY_PYC),y)
> diff --git a/package/python3/python3.mk b/package/python3/python3.mk
> index 31e7ca3d3af3..e8c200b2081e 100644
> --- a/package/python3/python3.mk
> +++ b/package/python3/python3.mk
> @@ -277,10 +277,11 @@ endif
>  define PYTHON3_CREATE_PYC_FILES
>  	$(PYTHON3_FIX_TIME)
>  	PYTHONPATH="$(PYTHON3_PATH)" \
> -	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
> +	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
>  		$(TOPDIR)/support/scripts/pycompile.py \
>  		$(if $(BR2_REPRODUCIBLE),--force) \
> -		usr/lib/python$(PYTHON3_VERSION_MAJOR)
> +		$(TARGET_DIR) \
> +		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)

So, when I was trying this script manually, it failed to work:

    $ mkdir -p test-target/foo/bar
    $ cp target/usr/lib/python*/*.py test-target/foo/bar
    $ python support/scripts/pycompile.py $(pwd)/test-target /foo/bar
    pycompile.py: error: PATH: no such directory: '/foo/bar'

That's because the PATH args must be absolute *on the host*, they are
not relatve to TARGET. But this is not very clear from the help text:

    PATH        Folder located inside TARGET to scan and compile

See below for more on this...

>  endef
>  
>  ifeq ($(BR2_PACKAGE_PYTHON3_PYC_ONLY)$(BR2_PACKAGE_PYTHON3_PY_PYC),y)
> diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
> index b713fe19323c..c60d4e13f3b4 100644
> --- a/support/scripts/pycompile.py
> +++ b/support/scripts/pycompile.py
> @@ -1,75 +1,93 @@
>  #!/usr/bin/env python
>  
> -'''Wrapper for python2 and python3 around compileall to raise exception
> -when a python byte code generation failed.
> -
> -Inspired from:
> -   http://stackoverflow.com/questions/615632/how-to-detect-errors-from-compileall-compile-dir
> -'''
> +"""
> +Byte compile all .py files from a target dir and encode a proper source runtime
> +path into them.
> +"""
>  
>  from __future__ import print_function
>  
>  import argparse
> -import compileall
> +import importlib
> +import os
>  import py_compile
> +import re
> +import struct
>  import sys
>  
>  
> -def check_for_errors(comparison):
> -    '''Wrap comparison operator with code checking for PyCompileError.
> -    If PyCompileError was raised, re-raise it again to abort execution,
> -    otherwise perform comparison as expected.
> -    '''
> -    def operator(self, other):
> -        exc_type, value, traceback = sys.exc_info()
> -        if exc_type is not None and issubclass(exc_type,
> -                                               py_compile.PyCompileError):
> -            print("Cannot compile %s" % value.file)

I am not python expert, but I tend to be using '{}'.format(..), and
indeed, it seems that python considers the % formatting to be "old":

    https://docs.python.org/3/tutorial/inputoutput.html#old-string-formatting

Do you think you could switch to using str.format() ?

[--SNIP--]
>  def main():
>      parser = argparse.ArgumentParser(description=__doc__)
>      parser.add_argument("target", metavar="TARGET",
> -                        help="Directory to scan")
> +                        help="Path on the build host that will be / at runtime")
> +    parser.add_argument("dirs", metavar="PATH", nargs="+",
> +                        help="Folder located inside TARGET to scan and compile")
>      parser.add_argument("--force", action="store_true",
>                          help="Force compilation even if already compiled")


So, back to this TARGET and PATH arguments. I find it pretty confusing
the way they are described.

First, what TARGET used to be in the previous script is now something
else, so keeping the naming is confusing. Also, 'target/' is a buildroot
naming; I think we could get a better generic name: root

As for PATH, I find it strange that the metavar and variable do not
match.

    parser.add_argument('root', metavar='ROOT',
                        help='Path on the build machine that will be / at runtime')
    parser.add_argument('dirs', metavar='DIR',
                        help='Directory, relative to ROOT, to scan for py-files to compile')

So now, it should be possible to use as thus:

    python support/scripts/pycompile.py \
        $(TARGET_DIR) \
        usr/lib/python$(PYTHON_VERSION_MAJOR)

Or even (which I like better, because the DIR is absolute as seen at
runtime too):

    python support/scripts/pycompile.py \
        $(TARGET_DIR) \
        /usr/lib/python$(PYTHON_VERSION_MAJOR)

But of course, dealing with path concatenation with an intermediate path
that starts with a '/' is always tricky (I don't understand why upstream
went that route...).

    for d in args.dirs:
        abs = os.path.join(args.root, d[(1 if d[0] == '/' else 0):])

Thoughts?

Thanks for the good rework so far. :-)

Regards,
Yann E. MORIN.

>      args = parser.parse_args()
>  
> -    compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
> +    if not os.path.isdir(args.target):
> +        parser.error("TARGET: no such directory: %r" % args.target)
> +
> +    # only work with absolute paths
> +    args.target = os.path.abspath(args.target)
> +
> +    try:
> +        # Make sure that all scanned dirs do exist and that they are located
> +        # *inside* the target dir.
> +        for d in args.dirs:
> +            if not os.path.isdir(d):
> +                parser.error("PATH: no such directory: %r" % d)
> +            d = os.path.abspath(d)
> +            if ".." in os.path.relpath(d, args.target):
> +                parser.error("PATH: not inside TARGET dir: %r" % d)
> +            for parent, _, files in os.walk(d):
> +                for f in files:
> +                    compile_one(os.path.join(parent, f), args.target, args.force)
> +
> +    except Exception as e:
> +        print("error: %s" % e, file=sys.stderr)
> +        return 1
>  
>      return 0
>  
> -- 
> 2.28.0
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 3/4] pycompile: fix .pyc original source file paths

[Robin Jarry]

2020-09-09, Yann E. MORIN:
[snip]
> I am not python expert, but I tend to be using '{}'.format(..), and
> indeed, it seems that python considers the % formatting to be "old":
> 
>     https://docs.python.org/3/tutorial/inputoutput.html#old-string-formatting
> 
> Do you think you could switch to using str.format() ?

Sure, I have no strong opinion on the matter. I am more used to old
formatting since it is often shorter, but the new str.format() is less
ambiguous. I'll do that in a v3 today.

> So, back to this TARGET and PATH arguments. I find it pretty confusing
> the way they are described.
> 
> First, what TARGET used to be in the previous script is now something
> else, so keeping the naming is confusing. Also, 'target/' is a buildroot
> naming; I think we could get a better generic name: root
> 
> As for PATH, I find it strange that the metavar and variable do not
> match.
> 
>     parser.add_argument('root', metavar='ROOT',
>                         help='Path on the build machine that will be / at runtime')
>     parser.add_argument('dirs', metavar='DIR',
>                         help='Directory, relative to ROOT, to scan for py-files to compile')
> 
> So now, it should be possible to use as thus:
> 
>     python support/scripts/pycompile.py \
>         $(TARGET_DIR) \
>         usr/lib/python$(PYTHON_VERSION_MAJOR)
> 
> Or even (which I like better, because the DIR is absolute as seen at
> runtime too):
> 
>     python support/scripts/pycompile.py \
>         $(TARGET_DIR) \
>         /usr/lib/python$(PYTHON_VERSION_MAJOR)
> 
> But of course, dealing with path concatenation with an intermediate path
> that starts with a '/' is always tricky (I don't understand why upstream
> went that route...).
> 
>     for d in args.dirs:
>         abs = os.path.join(args.root, d[(1 if d[0] == '/' else 0):])
> 
> Thoughts?

I have another suggestion: changing the current TARGET argument into
a --strip-root "optional" argument (which will always be specified
anyway, but if not cross compiling, it can make sense not to use it).
It seems explicit enough to me and it avoids confusion with runtime/host
paths. What do you think?

I'll submit this in a v3 as well so we can discuss on something
concrete.

Thanks for the thorough review.

-- 
Robin

[Buildroot] [PATCH v2 4/4] pycompile: add --verbose option

[Robin Jarry]

Add a new option that prints the (runtime) path of compiled .py files
when VERBOSE=1 is set.

Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
---
 package/python/python.mk     |  1 +
 package/python3/python3.mk   |  1 +
 support/scripts/pycompile.py | 11 +++++++++--
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/package/python/python.mk b/package/python/python.mk
index 7000658330e8..50e77db74628 100644
--- a/package/python/python.mk
+++ b/package/python/python.mk
@@ -263,6 +263,7 @@ define PYTHON_CREATE_PYC_FILES
 	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
+		$(if $(VERBOSE),--verbose) \
 		$(TARGET_DIR) \
 		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
 endef
diff --git a/package/python3/python3.mk b/package/python3/python3.mk
index e8c200b2081e..1ef757c6b66f 100644
--- a/package/python3/python3.mk
+++ b/package/python3/python3.mk
@@ -280,6 +280,7 @@ define PYTHON3_CREATE_PYC_FILES
 	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
+		$(if $(VERBOSE),--verbose) \
 		$(TARGET_DIR) \
 		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)
 endef
diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index c60d4e13f3b4..cb2ba7f5016e 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -16,7 +16,7 @@ import struct
 import sys
 
 
-def compile_one(host_path, target_root, force=False):
+def compile_one(host_path, target_root, force=False, verbose=False):
     """
     Compile a .py file into a .pyc file located next to it.
 
@@ -26,6 +26,8 @@ def compile_one(host_path, target_root, force=False):
         Absolute path of the target dir (i.e. the future /).
     :arg force:
         Recompile even if already up-to-date.
+    :arg verbose:
+        Print compiled file paths.
     """
     if os.path.islink(host_path):
         return  # only compile real files
@@ -49,6 +51,8 @@ def compile_one(host_path, target_root, force=False):
     # determine the runtime path of the file (i.e.: relative path to target
     # root dir prepended with "/").
     runtime_path = os.path.join("/", os.path.relpath(host_path, target_root))
+    if verbose:
+        print("  PYC  %s" % (runtime_path,))
 
     # will raise an error if the file cannot be compiled
     py_compile.compile(host_path, cfile=host_path + "c",
@@ -63,6 +67,8 @@ def main():
                         help="Folder located inside TARGET to scan and compile")
     parser.add_argument("--force", action="store_true",
                         help="Force compilation even if already compiled")
+    parser.add_argument("--verbose", action="store_true",
+                        help="Print compiled files")
 
     args = parser.parse_args()
 
@@ -83,7 +89,8 @@ def main():
                 parser.error("PATH: not inside TARGET dir: %r" % d)
             for parent, _, files in os.walk(d):
                 for f in files:
-                    compile_one(os.path.join(parent, f), args.target, args.force)
+                    compile_one(os.path.join(parent, f), args.target,
+                                args.force, args.verbose)
 
     except Exception as e:
         print("error: %s" % e, file=sys.stderr)
-- 
2.28.0

[Buildroot] [PATCH v2 4/4] pycompile: add --verbose option

[Yann E. MORIN]

Robin, All,

On 2020-09-08 10:10 +0200, Robin Jarry spake thusly:
> Add a new option that prints the (runtime) path of compiled .py files
> when VERBOSE=1 is set.

I said in my reply to the cover letter that I had something to say about
that patch, but in fact no; it's pretty OK.

Just do not forget to switch to using str.format() instead of using the
old-style % formatting. ;-)

Regards,
Yann E. MORIN.

> Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
> ---
>  package/python/python.mk     |  1 +
>  package/python3/python3.mk   |  1 +
>  support/scripts/pycompile.py | 11 +++++++++--
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/package/python/python.mk b/package/python/python.mk
> index 7000658330e8..50e77db74628 100644
> --- a/package/python/python.mk
> +++ b/package/python/python.mk
> @@ -263,6 +263,7 @@ define PYTHON_CREATE_PYC_FILES
>  	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
>  		$(TOPDIR)/support/scripts/pycompile.py \
>  		$(if $(BR2_REPRODUCIBLE),--force) \
> +		$(if $(VERBOSE),--verbose) \
>  		$(TARGET_DIR) \
>  		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
>  endef
> diff --git a/package/python3/python3.mk b/package/python3/python3.mk
> index e8c200b2081e..1ef757c6b66f 100644
> --- a/package/python3/python3.mk
> +++ b/package/python3/python3.mk
> @@ -280,6 +280,7 @@ define PYTHON3_CREATE_PYC_FILES
>  	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
>  		$(TOPDIR)/support/scripts/pycompile.py \
>  		$(if $(BR2_REPRODUCIBLE),--force) \
> +		$(if $(VERBOSE),--verbose) \
>  		$(TARGET_DIR) \
>  		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)
>  endef
> diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
> index c60d4e13f3b4..cb2ba7f5016e 100644
> --- a/support/scripts/pycompile.py
> +++ b/support/scripts/pycompile.py
> @@ -16,7 +16,7 @@ import struct
>  import sys
>  
>  
> -def compile_one(host_path, target_root, force=False):
> +def compile_one(host_path, target_root, force=False, verbose=False):
>      """
>      Compile a .py file into a .pyc file located next to it.
>  
> @@ -26,6 +26,8 @@ def compile_one(host_path, target_root, force=False):
>          Absolute path of the target dir (i.e. the future /).
>      :arg force:
>          Recompile even if already up-to-date.
> +    :arg verbose:
> +        Print compiled file paths.
>      """
>      if os.path.islink(host_path):
>          return  # only compile real files
> @@ -49,6 +51,8 @@ def compile_one(host_path, target_root, force=False):
>      # determine the runtime path of the file (i.e.: relative path to target
>      # root dir prepended with "/").
>      runtime_path = os.path.join("/", os.path.relpath(host_path, target_root))
> +    if verbose:
> +        print("  PYC  %s" % (runtime_path,))
>  
>      # will raise an error if the file cannot be compiled
>      py_compile.compile(host_path, cfile=host_path + "c",
> @@ -63,6 +67,8 @@ def main():
>                          help="Folder located inside TARGET to scan and compile")
>      parser.add_argument("--force", action="store_true",
>                          help="Force compilation even if already compiled")
> +    parser.add_argument("--verbose", action="store_true",
> +                        help="Print compiled files")
>  
>      args = parser.parse_args()
>  
> @@ -83,7 +89,8 @@ def main():
>                  parser.error("PATH: not inside TARGET dir: %r" % d)
>              for parent, _, files in os.walk(d):
>                  for f in files:
> -                    compile_one(os.path.join(parent, f), args.target, args.force)
> +                    compile_one(os.path.join(parent, f), args.target,
> +                                args.force, args.verbose)
>  
>      except Exception as e:
>          print("error: %s" % e, file=sys.stderr)
> -- 
> 2.28.0
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2 0/4] pycompile: fix .pyc source paths + improvements

[Yann E. MORIN]

Robin, All,

On 2020-09-08 10:10 +0200, Robin Jarry spake thusly:
> This series is about fixing the original .py source paths encoded into
> .pyc files. It comes with small improvements on pycompile.py as well.

Thanks for this new iteration.

> The first version of the patch was actually not working at all... No
> .pyc file was compiled. We did not see it because
> BR2_PACKAGE_PYTHON*_PYC_ONLY were not selected and therefore, the .pyc
> files were generated automatically at runtime.
> 
> This v2 should work properly.
> 
> Robin Jarry (4):
>   pycompile: add main entry point
>   pycompile: sort imports

I've applied those two ones.

>   pycompile: fix .pyc original source file paths
>   pycompile: add --verbose option

I have some comments about those two, so I'll reply separately.

Regards,
Yann E. MORIN.

> 
>  package/python/python.mk     |   6 +-
>  package/python3/python3.mk   |   6 +-
>  support/scripts/pycompile.py | 158 +++++++++++++++++++++--------------
>  3 files changed, 104 insertions(+), 66 deletions(-)
> 
> Differences with v1:
> 
>   * split in multiple commits
>   * fixed the script itself
>   * sorted imports at the top of the file
>   * added new --verbose option
> 
> -- 
> 2.28.0
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v3 0/2] pycompile: fix .pyc source paths + improvements

[Robin Jarry]

This series is about fixing the original .py source paths encoded into
.pyc files. It comes with small improvements on pycompile.py as well.

The first two commits of v2 have been applied already, I am sending the
last two which needed some rework.

Robin Jarry (2):
  support/scripts/pycompile: fix .pyc original source file paths
  support/scripts/pycompile: add --verbose option

 package/python/python.mk     |   6 +-
 package/python3/python3.mk   |   6 +-
 support/scripts/pycompile.py | 139 ++++++++++++++++++++++-------------
 3 files changed, 94 insertions(+), 57 deletions(-)

Differences since v2:

  * Used str.format() instead of % format.
  * Renamed the TARGET positional argument into --strip-root which is
    more explicit. The script is called with --strip-root=$(TARGET_DIR).
  * Homogenized the arguments metavars.
  * Used argparse type callbacks for directory arguments checking.

-- 
2.28.0

[Buildroot] [PATCH v3 1/2] support/scripts/pycompile: fix .pyc original source file paths

[Robin Jarry]

When generating a .pyc file, the original .py source file path is
encoded in it. It is used for various purposes: traceback generation,
.pyc file comparison with its .py source, and code inspection.

By default, the source path used when invoking compileall is encoded in
the .pyc file. Since we use paths relative to TARGET_DIR, we end up with
paths that are only valid when relative to '/' encoded in the installed
.pyc files on the target.

This breaks code inspection at runtime since the original source path
will be invalid unless the code is executed from '/'.

Unfortunately, compileall cannot be forced to use the proper path. It
was not written with cross-compilation usage in mind.

Rework the script to call py_compile.compile() directly with pertinent
options:

- The script now has a new --strip-root argument. This argument is
  optional but will always be specified when compiling py files in
  buildroot.
- All other (non-optional) arguments are folders in which all
  "importable" .py files will be compiled to .pyc.
- Using --strip-root=$(TARGET_DIR), the future runtime path of each .py
  file is computed and encoded into the compiled .pyc.

No need to change directory before running the script anymore.

The trickery used to handle error reporting was only applicable with
compileall. Since we implement our own "compileall", error reporting
becomes trivial.

Signed-off-by: Julien Floret <julien.floret@6wind.com>
Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
---
 package/python/python.mk     |   5 +-
 package/python3/python3.mk   |   5 +-
 support/scripts/pycompile.py | 132 +++++++++++++++++++++--------------
 3 files changed, 85 insertions(+), 57 deletions(-)

diff --git a/package/python/python.mk b/package/python/python.mk
index ccaaadd012a5..3fe5ecd00462 100644
--- a/package/python/python.mk
+++ b/package/python/python.mk
@@ -260,10 +260,11 @@ endif
 define PYTHON_CREATE_PYC_FILES
 	$(PYTHON_FIX_TIME)
 	PYTHONPATH="$(PYTHON_PATH)" \
-	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
+	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
-		usr/lib/python$(PYTHON_VERSION_MAJOR)
+		--strip-root $(TARGET_DIR) \
+		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
 endef
 
 ifeq ($(BR2_PACKAGE_PYTHON_PYC_ONLY)$(BR2_PACKAGE_PYTHON_PY_PYC),y)
diff --git a/package/python3/python3.mk b/package/python3/python3.mk
index 31e7ca3d3af3..4c8a12c7a3ad 100644
--- a/package/python3/python3.mk
+++ b/package/python3/python3.mk
@@ -277,10 +277,11 @@ endif
 define PYTHON3_CREATE_PYC_FILES
 	$(PYTHON3_FIX_TIME)
 	PYTHONPATH="$(PYTHON3_PATH)" \
-	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
+	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
-		usr/lib/python$(PYTHON3_VERSION_MAJOR)
+		--strip-root $(TARGET_DIR) \
+		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)
 endef
 
 ifeq ($(BR2_PACKAGE_PYTHON3_PYC_ONLY)$(BR2_PACKAGE_PYTHON3_PY_PYC),y)
diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index b713fe19323c..f3385555d647 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -1,75 +1,101 @@
 #!/usr/bin/env python
 
-'''Wrapper for python2 and python3 around compileall to raise exception
-when a python byte code generation failed.
-
-Inspired from:
-   http://stackoverflow.com/questions/615632/how-to-detect-errors-from-compileall-compile-dir
-'''
+"""
+Byte compile all .py files from provided directories. This script is an
+alternative implementation of compileall.compile_dir written with
+cross-compilation in mind.
+"""
 
 from __future__ import print_function
 
 import argparse
-import compileall
+import importlib
+import os
 import py_compile
+import re
+import struct
 import sys
 
 
-def check_for_errors(comparison):
-    '''Wrap comparison operator with code checking for PyCompileError.
-    If PyCompileError was raised, re-raise it again to abort execution,
-    otherwise perform comparison as expected.
-    '''
-    def operator(self, other):
-        exc_type, value, traceback = sys.exc_info()
-        if exc_type is not None and issubclass(exc_type,
-                                               py_compile.PyCompileError):
-            print("Cannot compile %s" % value.file)
-            raise value
-
-        return comparison(self, other)
-
-    return operator
-
-
-class ReportProblem(int):
-    '''Class that pretends to be an int() object but implements all of its
-    comparison operators such that it'd detect being called in
-    PyCompileError handling context and abort execution
-    '''
-    VALUE = 1
-
-    def __new__(cls, *args, **kwargs):
-        return int.__new__(cls, ReportProblem.VALUE, **kwargs)
-
-    @check_for_errors
-    def __lt__(self, other):
-        return ReportProblem.VALUE < other
-
-    @check_for_errors
-    def __eq__(self, other):
-        return ReportProblem.VALUE == other
-
-    def __ge__(self, other):
-        return not self < other
-
-    def __gt__(self, other):
-        return not self < other and not self == other
-
-    def __ne__(self, other):
-        return not self == other
+def compile_one(host_path, strip_root=None, force=False):
+    """
+    Compile a .py file into a .pyc file located next to it.
+
+    :arg host_path:
+        Absolute path to the file to compile on the host running the build.
+    :arg strip_root:
+        Prefix to remove from the original source paths encoded in compiled
+        files.
+    :arg force:
+        Recompile even if already up-to-date.
+    """
+    if os.path.islink(host_path) or not os.path.isfile(host_path):
+        return  # only compile real files
+
+    if not re.match(r"^[_A-Za-z][_A-Za-z0-9]+\.py$",
+                    os.path.basename(host_path)):
+        return  # only compile "importable" python modules
+
+    if not force:
+        # inspired from compileall.compile_file in the standard library
+        try:
+            with open(host_path + "c", "rb") as f:
+                header = f.read(12)
+            expect = struct.pack("<4sll", importlib.util.MAGIC_NUMBER,
+                                 0, int(os.stat(host_path).st_mtime))
+            if header == expect:
+                return  # .pyc file already up to date.
+        except OSError:
+            pass  # .pyc file does not exist
+
+    if strip_root is not None:
+        # determine the runtime path of the file (i.e.: relative path to root
+        # dir prepended with "/").
+        runtime_path = os.path.join("/", os.path.relpath(host_path, strip_root))
+    else:
+        runtime_path = host_path
+
+    # will raise an error if the file cannot be compiled
+    py_compile.compile(host_path, cfile=host_path + "c",
+                       dfile=runtime_path, doraise=True)
+
+
+def existing_dir_abs(arg):
+    """
+    argparse type callback that checks that argument is a directory and returns
+    its absolute path.
+    """
+    if not os.path.isdir(arg):
+        raise argparse.ArgumentTypeError('no such directory: {!r}'.format(arg))
+    return os.path.abspath(arg)
 
 
 def main():
     parser = argparse.ArgumentParser(description=__doc__)
-    parser.add_argument("target", metavar="TARGET",
-                        help="Directory to scan")
+    parser.add_argument("dirs", metavar="DIR", nargs="+", type=existing_dir_abs,
+                        help="Directory to recursively scan and compile")
+    parser.add_argument("--strip-root", metavar="ROOT", type=existing_dir_abs,
+                        help="""
+                        Prefix to remove from the original source paths encoded
+                        in compiled files
+                        """)
     parser.add_argument("--force", action="store_true",
                         help="Force compilation even if already compiled")
 
     args = parser.parse_args()
 
-    compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
+    try:
+        for d in args.dirs:
+            if args.strip_root and ".." in os.path.relpath(d, args.strip_root):
+                parser.error("DIR: not inside ROOT dir: {!r}".format(d))
+            for parent, _, files in os.walk(d):
+                for f in files:
+                    compile_one(os.path.join(parent, f), args.strip_root,
+                                args.force)
+
+    except Exception as e:
+        print("error: {}".format(e))
+        return 1
 
     return 0
 
-- 
2.28.0

[Buildroot] [PATCH v3 1/2] support/scripts/pycompile: fix .pyc original source file paths

[Robin Jarry]

2020-09-10, Robin Jarry:
> +    if not force:
> +        # inspired from compileall.compile_file in the standard library
> +        try:
> +            with open(host_path + "c", "rb") as f:
> +                header = f.read(12)
> +            expect = struct.pack("<4sll", importlib.util.MAGIC_NUMBER,
> +                                 0, int(os.stat(host_path).st_mtime))
> +            if header == expect:
> +                return  # .pyc file already up to date.
> +        except OSError:
> +            pass  # .pyc file does not exist

This bit does not work with python 2. I'll submit a fix in a v4.

Sorry about the noise...

-- 
Robin

[Buildroot] [PATCH v3 2/2] support/scripts/pycompile: add --verbose option

[Robin Jarry]

Add a new option that prints the (runtime) path of compiled .py files
when VERBOSE=1 is set.

Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
---
 package/python/python.mk     |  1 +
 package/python3/python3.mk   |  1 +
 support/scripts/pycompile.py | 11 +++++++++--
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/package/python/python.mk b/package/python/python.mk
index 3fe5ecd00462..4bff5f0c7c66 100644
--- a/package/python/python.mk
+++ b/package/python/python.mk
@@ -263,6 +263,7 @@ define PYTHON_CREATE_PYC_FILES
 	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
+		$(if $(VERBOSE),--verbose) \
 		--strip-root $(TARGET_DIR) \
 		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
 endef
diff --git a/package/python3/python3.mk b/package/python3/python3.mk
index 4c8a12c7a3ad..52c548accc9b 100644
--- a/package/python3/python3.mk
+++ b/package/python3/python3.mk
@@ -280,6 +280,7 @@ define PYTHON3_CREATE_PYC_FILES
 	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
+		$(if $(VERBOSE),--verbose) \
 		--strip-root $(TARGET_DIR) \
 		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)
 endef
diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index f3385555d647..d1daec5fac11 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -17,7 +17,7 @@ import struct
 import sys
 
 
-def compile_one(host_path, strip_root=None, force=False):
+def compile_one(host_path, strip_root=None, force=False, verbose=False):
     """
     Compile a .py file into a .pyc file located next to it.
 
@@ -28,6 +28,8 @@ def compile_one(host_path, strip_root=None, force=False):
         files.
     :arg force:
         Recompile even if already up-to-date.
+    :arg verbose:
+        Print compiled file paths.
     """
     if os.path.islink(host_path) or not os.path.isfile(host_path):
         return  # only compile real files
@@ -55,6 +57,9 @@ def compile_one(host_path, strip_root=None, force=False):
     else:
         runtime_path = host_path
 
+    if verbose:
+        print("  PYC  {}".format(runtime_path))
+
     # will raise an error if the file cannot be compiled
     py_compile.compile(host_path, cfile=host_path + "c",
                        dfile=runtime_path, doraise=True)
@@ -81,6 +86,8 @@ def main():
                         """)
     parser.add_argument("--force", action="store_true",
                         help="Force compilation even if already compiled")
+    parser.add_argument("--verbose", action="store_true",
+                        help="Print compiled files")
 
     args = parser.parse_args()
 
@@ -91,7 +98,7 @@ def main():
             for parent, _, files in os.walk(d):
                 for f in files:
                     compile_one(os.path.join(parent, f), args.strip_root,
-                                args.force)
+                                args.force, args.verbose)
 
     except Exception as e:
         print("error: {}".format(e))
-- 
2.28.0

[Buildroot] [PATCH v4 0/2] pycompile: fix .pyc source paths + improvements

[Robin Jarry]

This series is about fixing the original .py source paths encoded into
.pyc files. It comes with small improvements on pycompile.py as well.

The first two commits of v2 have been applied already, I am sending the
last two which needed some rework.

Also, v3 has a problem with python 3.4 and earlier (especially python
2). This v4 should work with all versions of python.

Robin Jarry (2):
  support/scripts/pycompile: fix .pyc original source file paths
  support/scripts/pycompile: add --verbose option

 package/python/python.mk     |   6 +-
 package/python3/python3.mk   |   6 +-
 support/scripts/pycompile.py | 153 +++++++++++++++++++++++------------
 3 files changed, 108 insertions(+), 57 deletions(-)

Differences since v3:

  * Fixed errors with python <3.4.

Differences since v2:

  * Used str.format() instead of % format.
  * Renamed the TARGET positional argument into --strip-root which is
    more explicit. The script is called with --strip-root=$(TARGET_DIR).
  * Homogenized the arguments metavars.
  * Used argparse type callbacks for directory arguments checking.

-- 
2.28.0

[Buildroot] [PATCH v4 1/2] support/scripts/pycompile: fix .pyc original source file paths

[Robin Jarry]

When generating a .pyc file, the original .py source file path is
encoded in it. It is used for various purposes: traceback generation,
.pyc file comparison with its .py source, and code inspection.

By default, the source path used when invoking compileall is encoded in
the .pyc file. Since we use paths relative to TARGET_DIR, we end up with
paths that are only valid when relative to '/' encoded in the installed
.pyc files on the target.

This breaks code inspection at runtime since the original source path
will be invalid unless the code is executed from '/'.

Unfortunately, compileall cannot be forced to use the proper path. It
was not written with cross-compilation usage in mind.

Rework the script to call py_compile.compile() directly with pertinent
options:

- The script now has a new --strip-root argument. This argument is
  optional but will always be specified when compiling py files in
  buildroot.
- All other (non-optional) arguments are folders in which all
  "importable" .py files will be compiled to .pyc.
- Using --strip-root=$(TARGET_DIR), the future runtime path of each .py
  file is computed and encoded into the compiled .pyc.

No need to change directory before running the script anymore.

The trickery used to handle error reporting was only applicable with
compileall. Since we implement our own "compileall", error reporting
becomes trivial.

Signed-off-by: Julien Floret <julien.floret@6wind.com>
Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
---
 package/python/python.mk     |   5 +-
 package/python3/python3.mk   |   5 +-
 support/scripts/pycompile.py | 146 ++++++++++++++++++++++-------------
 3 files changed, 99 insertions(+), 57 deletions(-)

diff --git a/package/python/python.mk b/package/python/python.mk
index ccaaadd012a5..3fe5ecd00462 100644
--- a/package/python/python.mk
+++ b/package/python/python.mk
@@ -260,10 +260,11 @@ endif
 define PYTHON_CREATE_PYC_FILES
 	$(PYTHON_FIX_TIME)
 	PYTHONPATH="$(PYTHON_PATH)" \
-	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
+	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
-		usr/lib/python$(PYTHON_VERSION_MAJOR)
+		--strip-root $(TARGET_DIR) \
+		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
 endef
 
 ifeq ($(BR2_PACKAGE_PYTHON_PYC_ONLY)$(BR2_PACKAGE_PYTHON_PY_PYC),y)
diff --git a/package/python3/python3.mk b/package/python3/python3.mk
index 31e7ca3d3af3..4c8a12c7a3ad 100644
--- a/package/python3/python3.mk
+++ b/package/python3/python3.mk
@@ -277,10 +277,11 @@ endif
 define PYTHON3_CREATE_PYC_FILES
 	$(PYTHON3_FIX_TIME)
 	PYTHONPATH="$(PYTHON3_PATH)" \
-	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
+	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
-		usr/lib/python$(PYTHON3_VERSION_MAJOR)
+		--strip-root $(TARGET_DIR) \
+		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)
 endef
 
 ifeq ($(BR2_PACKAGE_PYTHON3_PYC_ONLY)$(BR2_PACKAGE_PYTHON3_PY_PYC),y)
diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index b713fe19323c..04193f4a02c2 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -1,75 +1,115 @@
 #!/usr/bin/env python
 
-'''Wrapper for python2 and python3 around compileall to raise exception
-when a python byte code generation failed.
-
-Inspired from:
-   http://stackoverflow.com/questions/615632/how-to-detect-errors-from-compileall-compile-dir
-'''
+"""
+Byte compile all .py files from provided directories. This script is an
+alternative implementation of compileall.compile_dir written with
+cross-compilation in mind.
+"""
 
 from __future__ import print_function
 
 import argparse
-import compileall
+import os
 import py_compile
+import re
+import struct
 import sys
 
 
-def check_for_errors(comparison):
-    '''Wrap comparison operator with code checking for PyCompileError.
-    If PyCompileError was raised, re-raise it again to abort execution,
-    otherwise perform comparison as expected.
-    '''
-    def operator(self, other):
-        exc_type, value, traceback = sys.exc_info()
-        if exc_type is not None and issubclass(exc_type,
-                                               py_compile.PyCompileError):
-            print("Cannot compile %s" % value.file)
-            raise value
-
-        return comparison(self, other)
-
-    return operator
-
-
-class ReportProblem(int):
-    '''Class that pretends to be an int() object but implements all of its
-    comparison operators such that it'd detect being called in
-    PyCompileError handling context and abort execution
-    '''
-    VALUE = 1
-
-    def __new__(cls, *args, **kwargs):
-        return int.__new__(cls, ReportProblem.VALUE, **kwargs)
-
-    @check_for_errors
-    def __lt__(self, other):
-        return ReportProblem.VALUE < other
-
-    @check_for_errors
-    def __eq__(self, other):
-        return ReportProblem.VALUE == other
-
-    def __ge__(self, other):
-        return not self < other
-
-    def __gt__(self, other):
-        return not self < other and not self == other
-
-    def __ne__(self, other):
-        return not self == other
+if sys.version_info < (3, 4):
+    import imp  # import here to avoid deprecation warning when >=3.4
+    PYC_HEADER_ARGS = (imp.get_magic(),)
+else:
+    import importlib
+    PYC_HEADER_ARGS = (importlib.util.MAGIC_NUMBER,)
+if sys.version_info < (3, 7):
+    PYC_HEADER_LEN = 8
+    PYC_HEADER_FMT = "<4sl"
+else:
+    PYC_HEADER_LEN = 12
+    PYC_HEADER_FMT = "<4sll"
+    PYC_HEADER_ARGS += (0,)  # zero hash, we use timestamp invalidation
+
+
+def compile_one(host_path, strip_root=None, force=False):
+    """
+    Compile a .py file into a .pyc file located next to it.
+
+    :arg host_path:
+        Absolute path to the file to compile on the host running the build.
+    :arg strip_root:
+        Prefix to remove from the original source paths encoded in compiled
+        files.
+    :arg force:
+        Recompile even if already up-to-date.
+    """
+    if os.path.islink(host_path) or not os.path.isfile(host_path):
+        return  # only compile real files
+
+    if not re.match(r"^[_A-Za-z][_A-Za-z0-9]+\.py$",
+                    os.path.basename(host_path)):
+        return  # only compile "importable" python modules
+
+    if not force:
+        # inspired from compileall.compile_file in the standard library
+        try:
+            with open(host_path + "c", "rb") as f:
+                header = f.read(PYC_HEADER_LEN)
+            header_args = PYC_HEADER_ARGS + (int(os.stat(host_path).st_mtime),)
+            expect = struct.pack(PYC_HEADER_FMT, *header_args)
+            if header == expect:
+                return  # .pyc file already up to date.
+        except OSError:
+            pass  # .pyc file does not exist
+
+    if strip_root is not None:
+        # determine the runtime path of the file (i.e.: relative path to root
+        # dir prepended with "/").
+        runtime_path = os.path.join("/", os.path.relpath(host_path, strip_root))
+    else:
+        runtime_path = host_path
+
+    # will raise an error if the file cannot be compiled
+    py_compile.compile(host_path, cfile=host_path + "c",
+                       dfile=runtime_path, doraise=True)
+
+
+def existing_dir_abs(arg):
+    """
+    argparse type callback that checks that argument is a directory and returns
+    its absolute path.
+    """
+    if not os.path.isdir(arg):
+        raise argparse.ArgumentTypeError('no such directory: {!r}'.format(arg))
+    return os.path.abspath(arg)
 
 
 def main():
     parser = argparse.ArgumentParser(description=__doc__)
-    parser.add_argument("target", metavar="TARGET",
-                        help="Directory to scan")
+    parser.add_argument("dirs", metavar="DIR", nargs="+", type=existing_dir_abs,
+                        help="Directory to recursively scan and compile")
+    parser.add_argument("--strip-root", metavar="ROOT", type=existing_dir_abs,
+                        help="""
+                        Prefix to remove from the original source paths encoded
+                        in compiled files
+                        """)
     parser.add_argument("--force", action="store_true",
                         help="Force compilation even if already compiled")
 
     args = parser.parse_args()
 
-    compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
+    try:
+        for d in args.dirs:
+            if args.strip_root and ".." in os.path.relpath(d, args.strip_root):
+                parser.error("DIR: not inside ROOT dir: {!r}".format(d))
+            for parent, _, files in os.walk(d):
+                for f in files:
+                    compile_one(os.path.join(parent, f), args.strip_root,
+                                args.force)
+
+    except Exception as e:
+        print("error: {}".format(e))
+        return 1
 
     return 0
 
-- 
2.28.0

[Buildroot] [PATCH v4 1/2] support/scripts/pycompile: fix .pyc original source file paths

[Yann E. MORIN]

Robin, All,

On 2020-09-10 10:32 +0200, Robin Jarry spake thusly:
> When generating a .pyc file, the original .py source file path is
> encoded in it. It is used for various purposes: traceback generation,
> .pyc file comparison with its .py source, and code inspection.
[--SNIP--]
> +if sys.version_info < (3, 4):
> +    import imp  # import here to avoid deprecation warning when >=3.4
> +    PYC_HEADER_ARGS = (imp.get_magic(),)
> +else:
> +    import importlib
> +    PYC_HEADER_ARGS = (importlib.util.MAGIC_NUMBER,)
> +if sys.version_info < (3, 7):
> +    PYC_HEADER_LEN = 8
> +    PYC_HEADER_FMT = "<4sl"
> +else:
> +    PYC_HEADER_LEN = 12
> +    PYC_HEADER_FMT = "<4sll"
> +    PYC_HEADER_ARGS += (0,)  # zero hash, we use timestamp invalidation

This...

> +def compile_one(host_path, strip_root=None, force=False):
[--SNIP--]
> +    if not force:
> +        # inspired from compileall.compile_file in the standard library
> +        try:
> +            with open(host_path + "c", "rb") as f:
> +                header = f.read(PYC_HEADER_LEN)
> +            header_args = PYC_HEADER_ARGS + (int(os.stat(host_path).st_mtime),)
> +            expect = struct.pack(PYC_HEADER_FMT, *header_args)
> +            if header == expect:
> +                return  # .pyc file already up to date.
> +        except OSError:
> +            pass  # .pyc file does not exist

... and this is scary to me... :-(

I understand the reasoning: no need to re-compile a file that was
already compiled and has not changed. This is an understandable
optimisation, and one that was already present in the previous script.

Still, having to poke into the internals sounds a bit too invasive to
me, especially as those internals are version-specific (as your
coditional code demonstrates).

Can't we instead use ctime or mtime to detect whether a file needs
updating?

Alternatively, how much time do we actually shave off the build with
this optimisation? I've done a simple build with this defconfig:

    BR2_arm=y
    BR2_cortex_a7=y
    BR2_PER_PACKAGE_DIRECTORIES=y
    BR2_TOOLCHAIN_EXTERNAL=y
    BR2_INIT_NONE=y
    BR2_SYSTEM_BIN_SH_NONE=y
    # BR2_PACKAGE_BUSYBOX is not set
    BR2_PACKAGE_PYTHON3=y
    # BR2_PACKAGE_PYTHON3_UNICODEDATA is not set

That is, basically, only python3 and its dependencies are built.
I also applied this little patch on top of this one:

    diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
    index 04193f4a02..f563eff027 100644
    --- a/support/scripts/pycompile.py
    +++ b/support/scripts/pycompile.py
    @@ -14,6 +14,7 @@ import py_compile
     import re
     import struct
     import sys
    +import time
     
     
     if sys.version_info < (3, 4):
    @@ -100,12 +101,14 @@ def main():
     
         try:
             for d in args.dirs:
    +            t0 = time.time()
                 if args.strip_root and ".." in os.path.relpath(d, args.strip_root):
                     parser.error("DIR: not inside ROOT dir: {!r}".format(d))
                 for parent, _, files in os.walk(d):
                     for f in files:
                         compile_one(os.path.join(parent, f), args.strip_root,
                                     args.force)
    +            print('Duration {} {}'.format(time.time()-t0, d))
     
         except Exception as e:
             print("error: {}".format(e))

The build takes 3min 40s, and the pre-compilation takes less than a
second. Of course, adding more python module will only increase the
pre-compile duration.

I think the duration gain is negligible, while the intricacies of the
code to detect whether pre-compilation should occur is probably too much
of a burden, maintenance-wise.

So, it is my opinion we should rop this.

Of course, no need to reend for now: I'd like the opinion from the other
maitnainers,, and maybe we can leave the topic open for others to review
as well. Also, if we decide to drop it, I can do that pretty easily when
applying...

Thanks for the itetrations on this series! :-)

Regards,
Yann E. MORIN.

> +    if strip_root is not None:
> +        # determine the runtime path of the file (i.e.: relative path to root
> +        # dir prepended with "/").
> +        runtime_path = os.path.join("/", os.path.relpath(host_path, strip_root))
> +    else:
> +        runtime_path = host_path
> +
> +    # will raise an error if the file cannot be compiled
> +    py_compile.compile(host_path, cfile=host_path + "c",
> +                       dfile=runtime_path, doraise=True)
> +
> +
> +def existing_dir_abs(arg):
> +    """
> +    argparse type callback that checks that argument is a directory and returns
> +    its absolute path.
> +    """
> +    if not os.path.isdir(arg):
> +        raise argparse.ArgumentTypeError('no such directory: {!r}'.format(arg))
> +    return os.path.abspath(arg)
>  
>  
>  def main():
>      parser = argparse.ArgumentParser(description=__doc__)
> -    parser.add_argument("target", metavar="TARGET",
> -                        help="Directory to scan")
> +    parser.add_argument("dirs", metavar="DIR", nargs="+", type=existing_dir_abs,
> +                        help="Directory to recursively scan and compile")
> +    parser.add_argument("--strip-root", metavar="ROOT", type=existing_dir_abs,
> +                        help="""
> +                        Prefix to remove from the original source paths encoded
> +                        in compiled files
> +                        """)
>      parser.add_argument("--force", action="store_true",
>                          help="Force compilation even if already compiled")
>  
>      args = parser.parse_args()
>  
> -    compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
> +    try:
> +        for d in args.dirs:
> +            if args.strip_root and ".." in os.path.relpath(d, args.strip_root):
> +                parser.error("DIR: not inside ROOT dir: {!r}".format(d))
> +            for parent, _, files in os.walk(d):
> +                for f in files:
> +                    compile_one(os.path.join(parent, f), args.strip_root,
> +                                args.force)
> +
> +    except Exception as e:
> +        print("error: {}".format(e))
> +        return 1
>  
>      return 0
>  
> -- 
> 2.28.0
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v4 1/2] support/scripts/pycompile: fix .pyc original source file paths

[Robin Jarry]

Hi Yann,

2020-09-11, Yann E. MORIN:
[snip]
> This...
[snip]
> ... and this is scary to me... :-(
> 
> I understand the reasoning: no need to re-compile a file that was
> already compiled and has not changed. This is an understandable
> optimisation, and one that was already present in the previous script.
> 
> Still, having to poke into the internals sounds a bit too invasive to
> me, especially as those internals are version-specific (as your
> coditional code demonstrates).

Yes that is not very pretty and it will certainly be a burden to carry
in the long run. I made the effort to implement this so that the
previous incremental build feature was preserved.

> Can't we instead use ctime or mtime to detect whether a file needs
> updating?

I am not sure it would be reliable given that buildroot may change the
mtime of .py files in $(TARGET_DIR). But yes, it could be possible.
I think it would be safer to remove this feature completely.

> Alternatively, how much time do we actually shave off the build with
> this optimisation? I've done a simple build with this defconfig:
[snip]
> The build takes 3min 40s, and the pre-compilation takes less than a
> second. Of course, adding more python module will only increase the
> pre-compile duration.
> 
> I think the duration gain is negligible, while the intricacies of the
> code to detect whether pre-compilation should occur is probably too much
> of a burden, maintenance-wise.
> 
> So, it is my opinion we should rop this.

I agree, byte compiling python code is very fast and even if every
python package is selected, the time it takes to compile all .py files
will be completely negligible compared to the full duration of the
build. The --force option could be removed in my opinion.

> Of course, no need to reend for now: I'd like the opinion from the other
> maitnainers,, and maybe we can leave the topic open for others to review
> as well. Also, if we decide to drop it, I can do that pretty easily when
> applying...

I don't mind doing a v5 with --force removed or with a different (less
reliable?) way to determine if .py files should be recompiled.

Cheers,

-- 
Robin

[Buildroot] [PATCH v4 1/2] support/scripts/pycompile: fix .pyc original source file paths

[Yann E. MORIN]

Robin, All,

On 2020-09-12 13:44 +0200, Robin Jarry spake thusly:
> 2020-09-11, Yann E. MORIN:
[--SNIP--]
> > Still, having to poke into the internals sounds a bit too invasive to
> > me, especially as those internals are version-specific (as your
> > coditional code demonstrates).
> Yes that is not very pretty and it will certainly be a burden to carry
> in the long run. I made the effort to implement this so that the
> previous incremental build feature was preserved.

Yes, I understand why you did that, and that's good. :-)

[--SNIP--]
> I agree, byte compiling python code is very fast and even if every
> python package is selected, the time it takes to compile all .py files
> will be completely negligible compared to the full duration of the
> build. The --force option could be removed in my opinion.

I agree, and I'm going to do while applying.

Thanks!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v4 1/2] support/scripts/pycompile: fix .pyc original source file paths

[Yann E. MORIN]

Robin, All,

On 2020-09-10 10:32 +0200, Robin Jarry spake thusly:
> When generating a .pyc file, the original .py source file path is
> encoded in it. It is used for various purposes: traceback generation,
> .pyc file comparison with its .py source, and code inspection.
> 
> By default, the source path used when invoking compileall is encoded in
> the .pyc file. Since we use paths relative to TARGET_DIR, we end up with
> paths that are only valid when relative to '/' encoded in the installed
> .pyc files on the target.
> 
> This breaks code inspection at runtime since the original source path
> will be invalid unless the code is executed from '/'.
> 
> Unfortunately, compileall cannot be forced to use the proper path. It
> was not written with cross-compilation usage in mind.
> 
> Rework the script to call py_compile.compile() directly with pertinent
> options:
> 
> - The script now has a new --strip-root argument. This argument is
>   optional but will always be specified when compiling py files in
>   buildroot.
> - All other (non-optional) arguments are folders in which all
>   "importable" .py files will be compiled to .pyc.
> - Using --strip-root=$(TARGET_DIR), the future runtime path of each .py
>   file is computed and encoded into the compiled .pyc.
> 
> No need to change directory before running the script anymore.
> 
> The trickery used to handle error reporting was only applicable with
> compileall. Since we implement our own "compileall", error reporting
> becomes trivial.
> 
> Signed-off-by: Julien Floret <julien.floret@6wind.com>
> Signed-off-by: Robin Jarry <robin.jarry@6wind.com>

Applied to master, after removing the --force option as discussed in the
thread.

Thanks!

Regards,
Yann E. MORIN.

> ---
>  package/python/python.mk     |   5 +-
>  package/python3/python3.mk   |   5 +-
>  support/scripts/pycompile.py | 146 ++++++++++++++++++++++-------------
>  3 files changed, 99 insertions(+), 57 deletions(-)
> 
> diff --git a/package/python/python.mk b/package/python/python.mk
> index ccaaadd012a5..3fe5ecd00462 100644
> --- a/package/python/python.mk
> +++ b/package/python/python.mk
> @@ -260,10 +260,11 @@ endif
>  define PYTHON_CREATE_PYC_FILES
>  	$(PYTHON_FIX_TIME)
>  	PYTHONPATH="$(PYTHON_PATH)" \
> -	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
> +	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
>  		$(TOPDIR)/support/scripts/pycompile.py \
>  		$(if $(BR2_REPRODUCIBLE),--force) \
> -		usr/lib/python$(PYTHON_VERSION_MAJOR)
> +		--strip-root $(TARGET_DIR) \
> +		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
>  endef
>  
>  ifeq ($(BR2_PACKAGE_PYTHON_PYC_ONLY)$(BR2_PACKAGE_PYTHON_PY_PYC),y)
> diff --git a/package/python3/python3.mk b/package/python3/python3.mk
> index 31e7ca3d3af3..4c8a12c7a3ad 100644
> --- a/package/python3/python3.mk
> +++ b/package/python3/python3.mk
> @@ -277,10 +277,11 @@ endif
>  define PYTHON3_CREATE_PYC_FILES
>  	$(PYTHON3_FIX_TIME)
>  	PYTHONPATH="$(PYTHON3_PATH)" \
> -	cd $(TARGET_DIR) && $(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
> +	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
>  		$(TOPDIR)/support/scripts/pycompile.py \
>  		$(if $(BR2_REPRODUCIBLE),--force) \
> -		usr/lib/python$(PYTHON3_VERSION_MAJOR)
> +		--strip-root $(TARGET_DIR) \
> +		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)
>  endef
>  
>  ifeq ($(BR2_PACKAGE_PYTHON3_PYC_ONLY)$(BR2_PACKAGE_PYTHON3_PY_PYC),y)
> diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
> index b713fe19323c..04193f4a02c2 100644
> --- a/support/scripts/pycompile.py
> +++ b/support/scripts/pycompile.py
> @@ -1,75 +1,115 @@
>  #!/usr/bin/env python
>  
> -'''Wrapper for python2 and python3 around compileall to raise exception
> -when a python byte code generation failed.
> -
> -Inspired from:
> -   http://stackoverflow.com/questions/615632/how-to-detect-errors-from-compileall-compile-dir
> -'''
> +"""
> +Byte compile all .py files from provided directories. This script is an
> +alternative implementation of compileall.compile_dir written with
> +cross-compilation in mind.
> +"""
>  
>  from __future__ import print_function
>  
>  import argparse
> -import compileall
> +import os
>  import py_compile
> +import re
> +import struct
>  import sys
>  
>  
> -def check_for_errors(comparison):
> -    '''Wrap comparison operator with code checking for PyCompileError.
> -    If PyCompileError was raised, re-raise it again to abort execution,
> -    otherwise perform comparison as expected.
> -    '''
> -    def operator(self, other):
> -        exc_type, value, traceback = sys.exc_info()
> -        if exc_type is not None and issubclass(exc_type,
> -                                               py_compile.PyCompileError):
> -            print("Cannot compile %s" % value.file)
> -            raise value
> -
> -        return comparison(self, other)
> -
> -    return operator
> -
> -
> -class ReportProblem(int):
> -    '''Class that pretends to be an int() object but implements all of its
> -    comparison operators such that it'd detect being called in
> -    PyCompileError handling context and abort execution
> -    '''
> -    VALUE = 1
> -
> -    def __new__(cls, *args, **kwargs):
> -        return int.__new__(cls, ReportProblem.VALUE, **kwargs)
> -
> -    @check_for_errors
> -    def __lt__(self, other):
> -        return ReportProblem.VALUE < other
> -
> -    @check_for_errors
> -    def __eq__(self, other):
> -        return ReportProblem.VALUE == other
> -
> -    def __ge__(self, other):
> -        return not self < other
> -
> -    def __gt__(self, other):
> -        return not self < other and not self == other
> -
> -    def __ne__(self, other):
> -        return not self == other
> +if sys.version_info < (3, 4):
> +    import imp  # import here to avoid deprecation warning when >=3.4
> +    PYC_HEADER_ARGS = (imp.get_magic(),)
> +else:
> +    import importlib
> +    PYC_HEADER_ARGS = (importlib.util.MAGIC_NUMBER,)
> +if sys.version_info < (3, 7):
> +    PYC_HEADER_LEN = 8
> +    PYC_HEADER_FMT = "<4sl"
> +else:
> +    PYC_HEADER_LEN = 12
> +    PYC_HEADER_FMT = "<4sll"
> +    PYC_HEADER_ARGS += (0,)  # zero hash, we use timestamp invalidation
> +
> +
> +def compile_one(host_path, strip_root=None, force=False):
> +    """
> +    Compile a .py file into a .pyc file located next to it.
> +
> +    :arg host_path:
> +        Absolute path to the file to compile on the host running the build.
> +    :arg strip_root:
> +        Prefix to remove from the original source paths encoded in compiled
> +        files.
> +    :arg force:
> +        Recompile even if already up-to-date.
> +    """
> +    if os.path.islink(host_path) or not os.path.isfile(host_path):
> +        return  # only compile real files
> +
> +    if not re.match(r"^[_A-Za-z][_A-Za-z0-9]+\.py$",
> +                    os.path.basename(host_path)):
> +        return  # only compile "importable" python modules
> +
> +    if not force:
> +        # inspired from compileall.compile_file in the standard library
> +        try:
> +            with open(host_path + "c", "rb") as f:
> +                header = f.read(PYC_HEADER_LEN)
> +            header_args = PYC_HEADER_ARGS + (int(os.stat(host_path).st_mtime),)
> +            expect = struct.pack(PYC_HEADER_FMT, *header_args)
> +            if header == expect:
> +                return  # .pyc file already up to date.
> +        except OSError:
> +            pass  # .pyc file does not exist
> +
> +    if strip_root is not None:
> +        # determine the runtime path of the file (i.e.: relative path to root
> +        # dir prepended with "/").
> +        runtime_path = os.path.join("/", os.path.relpath(host_path, strip_root))
> +    else:
> +        runtime_path = host_path
> +
> +    # will raise an error if the file cannot be compiled
> +    py_compile.compile(host_path, cfile=host_path + "c",
> +                       dfile=runtime_path, doraise=True)
> +
> +
> +def existing_dir_abs(arg):
> +    """
> +    argparse type callback that checks that argument is a directory and returns
> +    its absolute path.
> +    """
> +    if not os.path.isdir(arg):
> +        raise argparse.ArgumentTypeError('no such directory: {!r}'.format(arg))
> +    return os.path.abspath(arg)
>  
>  
>  def main():
>      parser = argparse.ArgumentParser(description=__doc__)
> -    parser.add_argument("target", metavar="TARGET",
> -                        help="Directory to scan")
> +    parser.add_argument("dirs", metavar="DIR", nargs="+", type=existing_dir_abs,
> +                        help="Directory to recursively scan and compile")
> +    parser.add_argument("--strip-root", metavar="ROOT", type=existing_dir_abs,
> +                        help="""
> +                        Prefix to remove from the original source paths encoded
> +                        in compiled files
> +                        """)
>      parser.add_argument("--force", action="store_true",
>                          help="Force compilation even if already compiled")
>  
>      args = parser.parse_args()
>  
> -    compileall.compile_dir(args.target, force=args.force, quiet=ReportProblem())
> +    try:
> +        for d in args.dirs:
> +            if args.strip_root and ".." in os.path.relpath(d, args.strip_root):
> +                parser.error("DIR: not inside ROOT dir: {!r}".format(d))
> +            for parent, _, files in os.walk(d):
> +                for f in files:
> +                    compile_one(os.path.join(parent, f), args.strip_root,
> +                                args.force)
> +
> +    except Exception as e:
> +        print("error: {}".format(e))
> +        return 1
>  
>      return 0
>  
> -- 
> 2.28.0
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v4 1/2] support/scripts/pycompile: fix .pyc original source file paths

[Robin Jarry]

2020-09-13, Yann E. MORIN:
> Robin, All,
[snip]
> Applied to master, after removing the --force option as discussed in the
> thread.
> 
> Thanks!

Thanks Yann :)

-- 
Robin

[Buildroot] [PATCH v4 1/2] support/scripts/pycompile: fix .pyc original source file paths

[Peter Korsgaard]

>>>>> "Robin" == Robin Jarry <robin.jarry@6wind.com> writes:

 > When generating a .pyc file, the original .py source file path is
 > encoded in it. It is used for various purposes: traceback generation,
 > .pyc file comparison with its .py source, and code inspection.

 > By default, the source path used when invoking compileall is encoded in
 > the .pyc file. Since we use paths relative to TARGET_DIR, we end up with
 > paths that are only valid when relative to '/' encoded in the installed
 > .pyc files on the target.

 > This breaks code inspection at runtime since the original source path
 > will be invalid unless the code is executed from '/'.

 > Unfortunately, compileall cannot be forced to use the proper path. It
 > was not written with cross-compilation usage in mind.

 > Rework the script to call py_compile.compile() directly with pertinent
 > options:

 > - The script now has a new --strip-root argument. This argument is
 >   optional but will always be specified when compiling py files in
 >   buildroot.
 > - All other (non-optional) arguments are folders in which all
 >   "importable" .py files will be compiled to .pyc.
 > - Using --strip-root=$(TARGET_DIR), the future runtime path of each .py
 >   file is computed and encoded into the compiled .pyc.

 > No need to change directory before running the script anymore.

 > The trickery used to handle error reporting was only applicable with
 > compileall. Since we implement our own "compileall", error reporting
 > becomes trivial.

 > Signed-off-by: Julien Floret <julien.floret@6wind.com>
 > Signed-off-by: Robin Jarry <robin.jarry@6wind.com>

Committed to 2020.02.x, 2020.05.x and 2020.08.x, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v4 2/2] support/scripts/pycompile: add --verbose option

[Robin Jarry]

Add a new option that prints the (runtime) path of compiled .py files
when VERBOSE=1 is set.

Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
---
 package/python/python.mk     |  1 +
 package/python3/python3.mk   |  1 +
 support/scripts/pycompile.py | 11 +++++++++--
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/package/python/python.mk b/package/python/python.mk
index 3fe5ecd00462..4bff5f0c7c66 100644
--- a/package/python/python.mk
+++ b/package/python/python.mk
@@ -263,6 +263,7 @@ define PYTHON_CREATE_PYC_FILES
 	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
+		$(if $(VERBOSE),--verbose) \
 		--strip-root $(TARGET_DIR) \
 		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
 endef
diff --git a/package/python3/python3.mk b/package/python3/python3.mk
index 4c8a12c7a3ad..52c548accc9b 100644
--- a/package/python3/python3.mk
+++ b/package/python3/python3.mk
@@ -280,6 +280,7 @@ define PYTHON3_CREATE_PYC_FILES
 	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
 		$(TOPDIR)/support/scripts/pycompile.py \
 		$(if $(BR2_REPRODUCIBLE),--force) \
+		$(if $(VERBOSE),--verbose) \
 		--strip-root $(TARGET_DIR) \
 		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)
 endef
diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index 04193f4a02c2..55c09fe688d6 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -31,7 +31,7 @@ else:
     PYC_HEADER_ARGS += (0,)  # zero hash, we use timestamp invalidation
 
 
-def compile_one(host_path, strip_root=None, force=False):
+def compile_one(host_path, strip_root=None, force=False, verbose=False):
     """
     Compile a .py file into a .pyc file located next to it.
 
@@ -42,6 +42,8 @@ def compile_one(host_path, strip_root=None, force=False):
         files.
     :arg force:
         Recompile even if already up-to-date.
+    :arg verbose:
+        Print compiled file paths.
     """
     if os.path.islink(host_path) or not os.path.isfile(host_path):
         return  # only compile real files
@@ -69,6 +71,9 @@ def compile_one(host_path, strip_root=None, force=False):
     else:
         runtime_path = host_path
 
+    if verbose:
+        print("  PYC  {}".format(runtime_path))
+
     # will raise an error if the file cannot be compiled
     py_compile.compile(host_path, cfile=host_path + "c",
                        dfile=runtime_path, doraise=True)
@@ -95,6 +100,8 @@ def main():
                         """)
     parser.add_argument("--force", action="store_true",
                         help="Force compilation even if already compiled")
+    parser.add_argument("--verbose", action="store_true",
+                        help="Print compiled files")
 
     args = parser.parse_args()
 
@@ -105,7 +112,7 @@ def main():
             for parent, _, files in os.walk(d):
                 for f in files:
                     compile_one(os.path.join(parent, f), args.strip_root,
-                                args.force)
+                                args.force, args.verbose)
 
     except Exception as e:
         print("error: {}".format(e))
-- 
2.28.0

[Buildroot] [PATCH v4 2/2] support/scripts/pycompile: add --verbose option

[Yann E. MORIN]

Robin, All,

On 2020-09-10 10:32 +0200, Robin Jarry spake thusly:
> Add a new option that prints the (runtime) path of compiled .py files
> when VERBOSE=1 is set.
> 
> Signed-off-by: Robin Jarry <robin.jarry@6wind.com>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  package/python/python.mk     |  1 +
>  package/python3/python3.mk   |  1 +
>  support/scripts/pycompile.py | 11 +++++++++--
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/package/python/python.mk b/package/python/python.mk
> index 3fe5ecd00462..4bff5f0c7c66 100644
> --- a/package/python/python.mk
> +++ b/package/python/python.mk
> @@ -263,6 +263,7 @@ define PYTHON_CREATE_PYC_FILES
>  	$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR) \
>  		$(TOPDIR)/support/scripts/pycompile.py \
>  		$(if $(BR2_REPRODUCIBLE),--force) \
> +		$(if $(VERBOSE),--verbose) \
>  		--strip-root $(TARGET_DIR) \
>  		$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)
>  endef
> diff --git a/package/python3/python3.mk b/package/python3/python3.mk
> index 4c8a12c7a3ad..52c548accc9b 100644
> --- a/package/python3/python3.mk
> +++ b/package/python3/python3.mk
> @@ -280,6 +280,7 @@ define PYTHON3_CREATE_PYC_FILES
>  	$(HOST_DIR)/bin/python$(PYTHON3_VERSION_MAJOR) \
>  		$(TOPDIR)/support/scripts/pycompile.py \
>  		$(if $(BR2_REPRODUCIBLE),--force) \
> +		$(if $(VERBOSE),--verbose) \
>  		--strip-root $(TARGET_DIR) \
>  		$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)
>  endef
> diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
> index 04193f4a02c2..55c09fe688d6 100644
> --- a/support/scripts/pycompile.py
> +++ b/support/scripts/pycompile.py
> @@ -31,7 +31,7 @@ else:
>      PYC_HEADER_ARGS += (0,)  # zero hash, we use timestamp invalidation
>  
>  
> -def compile_one(host_path, strip_root=None, force=False):
> +def compile_one(host_path, strip_root=None, force=False, verbose=False):
>      """
>      Compile a .py file into a .pyc file located next to it.
>  
> @@ -42,6 +42,8 @@ def compile_one(host_path, strip_root=None, force=False):
>          files.
>      :arg force:
>          Recompile even if already up-to-date.
> +    :arg verbose:
> +        Print compiled file paths.
>      """
>      if os.path.islink(host_path) or not os.path.isfile(host_path):
>          return  # only compile real files
> @@ -69,6 +71,9 @@ def compile_one(host_path, strip_root=None, force=False):
>      else:
>          runtime_path = host_path
>  
> +    if verbose:
> +        print("  PYC  {}".format(runtime_path))
> +
>      # will raise an error if the file cannot be compiled
>      py_compile.compile(host_path, cfile=host_path + "c",
>                         dfile=runtime_path, doraise=True)
> @@ -95,6 +100,8 @@ def main():
>                          """)
>      parser.add_argument("--force", action="store_true",
>                          help="Force compilation even if already compiled")
> +    parser.add_argument("--verbose", action="store_true",
> +                        help="Print compiled files")
>  
>      args = parser.parse_args()
>  
> @@ -105,7 +112,7 @@ def main():
>              for parent, _, files in os.walk(d):
>                  for f in files:
>                      compile_one(os.path.join(parent, f), args.strip_root,
> -                                args.force)
> +                                args.force, args.verbose)
>  
>      except Exception as e:
>          print("error: {}".format(e))
> -- 
> 2.28.0
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'