diff mbox

[v4] python: rework *.pyc files generation

Message ID 1447948334-8440-1-git-send-email-yegorslists@googlemail.com
State Changes Requested
Headers show

Commit Message

Yegor Yefremov Nov. 19, 2015, 3:52 p.m. UTC 
From: Yegor Yefremov <yegorslists@googlemail.com>

This patch generates all *.pyc files from *.py located in
<target>/usr/lib/python*, before stripping the rootfs.

*.pyc generation on per package basis is disabled through
"--no-compile" option.

*.pyc generation for Python's internal packages is disabled
through --disable-pyc-build configure option.

This prevents modules from packages, that do not compile the *.py
files, from disappearing.

Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Cc: Samuel Martin <s.martin49@gmail.com>
---
Changes:
	v4: disable *.pyc creation for Python own modules (Thomas Petazzoni)
	v3: put pyc compilation command into own macro (Thomas Petazzoni)
	v2: move finalization code from root Makefile to python.mk and python3.mk (Thomas Petazzoni)

 package/pkg-python.mk      |  2 +-
 package/python/python.mk   | 16 +++++++++++++++-
 package/python3/python3.mk | 20 +++++++++++++++-----
 3 files changed, 31 insertions(+), 7 deletions(-)

Comments

Samuel Martin Dec. 11, 2015, 10:22 p.m. UTC | #1 
Hi Yegor, all,

On Thu, Nov 19, 2015 at 4:52 PM,  <yegorslists@googlemail.com> wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
>
> This patch generates all *.pyc files from *.py located in
> <target>/usr/lib/python*, before stripping the rootfs.
>
> *.pyc generation on per package basis is disabled through
> "--no-compile" option.
>
> *.pyc generation for Python's internal packages is disabled
> through --disable-pyc-build configure option.
>
> This prevents modules from packages, that do not compile the *.py
> files, from disappearing.

AFAI have tested, this change works as expected, except for broken
python modules.
In such cases, 'compileall.compile_dir(...)' does not raise any
exceptions when it failed to compile the *.py file, leading to the
unfortunate consequence: the *.pyc file is not generated and the *.py
file is lost if BR2_PACKAGE_PYTHON_PYC_ONLY is enabled.

This raises the question: do we want to make the build fail if some
*.pyc generation failed?
IMHO, the answer is yes.

Digging a bit this issue, the solution [2] inspired from [1] works
fine with python2 and python3.

Note: with python2: compileall.compile_dir fails on python-pyxml
package; with python3 I have not trigger any failure but I only tested
a limited number of packages

[1] http://stackoverflow.com/questions/615632/how-to-detect-errors-from-compileall-compile-dir
[2] http://code.bulix.org/2zk0mg-89949?raw

Regards,

