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 Mac build, update GHA build to take into account latest Surelog compatibility changes, remove conda #485

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

timkpaine
Copy link

@timkpaine timkpaine commented Apr 14, 2023

Opening this for visibility, please don't "Approve and Run" the CI as I will continue to be noisy. I am testing actions on my fork.

This PR has been validated locally against a non-conda Surelog / UHDM and source-installed Yosys on MacOS 13, but is still very much a work-in-progress as I work through lots of CI changes

xref / supercedes: #445
xref: #431
xref: #432
xref: #365

@timkpaine

This comment was marked as outdated.

@timkpaine timkpaine force-pushed the tkp/mac branch 14 times, most recently from fea0d17 to b48b19d Compare April 16, 2023 17:41
run: |
sudo apt-get update
sudo apt-get install git g++-9 build-essential bison flex \
libreadline-dev gawk tcl-dev libffi-dev git graphviz xdot \
pkg-config libboost-system-dev libboost-python-dev \
libboost-filesystem-dev zlib1g-dev clang-format-8 cmake
libboost-filesystem-dev zlib1g-dev clang-format-8 cmake \
nlohmann-json3-dev iverilog
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aren't there googletest dependencies available in Ubuntu and homebrew that we can use directly from the package manager ?

That way we don't need the separate googletest compilation step below.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure if they compile to support c++ 17 string views which are needed, I can test though

@hzeller
Copy link
Collaborator

hzeller commented Apr 30, 2023

I think removing the gtest from third_party and then use what is on the system is a good improvement that can be done in a smallish, separate PR and reduces the size of this one.

@hzeller
Copy link
Collaborator

hzeller commented May 16, 2023

Should the workflow be run ?
Did you have time to break out the "don't use gtest from third_party/ but always in system" change ?

@timkpaine
Copy link
Author

sorry will be getting back to this shortly, been a busy couple months 😢

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@timkpaine timkpaine force-pushed the tkp/mac branch 8 times, most recently from 2410266 to d849f50 Compare September 4, 2023 13:34
@timkpaine timkpaine marked this pull request as ready for review September 4, 2023 13:37
@timkpaine
Copy link
Author

timkpaine commented Sep 4, 2023

This PR is ready, please do NOT enable the workflows yet as there appears to be a new (~1-2 days ago) issue with homebrew ccache that I want to resolve on my fork before making extra noise for everyone.

Given the size of this PR, even though the complexity is quite low, I've started breaking changes out into other PRs:

There are a few major follow-on issues to investigate:

  • this repo pinned yosys to 0.17 via a custom conda channel. We now pull direct from the repo (we could also pull from conda-forge, where yosys is now also available), but I left the version at 0.17 which is quite old now. This should be upgraded to the latest version if possible
  • enable similar changes on https://github.com/chipsalliance/systemverilog-plugin ?
  • package for various distribution channels (I will do homebrew and conda-forge)

@timkpaine
Copy link
Author

timkpaine commented Sep 15, 2023

Everything except the linux ql-qlf plugin is working and passing all tests.

I believe this is due to little differences is path handling, I hit a bunch of those for e.g. how icarus deals with include paths, etc.

All conda reliance is completely removed.
I also have to rebase after #546

If possible id like to get #543 and #544 in first

rename synth_quicklogic to synth_quicklogic_f4pga to not conflict with yosys internal

update gha to just use vendored surelog deps

build and install flatbuffers

install orderedmultidict for surelog

fix surelog build on mac

Add test assets to gitignore

add specific version of surelog
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.

synth_quicklogic name conflict macOS: There's no -D switch in BSD install
3 participants