[1/1] package/opencv3: python support needs .py modules
diff mbox series

Message ID 20190905184403.19368-1-fontaine.fabrice@gmail.com
State New
Headers show
Series
  • [1/1] package/opencv3: python support needs .py modules
Related show

Commit Message

Fabrice Fontaine Sept. 5, 2019, 6:44 p.m. UTC
Fixes:
 - https://bugs.buildroot.org/show_bug.cgi?id=12171

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/opencv3/Config.in | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Thomas Petazzoni Sept. 5, 2019, 6:49 p.m. UTC | #1
Hello Fabrice,

On Thu,  5 Sep 2019 20:44:03 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> Fixes:
>  - https://bugs.buildroot.org/show_bug.cgi?id=12171
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

Thanks for the patch. A better commit log would be nice. But more
importantly, do we understand why it absolutely needs the .py file ?

Thomas
Fabrice Fontaine Sept. 5, 2019, 6:55 p.m. UTC | #2
Hello Thomas,

Le jeu. 5 sept. 2019 à 20:49, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> Hello Fabrice,
>
> On Thu,  5 Sep 2019 20:44:03 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > Fixes:
> >  - https://bugs.buildroot.org/show_bug.cgi?id=12171
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>
> Thanks for the patch. A better commit log would be nice. But more
> importantly, do we understand why it absolutely needs the .py file ?
It uses it to get version_info (here is an exract of
https://github.com/opencv/opencv/blob/f33f88de3138366bc370f5d49d08d351179b910d/modules/python/package/cv2/__init__.py):

load_first_config(['config.py'], True)
load_first_config([
'config-{}.{}.py'.format(sys.version_info[0], sys.version_info[1]),
'config-{}.py'.format(sys.version_info[0])
], True)
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice
Arnout Vandecappelle Sept. 8, 2019, 3:48 p.m. UTC | #3
On 05/09/2019 20:44, Fabrice Fontaine wrote:
> Fixes:
>  - https://bugs.buildroot.org/show_bug.cgi?id=12171

 To make sure this thread stays readable, I'll repeat the relevant part of the
bug report, which should have been part of the commit message:

When importing cv2 in python 3.7, the program stops with:

  File "usr/lib/python3.7/site-packages/cv2/__init__.py", line 89, in <module>
  File "usr/lib/python3.7/site-packages/cv2/__init__.py", line 58, in bootstrap
  File "usr/lib/python3.7/site-packages/cv2/__init__.py", line 56, in
load_first_config
ImportError: OpenCV loader: missing configuration file: ['config.py']. Check
OpenCV installation.

Since buildroot removed all *.py, leaving the corresponding *.pyc, the above
config.py file is not in target.

> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  package/opencv3/Config.in | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/package/opencv3/Config.in b/package/opencv3/Config.in
> index cf7763d4ae..a0bdea529e 100644
> --- a/package/opencv3/Config.in
> +++ b/package/opencv3/Config.in
> @@ -157,6 +157,7 @@ config BR2_PACKAGE_OPENCV3_LIB_PHOTO
>  config BR2_PACKAGE_OPENCV3_LIB_PYTHON
>  	bool "python"
>  	depends on BR2_PACKAGE_PYTHON || BR2_PACKAGE_PYTHON3
> +	depends on !BR2_PACKAGE_PYTHON_PYC_ONLY # runtime

 This is really a very large hammer to squash the problem IMO.

 I think it would be better to have infra to exclude files from the cleanup,
similar to how we can avoid files being stripped.


 Regards,
 Arnout


>  	depends on BR2_PACKAGE_PYTHON_NUMPY_ARCH_SUPPORTS
>  	depends on BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_MUSL # python-numpy
>  	select BR2_PACKAGE_PYTHON_NUMPY
> @@ -164,8 +165,9 @@ config BR2_PACKAGE_OPENCV3_LIB_PYTHON
>  	  Include opencv_python module into the OpenCV build.  No
>  	  python example is installed.
>  
> -comment "python support needs glibc or musl"
> -	depends on !(BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_MUSL)
> +comment "python support needs .py modules and glibc or musl"
> +	depends on BR2_PACKAGE_PYTHON_PYC_ONLY || \
> +		!(BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_MUSL)
>  
>  config BR2_PACKAGE_OPENCV3_LIB_SHAPE
>  	bool "shape"
>
Thomas Petazzoni Sept. 13, 2019, 8:10 p.m. UTC | #4
On Sun, 8 Sep 2019 17:48:46 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> On 05/09/2019 20:44, Fabrice Fontaine wrote:
> > Fixes:
> >  - https://bugs.buildroot.org/show_bug.cgi?id=12171  
> 
>  To make sure this thread stays readable, I'll repeat the relevant part of the
> bug report, which should have been part of the commit message:
> 
> When importing cv2 in python 3.7, the program stops with:
> 
>   File "usr/lib/python3.7/site-packages/cv2/__init__.py", line 89, in <module>
>   File "usr/lib/python3.7/site-packages/cv2/__init__.py", line 58, in bootstrap
>   File "usr/lib/python3.7/site-packages/cv2/__init__.py", line 56, in
> load_first_config
> ImportError: OpenCV loader: missing configuration file: ['config.py']. Check
> OpenCV installation.
> 
> Since buildroot removed all *.py, leaving the corresponding *.pyc, the above
> config.py file is not in target.

It is a bit weird/funky for OpenCV to require a .py file just for this.
It contains a single variable:

BINARIES_PATHS = [
    @CMAKE_PYTHON_BINARIES_PATH@
] + BINARIES_PATHS

And then the code looks like this:

    def load_first_config(fnames, required=True):
        for fname in fnames:
            fpath = os.path.join(LOADER_DIR, fname)
            if not os.path.exists(fpath):
                if DEBUG: print('OpenCV loader: config not found, skip: {}'.format(fpath))
                continue
            if DEBUG: print('OpenCV loader: loading config: {}'.format(fpath))
            exec_file_wrapper(fpath, g_vars, l_vars)
            return True
        if required:
            raise ImportError('OpenCV loader: missing configuration file: {}. Check OpenCV installation.'.format(fnames))

    load_first_config(['config.py'], True)
    load_first_config([
        'config-{}.{}.py'.format(sys.version_info[0], sys.version_info[1]),
        'config-{}.py'.format(sys.version_info[0])
    ], True)

So all it does is override the BINARIES_PATH array with some additional
value. I'm sure we can replace this with reading a simple text file
that does the exact same thing without requiring to compile/exec a .py
file.

Best regards,

Thomas
Arnout Vandecappelle Sept. 14, 2019, 5:18 p.m. UTC | #5
On 13/09/2019 22:10, Thomas Petazzoni wrote:
> On Sun, 8 Sep 2019 17:48:46 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>> On 05/09/2019 20:44, Fabrice Fontaine wrote:
>>> Fixes:
>>>  - https://bugs.buildroot.org/show_bug.cgi?id=12171  
>>
>>  To make sure this thread stays readable, I'll repeat the relevant part of the
>> bug report, which should have been part of the commit message:
>>
>> When importing cv2 in python 3.7, the program stops with:
>>
>>   File "usr/lib/python3.7/site-packages/cv2/__init__.py", line 89, in <module>
>>   File "usr/lib/python3.7/site-packages/cv2/__init__.py", line 58, in bootstrap
>>   File "usr/lib/python3.7/site-packages/cv2/__init__.py", line 56, in
>> load_first_config
>> ImportError: OpenCV loader: missing configuration file: ['config.py']. Check
>> OpenCV installation.
>>
>> Since buildroot removed all *.py, leaving the corresponding *.pyc, the above
>> config.py file is not in target.
> 
> It is a bit weird/funky for OpenCV to require a .py file just for this.
> It contains a single variable:
> 
> BINARIES_PATHS = [
>     @CMAKE_PYTHON_BINARIES_PATH@
> ] + BINARIES_PATHS
> 
> And then the code looks like this:
> 
>     def load_first_config(fnames, required=True):
>         for fname in fnames:
>             fpath = os.path.join(LOADER_DIR, fname)
>             if not os.path.exists(fpath):
>                 if DEBUG: print('OpenCV loader: config not found, skip: {}'.format(fpath))
>                 continue
>             if DEBUG: print('OpenCV loader: loading config: {}'.format(fpath))
>             exec_file_wrapper(fpath, g_vars, l_vars)
>             return True
>         if required:
>             raise ImportError('OpenCV loader: missing configuration file: {}. Check OpenCV installation.'.format(fnames))
> 
>     load_first_config(['config.py'], True)
>     load_first_config([
>         'config-{}.{}.py'.format(sys.version_info[0], sys.version_info[1]),
>         'config-{}.py'.format(sys.version_info[0])
>     ], True)
> 
> So all it does is override the BINARIES_PATH array with some additional
> value. I'm sure we can replace this with reading a simple text file
> that does the exact same thing without requiring to compile/exec a .py
> file.

 I agree that this approach is insanely and needlessly complicated. However, do
we really want to produce a pretty large package patch to simplify this, which
is probably not even upstreamable because it would break existing installations
that have overridden config.py?


 Regards,
 Arnout


> 
> Best regards,
> 
> Thomas
>
Thomas Petazzoni Sept. 15, 2019, 8:52 p.m. UTC | #6
On Sat, 14 Sep 2019 19:18:29 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>  I agree that this approach is insanely and needlessly complicated. However, do
> we really want to produce a pretty large package patch to simplify this, which
> is probably not even upstreamable because it would break existing installations
> that have overridden config.py?

I don't think it is meant to be overridden. It's installed in /usr, it's
not a configuration file.

Another approach would be to see if it's possible to achieve the same,
but using a .pyc file, like using "import" or something like this ?

Thomas

Patch
diff mbox series

diff --git a/package/opencv3/Config.in b/package/opencv3/Config.in
index cf7763d4ae..a0bdea529e 100644
--- a/package/opencv3/Config.in
+++ b/package/opencv3/Config.in
@@ -157,6 +157,7 @@  config BR2_PACKAGE_OPENCV3_LIB_PHOTO
 config BR2_PACKAGE_OPENCV3_LIB_PYTHON
 	bool "python"
 	depends on BR2_PACKAGE_PYTHON || BR2_PACKAGE_PYTHON3
+	depends on !BR2_PACKAGE_PYTHON_PYC_ONLY # runtime
 	depends on BR2_PACKAGE_PYTHON_NUMPY_ARCH_SUPPORTS
 	depends on BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_MUSL # python-numpy
 	select BR2_PACKAGE_PYTHON_NUMPY
@@ -164,8 +165,9 @@  config BR2_PACKAGE_OPENCV3_LIB_PYTHON
 	  Include opencv_python module into the OpenCV build.  No
 	  python example is installed.
 
-comment "python support needs glibc or musl"
-	depends on !(BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_MUSL)
+comment "python support needs .py modules and glibc or musl"
+	depends on BR2_PACKAGE_PYTHON_PYC_ONLY || \
+		!(BR2_TOOLCHAIN_USES_GLIBC || BR2_TOOLCHAIN_USES_MUSL)
 
 config BR2_PACKAGE_OPENCV3_LIB_SHAPE
 	bool "shape"