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

Support using sphinxcontrib-hdl-diagrams on MSYS2 #73

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

umarcor
Copy link
Member

@umarcor umarcor commented Apr 21, 2021

This was significantly more painful than I had expected, but I got something.

In this PR, 5 modifications are contributed:

  • A CI workflow named MSYS2.
    • netlistsvg is installed on the host, using the NodeJs available on the environment.
    • MSYS2 is setup with Action setup-msys2, and the following packages are installed through pacman: gcc, git, make, *-python-lxml, *-python-pip, *-python-sphinx, *-python-wheel, *-yosys.
      • All of them, except make, *-python-sphinx and *-yosys, are required because sphinx_symbiflow_theme is installed from sources, instead of using a package from pip.
    • nmigen and sphinx_symbiflow_theme are installed using pip.
    • The paths of nodejs and netlistsvg are added to the PATH, and the docs are built.
    • Artifacts are uploaded.
  • docs/Makefile had to be modified, for avoiding the Conda environment setup. Target html was added. make -C docs html allows building the docs using locally available tools.
  • docs/conf.py: in order to use the yosys installation on the system, hdl_diagram_yosys = 'system' is added.
  • yosys-bb and yosys-aig types produce failures. I think this is not related to this plugin, but an issue/bug that Yosys has internally.
  • Passing the yosys command as an string in sphinxcontrib_hdl_diagrams/__init__.py is broken. I modified it for using a list of arguments instead.

Therefore, I will mark this PR as a draft. The following issues need to be solved:

  • Currently, the paths of nodejs and netlistsvg are added to the PATH manually (hardcoded). Those are saved in a file named extra.paths, but I could not have them added to the PATH programmatically. I have problems when reading C:/Program Files/nodejs, because of the space between "Program" and "Files" and because of the /. See step "Build Docs" of the workflow.
  • In docs/Makefile, I think that SPHINXBUILD should not setup Conda by default. That's non idiomatic. Instead SPHINXBUILD should allow a regular/typical Sphinx build; what any user would expect in any Sphinx other project. Then, a custom target or envvar can be used for explicitly enabling Conda.
  • Need to investigate why Yosys fails when doing dot ... && move ... internally. To start with, move seems not to exist.
  • I believe that passing the yosys command as a list, instead of an string, can be considered a fix to be applied independently of this PR. @mithro @nobodywasishere @daniellimws, what do you think?

Ref: #72

@nobodywasishere
Copy link

nobodywasishere commented Apr 23, 2021

I'm running into issues with passing the yosys command as a list to subprocess.check_output(). What errors were you running into using it as it is? For some reason on my machine if it's passed a list, it will only run the first element of the list and ignore everything else.

For instance:

Python 3.9.3 (default, Apr  8 2021, 23:35:02) 
[GCC 10.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess as sp
>>> sp.check_output('yosys --help', shell=True)
b'\nUsage: yosys [options] [<infile> [..]]\n\n    -Q\n        suppress printing of banner (copyright, disclaimer, version)\n\n    -T\n        
... # returns normally
>>> sp.check_output(['yosys --help'], shell=True)
b'\nUsage: yosys [options] [<infile> [..]]\n\n    -Q\n        suppress printing of banner (copyright, disclaimer, version)\n\n    -T\n        
... # returns normally
>>> sp.check_output(['yosys', '--help'], shell=True)
# I press CTRL + D as it's stuck at the yosys prompt
... 
Yosys 0.9+4052 (open-tool-forge build) (git sha1 dce037a6, gcc 9.3.0-17ubuntu1~20.04 -Os)\n\n\nyosys> exit\nEnd of script. 
...
>>> sp.check_output(['echo look at me', 'yosys', '--help'], shell=True)
b'look at me\n'
# returns normally
>>>

It does this when I try build the docs (which uses conda python) as well.

@umarcor
Copy link
Member Author

umarcor commented Apr 27, 2021

@nobodywasishere. try with shell=False. I tried the following:

#!/usr/bin/env python3

import subprocess as sp
print('First')
print(sp.check_output(['yosys', '--version'], shell=True).decode("utf-8"))
print('Second')
print(sp.check_output('yosys --version', shell=True).decode("utf-8"))
print('Third')
print(sp.check_output(['yosys', '--version'], shell=False).decode("utf-8"))
print('Fourth')
print(sp.check_output('yosys --version', shell=False).decode("utf-8"))

On MSYS2 (MINGW64):

# ./test.py 
First
Yosys 0.9+4052 (git sha1 0ccc7229c, x86_64-w64-mingw32-g++ 10.2.0 -march=x86-64 -mtune=generic -O2 -Os)

Second
Yosys 0.9+4052 (git sha1 0ccc7229c, x86_64-w64-mingw32-g++ 10.2.0 -march=x86-64 -mtune=generic -O2 -Os)

Third
Yosys 0.9+4052 (git sha1 0ccc7229c, x86_64-w64-mingw32-g++ 10.2.0 -march=x86-64 -mtune=generic -O2 -Os)

Fourth
Yosys 0.9+4052 (git sha1 0ccc7229c, x86_64-w64-mingw32-g++ 10.2.0 -march=x86-64 -mtune=generic -O2 -Os)

On a container (hdlc/yosys):

First
# Is frozen...

Second
Yosys 0.9+4052 (git sha1 a5adb007, clang 7.0.1-8+deb10u2 -fPIC -Os)

Third
Yosys 0.9+4052 (git sha1 a5adb007, clang 7.0.1-8+deb10u2 -fPIC -Os)

Fourth
Traceback (most recent call last):
  File "./test.py", line 11, in <module>
    print(sp.check_output('yosys --version', shell=False).decode("utf-8"))
  File "/usr/lib/python3.7/subprocess.py", line 395, in check_output
    **kwargs).stdout
  File "/usr/lib/python3.7/subprocess.py", line 472, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib/python3.7/subprocess.py", line 775, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.7/subprocess.py", line 1522, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'yosys --version': 'yosys --version'

So, on Linux it seems that the list requires Shell false and the string needs shell true.

@umarcor
Copy link
Member Author

umarcor commented Apr 27, 2021

What errors were you running into using it as it is?

This is running make -C docs html on branch https://github.com/umarcor/sphinxcontrib-hdl-diagrams/commits/make/msys2:

...
reading sources... [100%] index
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... WARNING: unsupported theme option 'color_primary' given
WARNING: unsupported theme option 'color_accent' given
done
Exiting file: T:/symbiflow/sphinxcontrib-hdl-diagrams/docs/_build/html/_images/hdl-diagram-52-code-verilog-counter.svg
Exiting file: T:/symbiflow/sphinxcontrib-hdl-diagrams/docs/_build/html/_images/hdl-diagram-70-code-nmigen-counter.svg
Exiting file: T:/symbiflow/sphinxcontrib-hdl-diagrams/docs/_build/html/_images/hdl-diagram-100-code-rtlil-counter.svg
Running yosys: yosys -p 'prep -top top ; cd top; ; show -format svg -prefix T:/symbiflow/sphinxcontrib-hdl-diagrams/docs/_build/html/_images/hdl-diagram-123-code-verilog-dff' T:/symbiflow/sphinxcontrib-hdl-diagrams/docs/code/verilog/dff.v
[00000.012737] ERROR: Can't guess frontend for input file `top' (missing -f option)!

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

Successfully merging this pull request may close these issues.

None yet

2 participants