>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> Cc: Samuel Martin <s.martin49@gmail.com>
> ---
> Changes:
>         v4: disable *.pyc creation for Python own modules (Thomas Petazzoni)
>         v3: put pyc compilation command into own macro (Thomas Petazzoni)
>         v2: move finalization code from root Makefile to python.mk and python3.mk (Thomas Petazzoni)
>
>  package/pkg-python.mk      |  2 +-
>  package/python/python.mk   | 16 +++++++++++++++-
>  package/python3/python3.mk | 20 +++++++++++++++-----
>  3 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/package/pkg-python.mk b/package/pkg-python.mk
> index b7a702d..9725d24 100644
> --- a/package/pkg-python.mk
> +++ b/package/pkg-python.mk
> @@ -253,7 +253,7 @@ ifndef $(2)_INSTALL_TARGET_CMDS
>  define $(2)_INSTALL_TARGET_CMDS
>         (cd $$($$(PKG)_BUILDDIR)/; \
>                 $$($$(PKG)_BASE_ENV) $$($$(PKG)_ENV) \
> -               $$($(2)_PYTHON_INTERPRETER) setup.py install \
> +               $$($(2)_PYTHON_INTERPRETER) setup.py install --no-compile \
>                 $$($$(PKG)_BASE_INSTALL_TARGET_OPTS) \
>                 $$($$(PKG)_INSTALL_TARGET_OPTS))
>  endef
> diff --git a/package/python/python.mk b/package/python/python.mk
> index 80ffbde..5ef3589 100644
> --- a/package/python/python.mk
> +++ b/package/python/python.mk
> @@ -143,7 +143,8 @@ PYTHON_CONF_OPTS += \
>         --disable-tk            \
>         --disable-nis           \
>         --disable-dbm           \
> -       --disable-pyo-build
> +       --disable-pyo-build     \
> +       --disable-pyc-build
>
>  # This is needed to make sure the Python build process doesn't try to
>  # regenerate those files with the pgen program. Otherwise, it builds
> @@ -217,12 +218,25 @@ PYTHON_PATH = $(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)/sysconfigdata/
>  $(eval $(autotools-package))
>  $(eval $(host-autotools-package))
>
> +define PYTHON_CREATE_PYC_FILES
> +       PYTHONPATH="$(PYTHON_PATH)" \
> +               $(HOST_DIR)/usr/bin/python$(PYTHON_VERSION_MAJOR) -c "import compileall; \
> +                       compileall.compile_dir('$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)')"
> +endef
> +
>  ifeq ($(BR2_PACKAGE_PYTHON_PYC_ONLY),y)
>  define PYTHON_FINALIZE_TARGET
> +       $(PYTHON_CREATE_PYC_FILES)
>         find $(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR) -name '*.py' -print0 | xargs -0 rm -f
>  endef
>  endif
>
> +ifeq ($(BR2_PACKAGE_PYTHON_PY_PYC),y)
> +define PYTHON_FINALIZE_TARGET
> +       $(PYTHON_CREATE_PYC_FILES)
> +endef
> +endif
> +
>  ifeq ($(BR2_PACKAGE_PYTHON_PY_ONLY),y)
>  define PYTHON_FINALIZE_TARGET
>         find $(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR) -name '*.pyc' -print0 | xargs -0 rm -f
> diff --git a/package/python3/python3.mk b/package/python3/python3.mk
> index fd5709a..c87092f 100644
> --- a/package/python3/python3.mk
> +++ b/package/python3/python3.mk
> @@ -79,10 +79,6 @@ ifeq ($(BR2_PACKAGE_PYTHON3_PYC_ONLY),y)
>  PYTHON3_CONF_OPTS += --enable-old-stdlib-cache
>  endif
>
> -ifeq ($(BR2_PACKAGE_PYTHON3_PY_ONLY),y)
> -PYTHON3_CONF_OPTS += --disable-pyc-build
> -endif
> -
>  ifeq ($(BR2_PACKAGE_PYTHON3_SQLITE),y)
>  PYTHON3_DEPENDENCIES += sqlite
>  else
> @@ -137,7 +133,8 @@ PYTHON3_CONF_OPTS += \
>         --disable-tk            \
>         --disable-nis           \
>         --disable-idle3         \
> -       --disable-pyo-build
> +       --disable-pyo-build     \
> +       --disable-pyc-build
>
>  # This is needed to make sure the Python build process doesn't try to
>  # regenerate those files with the pgen program. Otherwise, it builds
> @@ -208,12 +205,25 @@ PYTHON3_PATH = $(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)/sysconfigdat
>  $(eval $(autotools-package))
>  $(eval $(host-autotools-package))
>
> +define PYTHON3_CREATE_PYC_FILES
> +       PYTHONPATH="$(PYTHON3_PATH)" \
> +               $(HOST_DIR)/usr/bin/python$(PYTHON3_VERSION_MAJOR) -c "import compileall; \
> +                       compileall.compile_dir('$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)')"
> +endef
> +
>  ifeq ($(BR2_PACKAGE_PYTHON3_PYC_ONLY),y)
>  define PYTHON3_FINALIZE_TARGET
> +       $(PYTHON3_CREATE_PYC_FILES)
>         find $(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR) -name '*.py' -print0 | xargs -0 rm -f
>  endef
>  endif
>
> +ifeq ($(BR2_PACKAGE_PYTHON3_PY_PYC),y)
> +define PYTHON3_FINALIZE_TARGET
> +       $(PYTHON3_CREATE_PYC_FILES)
> +endef
> +endif
> +
>  ifeq ($(BR2_PACKAGE_PYTHON3_PY_ONLY),y)
>  define PYTHON3_FINALIZE_TARGET
>         find $(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR) -name '*.pyc' -print0 | xargs -0 rm -f
> --
> 2.1.4
>
Thomas Petazzoni Dec. 11, 2015, 10:32 p.m. UTC | #2 
Samuel,

On Fri, 11 Dec 2015 23:22:52 +0100, Samuel Martin wrote:

> AFAI have tested, this change works as expected, except for broken
> python modules.
> In such cases, 'compileall.compile_dir(...)' does not raise any
> exceptions when it failed to compile the *.py file, leading to the
> unfortunate consequence: the *.pyc file is not generated and the *.py
> file is lost if BR2_PACKAGE_PYTHON_PYC_ONLY is enabled.

Do you have some specific examples that exhibit this behavior?

Thanks,

