python-vlc icon indicating copy to clipboard operation
python-vlc copied to clipboard

Rewrite of the bindings' generator using Tree-sitter

Open yanns1 opened this issue 1 year ago • 3 comments

The most important change is the rewrite of Parser using Tree-sitter. The goal is to make the parsing of libvlc's header files more robust, maintainable and powerful than the current regex-based parsing.

In addition, a few secondary improvements have been made:

  • The use of Tree-sitter allowed us to handle previously blacklisted, overridden or even ignored items. This includes functions in _blacklist (see generate.py), nested structs and unions and function pointers as function parameter or struct/union field. We thus made some changes to the generator (PythonGenerator) to take advantage of the newly parsed items, and actually generate bindings for them.
  • We moved from epydoc to Sphinx for documentation generation. epydoc becoming obsolete and Sphinx becoming the tool of choice in Python, we made the change. We thus had to convert libvlc source code comments written in Doxygen to the default Sphinx docstring format (see methods docs_in_sphinx_format).
  • On the development side, we put in place precommit hooks and Github actions to ensure that the code is properly formatted, has no errors and passes tests. We also made a little script (dev_setup.py) to quickly setup the project for new contributors (this is documented in the generator's README).

yanns1 avatar May 17 '24 12:05 yanns1

Great! But... I tried the described method (on Debian/testing) python3 dev_setup.py failed with

Clone vendored C Tree-sitter grammar... Done.
Create a virtual environment in .venv... Done.
Upgrade pip... Traceback (most recent call last):
  File "/home/oaubert/doc/projects/mast/ptrans/python-vlc/dev_setup.py", line 39, in <module>
    proc = run(cmd, capture_output=True, check=True)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['.venv/bin/python3', '-m', 'pip', 'install', '--upgrade', 'pip']' returned non-zero exit status 1.
(.venv) oaubert@gnozyme:mast/ptrans/python-vlc>which python3
/home/oaubert/doc/projects/mast/ptrans/python-vlc/.venv/bin/python3
(.venv) oaubert@gnozyme:mast/ptrans/python-vlc>python3 -m pip install --upgrade pip
ERROR: Can not perform a '--user' install. User site-packages are not visible in this virtualenv.

I could solve this by editing .venv/pyvenv and changing include-system-site-packages to true, but it is not quite satisfactory.

The first point would be to have the dev_setup.py display error messages when there are errors, and do some more checks: do not execute venv if it is already created, etc. Maybe a plain shell script would be easier. I tried in a python3 docker image, and the devscript executed correctly. But the make failed with a OSError: build/c.so: cannot open shared object file: No such file or directory Indeed, build is a link to /home/oaubert/.cache/tree-sitter/lib which does not exist.

oaubert avatar May 17 '24 16:05 oaubert

Ok for making better errors and checking if venv already present etc.

We made a Python script so as to have one script work on multiple OS, but indeed a shell script might be easier.

For the error on pip upgrade, I suspect it is because the environment variable PIP_USER is set to 1/true (did you run it in Docker? because others seem to have the issue to, see https://stackoverflow.com/q/73962053). Solution: env PIP_USER=false in your Dockerfile (similar issue: https://github.com/gitpod-io/gitpod/issues/1997)

For the OSError, I don't understand how build became a link. This is the method Language.build_library from py-tree-sitter that create build if necessary when creating the parser c.so.

yanns1 avatar May 17 '24 16:05 yanns1

My bad for the OSError - I had modified generate.py some time ago, and it kept it.

oaubert avatar May 17 '24 18:05 oaubert