zbar icon indicating copy to clipboard operation
zbar copied to clipboard

python binding and build enhancement

Open wqh17101 opened this issue 4 years ago • 7 comments

add zbar_llite to support building python whl from any OS add setup_plus.py to support building wheels with pre-compilered zbar lib add ZBAR_LITE marco for future updates add patch version to zbar python module

wqh17101 avatar Feb 12 '21 11:02 wqh17101

Not sure what you're trying to achive with this patch series. Is it just due to build issues with Python bindings on Windows?

If so, the problem is very likely because PYTHON_LIBS was not set. In the case of Github's workflow, it should be initialized with:

PYTHON_LIBS= -LD:/a/_temp/msys/msys64/mingw64/lib -lpython3.8  -lm -lversion -lshlwapi -lm  

I have a patch fixing it, but it has a drawback: it causes MacOS build to fail.

mchehab avatar Feb 12 '21 17:02 mchehab

The patch which auto-detect the LDFLAGS is:

Author: Mauro Carvalho Chehab <[email protected]>
Date:   Fri Feb 12 17:08:13 2021 +0100

    configure.ac: parse python LDFLAGS
    
    At least on Windows, python bindings produce lots of linkedit
    errors, like:
    
            D:/a/_temp/msys/msys64/mingw32/lib/gcc/i686-w64-mingw32/10.2.0/../../../../i686-w64-mingw32/bin/ld.exe: python/.libs/zbar_la-zbarmodule.o: in function `Py_DECREF':
            D:/a/_temp/msys/msys64/mingw32/include/python3.8/object.h:478: undefined reference to `_Py_Dealloc'
            D:/a/_temp/msys/msys64/mingw32/lib/gcc/i686-w64-mingw32/10.2.0/../../../../i686-w64-mingw32/bin/ld.exe: python/.libs/zbar_la-zbarmodule.o: in function `increase_verbosity':
            D:\a\zbar\zbar/python/zbarmodule.c:173: undefined reference to `PyArg_ParseTuple'
    
    That's probably because the python libraries were not included.
    
    So, add a LDFLAGS to configure.
    
    Signed-off-by: Mauro Carvalho Chehab <[email protected]>

diff --git a/configure.ac b/configure.ac
index f1f0c84a97e6..6555f4e7c7c7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -560,7 +560,7 @@ Please notice that PYTHON var, if especified, takes precedence.])],
 
 AC_ARG_VAR([PYTHON_CONFIG], [full path to python-config program])
 AC_ARG_VAR([PYTHON_CFLAGS], [compiler flags for building python extensions])
-AC_ARG_VAR([PYTHON_LIBS], [linker flags for building python extensions])
+AC_ARG_VAR([PYTHON_LDFLAGS], [linker flags for building python extensions])
 
 AC_ARG_VAR([PYGTK_H2DEF], [full path to PyGTK h2def.py module (python2 only)])
 AC_ARG_VAR([PYGTK_CODEGEN], [full path to pygtk-codegen program (python2 only)])
@@ -593,7 +593,13 @@ AS_IF([test "x$PYTHON_VERSION" != "x" && test "x$with_python" != "xno"],
      [test -x "$PYTHON-config"],
      [PYTHON_CFLAGS=`$PYTHON-config --cflags`],
      [PYTHON_CFLAGS=`$PYTHON -c 'import distutils.sysconfig as s, sys; sys.stdout.write(" ".join(s.get_config_vars("CFLAGS")) + " -I"+s.get_python_inc() + " -I"+s.get_python_inc(plat_specific=True))'`])
-
+   AS_IF([test "x$PYTHON_LDFLAGS" != "x"],
+     [],
+     [test "x$PYTHON_CONFIG" != "x" && test -x "$PYTHON_CONFIG"],
+     [PYTHON_LDFLAGS=`$PYTHON_CONFIG --ldflags`],
+     [test -x "$PYTHON-config"],
+     [PYTHON_LDFLAGS=`$PYTHON-config --ldflags`],
+     [PYTHON_LDFLAGS=`$PYTHON -c 'import distutils.sysconfig as s, sys; sys.stdout.write(" ".join(s.get_config_vars("LDFLAGS")) + " -I"+s.get_python_inc() + " -I"+s.get_python_inc(plat_specific=True))'`])
    dnl check that #include <Python.h> compiles (bug #3092663)
    CPPFLAGS_save="$CPPFLAGS"
    CPPFLAGS="$CPPFLAGS $PYTHON_CFLAGS"
diff --git a/pygtk/Makefile.am.inc b/pygtk/Makefile.am.inc
index 9b34c3dfef69..033d601c4f0a 100644
--- a/pygtk/Makefile.am.inc
+++ b/pygtk/Makefile.am.inc
@@ -2,9 +2,9 @@ pyexec_LTLIBRARIES += pygtk/zbarpygtk.la
 pygtk_zbarpygtk_la_CPPFLAGS = \
     $(GTK_CFLAGS) $(PYTHON_CFLAGS) $(PYGTK_CFLAGS) $(AM_CPPFLAGS)
 pygtk_zbarpygtk_la_LDFLAGS = -shared -module -avoid-version -export-dynamic \
