Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build error for SWIG 4.2.1 Python 3.12.5, potential syntax error in importsym.py #4054

Open
Axiinos opened this issue Aug 26, 2024 · 17 comments

Comments

@Axiinos
Copy link

Axiinos commented Aug 26, 2024

Describe the bug

The build error is due to the original SWIG build, and it needs to be refreshed by rerunning the input/output formatter manually to build properly. running make in either swig folder or swig/python will produce errors.

Steps to reproduce

The steps are more to fix the bug that is still in the source code.

  1. install latest SWIG version by building it form their own source.
    "Required libraries"
    sudo apt install build-essential libpcre2-dev libpcre3-dev autoconf automake libtool bison git libboost-dev golang-go guile-2.2-dev nodejs node-gyp libwebkit2gtk-4.0-dev lua5.3 liblua5.3-dev mono-devel octave liboctave-dev default-jdk-headless php-cli php-dev python3-dev r-base ruby ruby-dev tcl-dev scilab libxml2-dev
    "this can also be done in a env instead of system python."

  2. install python 3.12.5

  3. install setuptools with pip, it does not come with normal python install and is required.

  4. install all the required build libraries that SWIG needs to run and rebuild the output file for pjsua2.i and its links that turn into pjsua2_wrap.ccp and pjsua2_wrap.h.
    "gcc is needed"

5.run ./configure CFLAGS="-fPIC" in the root project folder
"if you are running a virtual environment like me, run with the following arg : --build:x86_64-ubuntu-linux"

  1. after it is done, run make dep && make to build the libraries.

  2. return to the swig folder, and run swig to rebuild the generated swig install file. "swig -python -c++ -I../../../pjsip/include -I../../../pjlib/include -I../../../pjlib-util/include -I../../../pjmedia/include -I../../../pjnath/include -o python/pjsua2_wrap.cpp pjsua2.i"
    this then rebuilds the wrapper properly, its needed due to the way that it needs to work between latest SWIG version and python 3.12* versions.

  3. go to the python folder and do a "make" inside of it.
    "this should now build properly, if you try to run make in swig, you will get an error about linking issues. the errors have been attached"

  4. install the package with "pip install ." this will build it and install it as a wheel to either system or venv, depending on if you have activated a venv ofcourse.

PJSIP version

2.14.1

Context

System: Ubuntu 24.04 LTS in a HyperV environment, Windows 10 latest build.
"i specified in the ./configure what host to use, which was : "./configure CFLAGS="-fPIC" --build=x86_64-ubuntu-linux" and it build succesfully."

Python was used in a venv as not to mess with system wide packages.

the problem is running the make file or just running "pythonxxx setup.py build" and the errors have been attached.
running the swig builder manually does not produce any problems as described in the steps to reproduce.

also, do note, that the importsym.py has print commands that lack parenthesis, running the importsym.py directly will produce error logs for each print missing parenthesis and should be fixed. images attached show 2 lines, but there are more.
image

errorlog.txt
errorlog contains 2 outputs where it failed to build with two different errors. "top half part seems to be related to me building a succesful version that is newer then current attempt. and second lower half is errors where it cant find files."
none of these happen with a manual swig build, then "pythonxxx setup.py build | pip install ."
pythonxxx simply means version, i am using latest python version 3.12.5 in the venv.

Log, call stack, etc

x86_64-linux-gnu-gcc: error: Successfully remade target file 'cflags'.: linker input file not found: No such file or directory
error: command '/usr/bin/x86_64-linux-gnu-gcc' failed with exit code 1
Reaping losing child 0x58a72ddcf6e0 PID 1320241 
make[1]: *** [Makefile:40: _pjsua2.so] Error 1
Removing child 0x58a72ddcf6e0 PID 1320241 from chain.
Reaping losing child 0x62eae9362e70 PID 1320239 
make: *** [Makefile:27: python] Error 2
Removing child 0x62eae9362e70 PID 1320239 from chain.

log file attached in Context.
@Axiinos
Copy link
Author

Axiinos commented Aug 26, 2024

also, for future note, distutils.core package has been completely removed in python version 3.12 and is replaced with setuptools package instead

@Axiinos
Copy link
Author

Axiinos commented Aug 26, 2024