Thomas
Samuel Martin Dec. 11, 2015, 10:58 p.m. UTC | #3 
On Fri, Dec 11, 2015 at 11:32 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Samuel,
>
> On Fri, 11 Dec 2015 23:22:52 +0100, Samuel Martin wrote:
>
>> AFAI have tested, this change works as expected, except for broken
>> python modules.
>> In such cases, 'compileall.compile_dir(...)' does not raise any
>> exceptions when it failed to compile the *.py file, leading to the
>> unfortunate consequence: the *.pyc file is not generated and the *.py
>> file is lost if BR2_PACKAGE_PYTHON_PYC_ONLY is enabled.
>
> Do you have some specific examples that exhibit this behavior?
Do you mean some case where this happens and the trace it does?

Here is a log exhibiting the kind of issue aforementioned [1] (got
with this defconfig [2] on top of master + this patch)
In this case, /usr/lib/python2.7/site-packages/_xmlplus/xpath/ParsedAbbreviatedRelativeLocationPath.py
and /usr/lib/python2.7/site-packages/_xmlplus/xpath/ParsedAbbreviatedAbsoluteLocationPath.py
cannot be compiled due to syntax error, so only their corresponding
*.pyc are missing in the final target fs.

[1] http://code.bulix.org/k3brrc-89950?raw
[2] http://code.bulix.org/idvog6-89952?raw

>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Regards,
Yegor Yefremov Dec. 14, 2015, 11:30 a.m. UTC | #4 
On Fri, Dec 11, 2015 at 11:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> On Fri, Dec 11, 2015 at 11:32 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> Samuel,
>>
>> On Fri, 11 Dec 2015 23:22:52 +0100, Samuel Martin wrote:
>>
>>> AFAI have tested, this change works as expected, except for broken
>>> python modules.
>>> In such cases, 'compileall.compile_dir(...)' does not raise any
>>> exceptions when it failed to compile the *.py file, leading to the
>>> unfortunate consequence: the *.pyc file is not generated and the *.py
>>> file is lost if BR2_PACKAGE_PYTHON_PYC_ONLY is enabled.
>>
>> Do you have some specific examples that exhibit this behavior?
> Do you mean some case where this happens and the trace it does?
>
> Here is a log exhibiting the kind of issue aforementioned [1] (got
> with this defconfig [2] on top of master + this patch)
> In this case, /usr/lib/python2.7/site-packages/_xmlplus/xpath/ParsedAbbreviatedRelativeLocationPath.py
> and /usr/lib/python2.7/site-packages/_xmlplus/xpath/ParsedAbbreviatedAbsoluteLocationPath.py
> cannot be compiled due to syntax error, so only their corresponding
> *.pyc are missing in the final target fs.
>
> [1] http://code.bulix.org/k3brrc-89950?raw
> [2] http://code.bulix.org/idvog6-89952?raw

I could reproduce the syntax error issue for pyxml package:

Compiling /home/YegorYefremov/MyProjects/oss/buildroot/testing/target/usr/lib/python2.7/site-packages/_xmlplus/xpath/ParsedAbbreviatedAbsoluteLocationPath.py
...
  File "/home/YegorYefremov/MyProjects/oss/buildroot/testing/target/usr/lib/python2.7/site-packages/_xmlplus/xpath/ParsedAbbreviatedAbsoluteLocationPath.py",
line 27
    as = ParsedAxisSpecifier.ParsedAxisSpecifier('descendant-or-self')
     ^
SyntaxError: invalid syntax

Compiling /home/YegorYefremov/MyProjects/oss/buildroot/testing/target/usr/lib/python2.7/site-packages/_xmlplus/xpath/ParsedAbbreviatedRelativeLocationPath.py
...
  File "/home/YegorYefremov/MyProjects/oss/buildroot/testing/target/usr/lib/python2.7/site-packages/_xmlplus/xpath/ParsedAbbreviatedRelativeLocationPath.py",
line 31
    as = ParsedAxisSpecifier.ParsedAxisSpecifier('descendant-or-self')
     ^
SyntaxError: invalid syntax

What should we do next? I'd suggest to commit this patch as is, add
Samuel's compiling error detection patch and a syntax fix patch for
pyxml. I'll try to hack it.

What do you think about it?

Yegor
diff mbox

Patch

