Bytecode comparison PR scripts for solc-bin #10838
Conversation
cameel
force-pushed
the
solc-bin-bytecode-comparison-pr-check-scripts
branch
4 times, most recently
from
bc6421e to
b2c8425
Compare
cameel
force-pushed
the
more-features-in-prepare-report
branch
from
5b85da1 to
58824db
Compare
cameel
force-pushed
the
solc-bin-bytecode-comparison-pr-check-scripts
branch
from
b2c8425 to
23bfdba
Compare
cameel
force-pushed
the
more-features-in-prepare-report
branch
from
58824db to
5f81bb2
Compare
cameel
force-pushed
the
solc-bin-bytecode-comparison-pr-check-scripts
branch
2 times, most recently
from
495b2e7 to
ac9b715
Compare
|
#10680 has been merged so this is now ready for review. |
ekpyron
left a comment
ekpyron
left a comment
There was a problem hiding this comment.
Phew... I can't say I'm absolutely confident that this really works... but before it hangs in limbo forever, maybe we can just merge it and see what happens ;-)? I trust you tested it and we can reconfirm that it works in a test PR later on, right?
| num_failed_comparisons=0 | ||
| for solidity_version_and_commit in $versions_in_report_names; do | ||
| echo "Comparing reports for Solidity ${solidity_version_and_commit}:" | ||
| mapfile -t report_files_for_version < <( |
There was a problem hiding this comment.
mapfile... fancy stuff... never saw it before :-)...
There was a problem hiding this comment.
My feeling reading @cameel 's bash and python PRs
There was a problem hiding this comment.
That's just the bad influence of shellcheck dragging me to the dark side :) See SC2207.
I could get away with simpler stuff while this was sitting in the github action file but now that I moved it to the main repo I had to obey :)
cameel
force-pushed
the
solc-bin-bytecode-comparison-pr-check-scripts
branch
from
ac9b715 to
800c338
Compare
ekpyron
left a comment
ekpyron
left a comment
There was a problem hiding this comment.
Reapproving (still largely based on the fact that we'll be able to stress-test it post-facto ;-)).
|
Yeah, argotorg/solc-bin#76 was actually a huge stress test for it. The PR sits on top of the bytecode compare PR check (argotorg/solc-bin#77) so these scripts were executed for all macOS binaries between 0.4.0 and 0.6.0. I also ran it on the whole 0.3.6-0.8.0 range separately just to be sure it works for all the old versions. You can take a look at https://github.com/ethereum/solc-bin/pull/76/checks?check_run_id=1761261024 to see how it behaves in practice. |
Depends on #10680.It's ondevelopnow.The scripts added here contain the main logic of the bytecode comparison PR check in argotorg/solc-bin#77. The scripts assume the availability of extra features added to
prepare_report.pyandprepare_report.jsin #10680, #10679 and #10676.The code was originally directly in the Github action but it grew enough to warrant putting it in standalone scripts. The PR creates
scripts/solc-bin/directory in solidity repo and I think we should start putting scripts used by solc-bin there. The downside of such a policy is that every time you want to run a script in solc-bin CI, you need to clone the solidity repo, which is sometimes an unnecessary overhead. In this case, however, the PR check needs a clone anyway for other purposes and the upside is that we avoid duplicating the CI infrastructure around scripts, e.g. shellcheck.