i have also found a small solution to get docstrings into python with doxygen, then modifying the resulting pjsua2.py generated by swig to take the data from the property(arg1, arg2, doc=r"""""" and place the docstring outside on a new line below the actual call, allowing to show the documentation in the editor for python

image

transform_docstrings.txt

the text file contains the python code to fix the docstring problems in the file before running the "pip install ." command for installing the package.

@trengginas
Copy link
Member

I tested here using python 3.12 with swig 4.2.1 and didn't have the same error.
By specifying ./configure CFLAGS="-fPIC", the build went successfully with the swig wrapper.
I haven't tested with python 3.12.5 since I needed to build and install it manually from the source and somehow it is not working on my machine.

For importsym.py, it is intended to be used with python2 and it is used to generate the C symbols to be used by the swig wrapper (symbol.i). On a normal build, it is not necessary to regenerate the symbols each time.

For the deprecated distutils, please check #4057.

@Axiinos
Copy link
Author

Axiinos commented Aug 30, 2024

I tested here using python 3.12 with swig 4.2.1 and didn't have the same error. By specifying ./configure CFLAGS="-fPIC", the build went successfully with the swig wrapper. I haven't tested with python 3.12.5 since I needed to build and install it manually from the source and somehow it is not working on my machine.

For importsym.py, it is intended to be used with python2 and it is used to generate the C symbols to be used by the swig wrapper (symbol.i). On a normal build, it is not necessary to regenerate the symbols each time.

For the deprecated distutils, please check #4057.

Alright, glad i could get some understanding with the importsym.

And thanks for bumping the distutils to setuptools.

The reason your build failed with 3.12.5 "since you had to build it manually" doesnt add a python3 in the bin, it adds a python3.12.5 instead, and has to be edited in the Makefile, but it really isnt necessary, the 3.12.5 version is compatible with the highest apt install 3.12 version, since there isnt any major changes between the versions

As i mentioned in last comment, i wrote a little python script to allow enabling doxygen, since doxygen outputs docstrings in js docstring format, this small line of code opens up the pjsua2 file and corrects all the doc variables that have a "doc=r******" variable inside the calls of property

import re

file_path = 'pjsua2.py'

with open(file_path, 'r') as file:
    content = file.read()

# Regular expression to match the property with doc=...
pattern = re.compile(r'(\w+\s*=\s*property\([^,]+,[^,]+),\s*doc=r?\"\"\"(.*?)\"\"\"\)', re.DOTALL)

# Replace the matched pattern
new_content = pattern.sub(r'\1)\n    """\2"""', content)

with open(file_path, 'w') as file:
    file.write(new_content)

@Axiinos Axiinos closed this as completed Aug 30, 2024
@Axiinos Axiinos reopened this Aug 30, 2024
@Axiinos
Copy link
Author

Axiinos commented Aug 30, 2024

sorry, accidentally hit the wrong button.

@trengginas
Copy link
Member

I managed to get Python3.12.5 on my Mac, but there was no error as described. Looking at your description, it seems you are regenerating the swig wrapper manually:

swig -python -c++ -I../../../pjsip/include -I../../../pjlib/include -I../../../pjlib-util/include -I../../../pjmedia/include -I../../../pjnath/include -o python/pjsua2_wrap.cpp pjsua2.i

This could be skipped from the 'make' command if a built swig wrapper existed.
It is recommended to rebuilt the wrapper (e.g.: when there is a version update) by calling make clean or make realclean before calling make.

Another issue I encounter is the error when calling make install or python setup.py install. You can use the command python3 -m pip install . to install the pjsua2 module.

Regarding the python script (transform_docstring.txt), I tried to run it however the output (pjsua2.py) is still the same as the one generated by swig. Ideally, the generated pjsua2.py from swig has the required format without any tools to change it.

@Axiinos
Copy link
Author

Axiinos commented Sep 3, 2024

Regarding the python script (transform_docstring.txt), I tried to run it however the output (pjsua2.py) is still the same as the one generated by swig. Ideally, the generated pjsua2.py from swig has the required format without any tools to change it.

did you run the swig wrapper manually with the -doxygen at the very start of the command?

the method i did was use doxygen to produce the code comments from c++, which made it into Java doc=r****** inside the property like so : property(arg0, arg1, docstring)

the script needs to be in the python folder and executed before "pip install ." command, it searches for doc=r, copying the docstring and placing it outside the parentheses

Unedited version
unedited

and the after edit "running the script"
after edit

granted, it doesnt look for doc=, only doc=r

@trengginas
Copy link
Member

I appreciate the clarification. By default, -doxygen is not used. After regenerating the python wrapper, using VS Code, I can confirm that the script works (e.g.: changing the property doc). I couldn't find the option for swig to define the docstring differently, but couldn't find any. The other option should be the option in the IDE to show the doc specified in the property definition.

In summary, for users that want to have the doc included in the Python generated wrapper:

@Axiinos
Copy link
Author

Axiinos commented Sep 4, 2024

Yeah, i have been trying the same with attempting to finding the options for it, and nothing could be found, so thats why i came up with this solution instead

@Axiinos
Copy link
Author

Axiinos commented Sep 10, 2024

alright, now after testing a few things i am trying the pygui sample application, and setting USE_THREADS = True

digging around, when it reaches the point of self.ep.libstart() it is hit with a segfault, SIGSEGV.

i run gdb and run its traceback, getting the following result.
image

i am not sure if this is directly a fault between SWIG or a change in this version of Python?

i did read that this versions from 3.11 python and forward uses c11, and i dont know if this is the cause either?

@sauwming
Copy link
Member

@Axiinos
Copy link
Author

Axiinos commented Sep 11, 2024

i know its been described, it was more so if its been looked at for a potential fix

@Axiinos
Copy link
Author

Axiinos commented Sep 11, 2024

actually found the solution, removing the -DSWIG_NO_EXPORT_ITERATOR_METHODS from the swig call fixed it entirely.

I tested by reducing the log level in the program, then running gdb, when i reached around log level 1, i got another error linked to line 9777 in pjsua2_wrap.cpp "SwigDirector_Account::onRegState(pj::OnRegStateParam &prm)"

seemed odd that it was some reference issues, so i tried removing the above command doing a manual swig compile, and it now works with USE_THREADS = True

used command
swig -doxygen -I../../../pjsip/include -I../../../pjlib/include -I../../../pjlib-util/include -I../../../pjmedia/include -I../../../pjnath/include -c++ -w312 -threads -python -o python/pjsua2_wrap.cpp pjsua2.i

@Axiinos
Copy link
Author

Axiinos commented Sep 12, 2024

further, i see the issue has been corrected in master branch, however the release version 2.14.1 has a older version, so users who download the release version will get -DSWIG_NO_EXPORT_ITERATOR_METHODS

"release 2.14.1"

USE_THREADS = -threads -DSWIG_NO_EXPORT_ITERATOR_METHODS

"master"

USE_THREADS = -threads
# In SWIG 4.2 the build will fail if we use this flag
#-DSWIG_NO_EXPORT_ITERATOR_METHODS
SWIG_FLAGS += -w312 $(USE_THREADS)

which seems to be quite an important note to have if you use swig 4.2 or higher and try to use python at the very least.

was there a reason behind not exporting iterator methods for python at some point and caused issues?

@sauwming
Copy link
Member

I had the same question in #3848, and quite frankly, I didn't know the answer myself.

Anyway, it's great that you could make it work. Let us know if there are still unresolved issues.

@Axiinos
Copy link
Author

Axiinos commented Sep 13, 2024

I may have found the cause why SWIG 4.2 causes issues with the arg.

looking at their 4.2 summery, they changed how STL container wrappers work, and they now use Python Iterator Protocol aswell, so the arg would essentially break it

image

image

so it may be advisable to do a check for which version of swig is used, pre 4.2 or 4.2 and above.

since it seems the arg is needed for pre 4.2.

found the reason it was added in the first place

f673a6e

@Axiinos
Copy link
Author

Axiinos commented Oct 1, 2024

I have been looking back at the importsym.py file, and there is something to me that doesnt make sense.

Two dir's have been hardcoded, that expects win32 to have a D:\ dir, or linux2 to have bennylp.

I have attempted to run the make importsym function in mingw-w64 "installed python2.7 to run it at all" only to be met with implicit declaration at first if i try to just "run it" with make, but getting overall errors trying to run the importsym function with pycparser "it seems to be heavily dependant on old versions of pycparser".

So i would like to suggest that it may be time to update the importsym.py to work by default with python3, cause that means users that want to compile via the make method wont have to do my method of using the workaround to even get swig to make a wrapper in the first place.

At the same time, it makes it less complicated to have it rewritten to conform Python3, you wont have to install 2 version of python to get it to just run the importsym.py functions, and it overall makes the dev environment safer, given python 2.7.18 was the last update to it, which means that installing 2.7 bloats the dev environment and adds a security risk overall to the system, at the same time it requires alot of hopping through loops to get it to run, with the exception of using the manual forced method with just running SWIG directly.

I am not an expert on the behind the scenes of how importsym works and it's functionality, but i do understand that the symbols are important and needed by SWIG to run pjsua2.i

"reference to the hardcoded dir's"

if sys.platform == 'win32':

I have tried multiple different versions of pycparser, and make edits here and there in an attempt to get it to run, but simply cannot on win10. "firstly due to the hardcoded dir, made slight modification to it in importsym.py "see below for how", second there is a fault with the symbol.lst it points to pjmedia/frames.h, it doesnt exist, however pjmedia/frame.h does. and third, when fixes are applied, the error pycparser throws is : pycparser.plyparser.ParseError: /usr/lib/gcc/x86_64-linux-gnu/13/include/stdarg.h:40:27: before: __gnuc_va_list"

I have made a pull request towards a solution i have made which can be viewed in #4093 (comment)

It is not a upgrade to python3 on the importsym, however it is more of a adaption to removing hardcoded paths and find the paths for users who have to use make symbols command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants
@trengginas @sauwming @Axiinos and others