Optional Echidna compatibility check as a Github Action#1039
Conversation
|
This would be really nice actually! So we know when we break Echidna :) |
2bd2edd to
7ab28fd
Compare
|
The one from echidna just <<loop>>s
|
Btw, it is expected that this fails as echidna fails with the latest hevm revision |
|
OH, it's good now? I'll review today :) Thanks! |
|
@blishko do you also wanna have a go at this? I'm actually OK with it -- it doesn't add anything to hevm, only adds another hook to build. Ultimately, I think it's fine as long as it doesn't break by accident, and only breaks if it really ought to break because we broke Echidna :D |
.github/workflows/echidna-compat.yml
Outdated
| if 'extra-deps' in data: | ||
| new_deps = [] | ||
| found_hevm = False | ||
| for dep in data['extra-deps']: | ||
| # Identify if this is the remote hevm dependency | ||
| is_hevm = (isinstance(dep, dict) | ||
| and 'git' in dep | ||
| and 'hevm' in dep['git']) | ||
| if is_hevm: | ||
| if not found_hevm: | ||
| new_deps.append('./hevm') | ||
| found_hevm = True | ||
| continue | ||
| if dep == './hevm': | ||
| found_hevm = True | ||
| new_deps.append(dep) | ||
| continue | ||
| new_deps.append(dep) | ||
|
|
||
| if not found_hevm: | ||
| # Insert hevm at the beginning of extra-deps | ||
| new_deps.insert(0, './hevm') | ||
|
|
||
| data['extra-deps'] = new_deps |
There was a problem hiding this comment.
Why is this complicated logic needed?
Isn't the goal just to point echidna to the local hevm version? Isn't there a simpler way to do it?
There was a problem hiding this comment.
I think there is a simpler way to do it using awk/sed but it won't be robust if we change some newlines/comments. Maybe @elopez has more ideas.
There was a problem hiding this comment.
Why not just always prepend ./hevm to existing dependencies? Why all the checks?
There was a problem hiding this comment.
hevm is an existing dependency referenced via git, so then there would be two hevms listed potentially with the same version number. But I agree this script is too complicated for what it does. I simplified it a bunch on the latest commit.
There was a problem hiding this comment.
Great, thanks! This version is much more understandable!
|
Wouldn't it be better to just fail the workflow instead of marking it as successful and just posting a comment? |
|
@blishko this check is optional to avoid imposing a new burden to the hevm team. You can decide to fail the workflow in the future if you feel the ABI is stable enough. |
|
I would prefer the strong indication that this workflow is failing. |
|
I removed the comment stuff now. The only con of just letting it fail is that whenever a breaking change is introduced to hevm, all commits, PRs, etc will then start to be marked with ❌ in the GitHub UI. That might make it hard to tell apart actual failures that need attention from the known compatibility breakage. |
|
Having the breakage be more visible is better in this case, but that is my personal opinion. |
|
BTW, what was the reason for the current breakage? Do you plan to fix it in Echidna soon? |
|
I think we can squash this to a single commit, no? |
|
Yes |
Description
The workflow is located at hevm/.github/workflows/echidna-compat.yml. It performs the following steps when a Pull Request is opened or updated in the hevm repository:
Optional Status: The job uses continue-on-error: true, which means it will act as a warning. If compatibility is broken, the PR will show a warning icon but won't be blocked (unless you specifically mark this check as required in GitHub settings). The job will also post a status update with a build log in case the build fails.
Checklist