-
Notifications
You must be signed in to change notification settings - Fork 82
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
feat(ci,eof): include eofwrap in EOF prerelease #962
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for cracking on with this @pdobacz!
Looks good so far, just a minor comment below.
ICYMI: Did you know that you test this action locally before pushing using nectos/act? See:
https://ethereum.github.io/execution-spec-tests/main/dev/test_actions_locally/
I am already using nectos/act (it's very flaky though, breaks 80% of runs because of network hiccups π€· ). I have the artifacts locally on disk and have eyeballed them for correctness. @danceratopz BTW, any clues what might have caused the cancel of the tag build? Can I rerun? I think it will draft the release under ipsilon fork so should not cause disruption? |
Shame about the flakiness, must admit I haven't used it for testing a release flow. Perhaps Mario has (but he's ooo at the mo).
Feel free to make a release whereever you like for "shorter" lived releases. If it will be a longer lasting and communicated to other clients, it's probably doing it from ethereum/execution-spec-tests (main?) for transparency. Below, I'm looking at this raw log from this run:
@winsvega had similiar issues building evmone relatively recently. I think he suspected that the runner was running out of memory while trying to build evmone. I've got a feeling it was stopping at exactly the same point in the build every time. I think he solved it in his Github Workflow by reducing the scope of evmone targets built? I managed to track down one of failing builds that he was struggling with: This failed in exactly the same line as your tag build above:
Def worth discussing with @winsvega in the chat. Probably unrelated and I know these are only deprecation warnings, but it's probably a good idea to clean these up? There's a lot of them ))
|
Ah yes, that might be it, thanks! When working locally I need to limit the parallelism in cmake build command. I'll do that in the CI maybe as well, I already have it for nektos/act's sake |
You don't need to build unittests in evmone. Select only the desired target (t8n, statetest runner) it saves the time and avoid this issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Glad you could clear up the evmone build issue so quickly. Thanks @winsvega for the help!
Feel free to merge, if/when you're happy with the releases generated!
ποΈ Description
Follows up #896 with a new build step which adds eofwrapped
ethereum/tests
to the EOF tests prerelease. Depends on #961 , hence opening as draft.π Related Issues
na
β Checklist