| Message ID | 20190905184403.19368-1-fontaine.fabrice@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] package/opencv3: python support needs .py modules | expand |
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
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
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" >
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
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 >
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
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"
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(-)