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

Gtkwave update breaks verilator_ext_tests #1633

Closed
veripoolbot opened this issue Dec 10, 2019 · 1 comment
Closed

Gtkwave update breaks verilator_ext_tests #1633

veripoolbot opened this issue Dec 10, 2019 · 1 comment

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Dec 10, 2019


Author Name: Todd Strader (@toddstrader)
Original Redmine Issue: 1633 from https://www.veripool.org

Original Assignee: Wilson Snyder (@wsnyder)


t_gtkwave_diff is failing because gtkwave has updated fstapi.[ch]:
https://travis-ci.com/verilator/verilator_ext_tests/jobs/265626159

Some questions:
What is this repo we're looking at in verilator_ext_tests?
https://github.com/gtkwave/gtkwave
Did gtkwave move to GitHub? Google seems to think it's still on Sourceforge.

The test is mad about this gtkwave commit:
gtkwave/gtkwave@a6fd1e6
which adds Verilator helper functions. Is this anticipated?

This diff is mostly new functions, however there are other small changes. I can copy this into Verilator and check the regression tests if desired to make verilator_ext_tests happy. However, if gtkwave is really being hosted on GitHub now, should we just make gtkwave a submodule of Verilator? That way we can stop copying these files around and lose the consistency check.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 11, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-11T00:05:32Z


I had added the ext test to catch this sort of delta.

Gtkwave is now in github, that repo is the new master.

Until we have a more serious need, I'd like to avoid submodule requirements, as it adds friction which may reduce people's ability to install it.

Anyhow, the API change was on a request I had made for a performance enhancement, so updated everything to latest and updated our calling code for the new API.

Passes locally, presuming will satisfy Travis, if not reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.