diff --git a/package/pkg-python.mk b/package/pkg-python.mk
index b7a702d..9725d24 100644
--- a/package/pkg-python.mk
+++ b/package/pkg-python.mk
@@ -253,7 +253,7 @@  ifndef $(2)_INSTALL_TARGET_CMDS
 define $(2)_INSTALL_TARGET_CMDS
 	(cd $$($$(PKG)_BUILDDIR)/; \
 		$$($$(PKG)_BASE_ENV) $$($$(PKG)_ENV) \
-		$$($(2)_PYTHON_INTERPRETER) setup.py install \
+		$$($(2)_PYTHON_INTERPRETER) setup.py install --no-compile \
 		$$($$(PKG)_BASE_INSTALL_TARGET_OPTS) \
 		$$($$(PKG)_INSTALL_TARGET_OPTS))
 endef
diff --git a/package/python/python.mk b/package/python/python.mk
index 80ffbde..5ef3589 100644
--- a/package/python/python.mk
+++ b/package/python/python.mk
@@ -143,7 +143,8 @@  PYTHON_CONF_OPTS += \
 	--disable-tk		\
 	--disable-nis		\
 	--disable-dbm		\
-	--disable-pyo-build
+	--disable-pyo-build	\
+	--disable-pyc-build
 
 # This is needed to make sure the Python build process doesn't try to
 # regenerate those files with the pgen program. Otherwise, it builds
@@ -217,12 +218,25 @@  PYTHON_PATH = $(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)/sysconfigdata/
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))
 
+define PYTHON_CREATE_PYC_FILES
+	PYTHONPATH="$(PYTHON_PATH)" \
+		$(HOST_DIR)/usr/bin/python$(PYTHON_VERSION_MAJOR) -c "import compileall; \
+			compileall.compile_dir('$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)')"
+endef
+
 ifeq ($(BR2_PACKAGE_PYTHON_PYC_ONLY),y)
 define PYTHON_FINALIZE_TARGET
+	$(PYTHON_CREATE_PYC_FILES)
 	find $(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR) -name '*.py' -print0 | xargs -0 rm -f
 endef
 endif
 
+ifeq ($(BR2_PACKAGE_PYTHON_PY_PYC),y)
+define PYTHON_FINALIZE_TARGET
+	$(PYTHON_CREATE_PYC_FILES)
+endef
+endif
+
 ifeq ($(BR2_PACKAGE_PYTHON_PY_ONLY),y)
 define PYTHON_FINALIZE_TARGET
 	find $(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR) -name '*.pyc' -print0 | xargs -0 rm -f
diff --git a/package/python3/python3.mk b/package/python3/python3.mk
index fd5709a..c87092f 100644
--- a/package/python3/python3.mk
+++ b/package/python3/python3.mk
@@ -79,10 +79,6 @@  ifeq ($(BR2_PACKAGE_PYTHON3_PYC_ONLY),y)
 PYTHON3_CONF_OPTS += --enable-old-stdlib-cache
 endif
 
-ifeq ($(BR2_PACKAGE_PYTHON3_PY_ONLY),y)
-PYTHON3_CONF_OPTS += --disable-pyc-build
-endif
-
 ifeq ($(BR2_PACKAGE_PYTHON3_SQLITE),y)
 PYTHON3_DEPENDENCIES += sqlite
 else
@@ -137,7 +133,8 @@  PYTHON3_CONF_OPTS += \
 	--disable-tk		\
 	--disable-nis		\
 	--disable-idle3		\
-	--disable-pyo-build
+	--disable-pyo-build	\
+	--disable-pyc-build
 
 # This is needed to make sure the Python build process doesn't try to
 # regenerate those files with the pgen program. Otherwise, it builds
@@ -208,12 +205,25 @@  PYTHON3_PATH = $(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)/sysconfigdat
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))
 
+define PYTHON3_CREATE_PYC_FILES
+	PYTHONPATH="$(PYTHON3_PATH)" \
+		$(HOST_DIR)/usr/bin/python$(PYTHON3_VERSION_MAJOR) -c "import compileall; \
+			compileall.compile_dir('$(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR)')"
+endef
+
 ifeq ($(BR2_PACKAGE_PYTHON3_PYC_ONLY),y)
 define PYTHON3_FINALIZE_TARGET
+	$(PYTHON3_CREATE_PYC_FILES)
 	find $(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR) -name '*.py' -print0 | xargs -0 rm -f
 endef
 endif
 
+ifeq ($(BR2_PACKAGE_PYTHON3_PY_PYC),y)
+define PYTHON3_FINALIZE_TARGET
+	$(PYTHON3_CREATE_PYC_FILES)
+endef
+endif
+
 ifeq ($(BR2_PACKAGE_PYTHON3_PY_ONLY),y)
 define PYTHON3_FINALIZE_TARGET
 	find $(TARGET_DIR)/usr/lib/python$(PYTHON3_VERSION_MAJOR) -name '*.pyc' -print0 | xargs -0 rm -f