-    -export-symbols-regex initzbarpygtk
+     -export-symbols-regex initzbarpygtk $(PYTHON_LDFLAGS)
 pygtk_zbarpygtk_la_LIBADD = \
-    $(PYTHON_LIBS) $(PYGTK_LIBS) gtk/libzbargtk.la $(AM_LIBADD)
+    $(PYGTK_LIBS) gtk/libzbargtk.la $(AM_LIBADD)
 
 pygtk_zbarpygtk_la_DEPENDENCIES = gtk/libzbargtk.la
 dist_pygtk_zbarpygtk_la_SOURCES = pygtk/zbarpygtkmodule.c
diff --git a/python/Makefile.am.inc b/python/Makefile.am.inc
index f61214fe6488..ecc66df866a3 100644
--- a/python/Makefile.am.inc
+++ b/python/Makefile.am.inc
@@ -1,8 +1,9 @@
 pyexec_LTLIBRARIES += python/zbar.la
 python_zbar_la_CPPFLAGS = $(PYTHON_CFLAGS) $(AM_CPPFLAGS)
 python_zbar_la_LDFLAGS = -shared -module -avoid-version -export-dynamic \
-    -export-symbols-regex '(initzbar|PyInit_zbar)'
-python_zbar_la_LIBADD = $(PYTHON_LIBS) zbar/libzbar.la $(AM_LIBADD)
+    -export-symbols-regex '(initzbar|PyInit_zbar)' -no-undefined \
+    $(PYTHON_LDFLAGS)
+python_zbar_la_LIBADD = zbar/libzbar.la $(AM_LIBADD)
 
 python_zbar_la_SOURCES = python/zbarmodule.c python/zbarmodule.h \
     python/enum.c python/exception.c python/symbol.c python/symbolset.c \

mchehab avatar Feb 12 '21 17:02 mchehab

No, in the old version , the way python work with is to build a zbarlib and a python wrapper, there are two libs. But for now using a pythonic way we can make a installer(wheel) with one lib contain above , it is easy for user to use without downloading the large source code to build. So keep a original for some users, and provide a more convinient way is a good idea

wqh17101 avatar Feb 12 '21 17:02 wqh17101

Let me explain how ZBAR-LITE work. run preparation.sh will copy the source code from ZBAR to a workdir named zbar_lite. then if you want to install , run python setup.py install . It will intall a python package to your OS where other python packages in. if you want to generate a python whl to release or just want to make a Plug-and-Play installer (.exe is also available too). you can run python setup.py bdist_wheel . It will build a whl to the build dir(defalut is zbar_lite/build). As setup.py ` s param , users can easily specify the path or other options.

wqh17101 avatar Feb 12 '21 18:02 wqh17101

Threre are three ways for Python users to use zbar now.

  1. build and install libzbar and python zbar wrapper -- both the old one and setup_plus provide it .
  2. only build the python zbar wrapper, and provide a libzbar when using. --setup_plus provide it this two above for now is the python/setup_plus.py which is compatible with the old setup.py As a bakup i keep the old setup.py , if you think there is no need to keep it any more, i can remove it or just overwrite it with the content of setup_plus.py
  3. use zbar-lite which build only one lib with wrapper and libzbar -- zbar-lite provide it. or just use pip install zbar-lite to install from pypi.org which is the best way for python users.

wqh17101 avatar Feb 13 '21 04:02 wqh17101

No, in the old version , the way python work with is to build a zbarlib and a python wrapper, there are two libs. But for now using a pythonic way we can make a installer(wheel) with one lib contain above , it is easy for user to use without downloading the large source code to build. So keep a original for some users, and provide a more convinient way is a good idea

Keeping the original and the new one as separate libraries is a bad idea, as, if someone compiles something against the libraries, there will be duplicated symbols. The best is to add the newer functions needed by the new python code at the same library (don't forget to increment library's version at configure.ac), and add a new configure.ac option to allow enabling it at compile time.

mchehab avatar Feb 15 '21 07:02 mchehab

Not sure what you're trying to achive with this patch series. Is it just due to build issues with Python bindings on Windows?

If so, the problem is very likely because PYTHON_LIBS was not set. In the case of Github's workflow, it should be initialized with:

PYTHON_LIBS= -LD:/a/_temp/msys/msys64/mingw64/lib -lpython3.8  -lm -lversion -lshlwapi -lm  

I have a patch fixing it, but it has a drawback: it causes MacOS build to fail. Fixed. The problem is that Windows build required a different ld flag(-no-undefined):

   AS_IF([test "x$win32" = "xyes"], PYTHON_LDFLAGS="$PYTHON_LDFLAGS -no-undefined")

mchehab avatar Feb 15 '21 07:02 mchehab