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

Refactor Verilator building in Travis #1547

Closed
veripoolbot opened this issue Oct 11, 2019 · 3 comments
Closed

Refactor Verilator building in Travis #1547

veripoolbot opened this issue Oct 11, 2019 · 3 comments

Comments

@veripoolbot
Copy link
Contributor

@veripoolbot veripoolbot commented Oct 11, 2019


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

Original Assignee: Todd Strader (@toddstrader)


Currently, for verilator CI we are caching object files when building Verilator but for verilator_ext_tests we are doing that and caching the binaries themselves. We're also doing this in a separate build stage. This allows us to skip the entire build if we've already built the Git revision that we're on. It also allows all subsequent Travis stages to share the Verilator build.

I've now brought all of that into the verilator CI environment:
https://github.com/toddstrader/verilator-dev/tree/travis-stages

Two things which could be landed as separate commits (or not, just let me know):
t_a4_examples was using the root Makefile which will cause Verilator to re-build if we're pulling from cache and the intermediate objects don't exist. I've changed it so that this won't happen which is how the rest of the regression suite behaves.

driver.pl was modified to report what tests are running if it doesn't report anything or five minutes. This is how I discovered what was going on with t_a4_examples. I could push some of this to Parallel::Forker, but then we'd need to bump the version of that module, etc., etc. But if that's desirable I can go that route instead.

All of this allows for more streamlined builds (especially when there is contention on Travis). But even more so it allows users of Verilator (e.g. verilator_ext_tests, SweRV, cocotb) to be able to use ci/build_verilator.sh for themselves thus simplifying users' CI setups. I've done this in verilator_ext_tests here:
https://github.com/toddstrader/verilator_ext_tests/tree/vlt-build

The verilator_ext_tests branch has a commit that I will not push to trunk but was necessary to test this against Travis while in this intermediate state.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 15, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-10-15T22:09:28Z


These look good. Only nit I would suggest is in the new shell scripts indent the # comments to match the block they are in rather than left-aligned.

Agree it's good to split out the driver/a4 change from ci.

Feel free to squash & push all these.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 17, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-10-17T03:18:45Z


Note I tweaked the driver timeout logic to look more like previous - your change meant it printed the running tests every status message, I believe the intent was to print them only when no tests have finished in the last 30 seconds.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 17, 2019


Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2019-10-17T09:40:47Z


Ugh, thanks. I was only testing with --dist which appears to be why I didn't notice this. Also, I was going for a longer timeout on printing the running tests, but this is good too. Basically the problem is that Travis appears to kill a job if it doesn't update STDOUT/STDERR for 10 minutes. So it's good to know what test is hung when that happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.