Skip to content

Conversation

@donn
Copy link
Contributor

@donn donn commented Oct 18, 2025

What are the reasons/motivation for this change?

@Ravenslofty noted that unlike the previous setup, users may have to manually manage a virtual environment to get the new Python build dependencies (pybind11 and cxxheaderparser,) and that is cumbersome.

While pybind11 can in fact be installed via package managers, it tends to be out of date more often than not. cxxheaderparser is not in any package manager that isn't Nix.

I considered just handling the ImportError to instruct the user on creating a venv but that's also just kind of cumbersome.

Explain how this is achieved.

  • add tox to list of dependencies: saves builders from manually having to manage a venv for python build dependencies
    • when building wheels, pip automatically creates the environment with those dependencies, so no need for tox
    • when running simply make ENABLE_PYOSYS=1, this is not the case. people attempting to pip3 install --upgrade pybind11 cxxheaderparser to add it to their system packages will be met with a scare message about "breaking system packages"
    • tox is available from all major package managers all the way back to ubuntu 20.04 and resolves this issue
    • if someone really prefers to provide their own dependencies (such as within Nix,) passing PYOSYS_USE_TOX=0 suppresses this behavior
  • update installation instructions to drop boost and add tox instead

Naturally, tox is not required if you're not building Yosys with Python support.

Make sure your change comes with tests. If not possible, share how a reviewer might evaluate it.

On an Ubuntu 22.04 installation (Ubuntu 20.04 also in theory but the Bison on there is out of date so you have to build that on your own), apt-get install build-essential clang lld bison flex libfl-dev libreadline-dev gawk tcl-dev libffi-dev graphviz xdot pkg-config python3-dev tox zlib1g-dev should be enough to allow make ENABLE_PYOSYS=1 to build successfully without further intervention.

I validated that statement as follows:

podman run --rm -ti docker.io/ubuntu:22.04 bash -c "
    apt-get update &&
    DEBIAN_FRONTEND=noninteractive apt-get install -y git build-essential clang lld bison flex libfl-dev libreadline-dev gawk tcl-dev libffi-dev graphviz xdot pkg-config python3-dev tox zlib1g-dev &&
    git clone --recursive --branch pyosys_tweaks https://github.com/donn/yosys &&
    cd yosys &&
    make ENABLE_PYOSYS=1 -j\$(nproc) &&
    ./yosys -H
"

@donn donn changed the title pyosys: tox for non-wheels builds, update instructions pyosys: tox for non-wheel builds, update instructions Oct 18, 2025
@Ravenslofty Ravenslofty self-assigned this Oct 18, 2025
Copy link
Collaborator

@Ravenslofty Ravenslofty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ git reset --hard
HEAD is now at 7193598fb pyosys: `tox` for non-wheels builds, update instructions
$ make clean
[...]
$ git diff --stat
 kernel/binding.h   |   60 -----
 kernel/celltypes.h |  556 --------------------------------------------
 kernel/consteval.h |  426 ----------------------------------
 kernel/cost.h      |   99 --------
 kernel/log.h       |  528 ------------------------------------------
 kernel/register.h  |  179 ---------------
 kernel/rtlil.h     | 2447 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 kernel/yosys.h     |  103 ---------
 libs/sha1/sha1.h   |   57 -----
 9 files changed, 4455 deletions(-)

(we're discussing this privately, but I want to clarify I don't think this is presently ready to merge)

- add `tox` to list of dependencies: saves builder(s) from manually having to manage a venv for python build dependencies
  - when building wheels, pip automatically creates the environment with those dependencies, so no need for tox
  - when running simply `make ENABLE_PYOSYS=1`, this is not the case. people attempting to `pip3 install --upgrade pybind11 cxxheaderparser` to add it to their system packages will be met with a scare message about "breaking system packages"
  - tox is available from all major package managers all the way back to ubuntu 20.04 and resolves this issue
- update installation instructions to drop boost and add tox instead
- update ci scripts to use `macos-15[-intel]` (`macos-13` sunset in early december)
@donn
Copy link
Contributor Author

donn commented Oct 18, 2025

Rebased on main to pull in #5435

Copy link
Collaborator

@Ravenslofty Ravenslofty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It builds now; might wait for a JF to get more eyes on this though.

@Ravenslofty Ravenslofty dismissed their stale review October 18, 2025 14:19

(now working after #5435)

Copy link
Collaborator

@Ravenslofty Ravenslofty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the JF it was decided that we'd prefer uv over tox for this process.

@donn donn marked this pull request as draft October 22, 2025 03:46
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.

3 participants