solidity
solidity copied to clipboard
ethereum
Upgrade to evmone 0.9.1 and hera 0.6.0 in buildpack
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:1c3f2444c7ab72b70cad8d06cb78f166f8a034aea77141fe8a1142242a2a0e9d].
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004.clang-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:33fe883a3dde565eeec7daf9070d5421f6a45e4a16c712d2274593095644c064].
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004.clang-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:574aeceb96ac0a8300bb9f065781b5b43bb083958a802b3d097b8a475a5becb4].
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:d396d91afec9626442a1479e25d7b2b0546126f9bc4f747bdec0111e4ffdc853].
Is this PR independent of other changes? I.e. can it be merged without bumping the EVMC API version?
Well it is independent assuming you don't use these buildpack versions. The EVMC ABI is breaking so they can't be used without the other PR.
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:82fb99fe88da0133b5366385219700ca19c3042dda210d42061ca4612d8724b6].
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004.clang-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:b7129519d5028b753693bca8dc71fceb874b082e409f35d68e15644f837df95b].
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004.clang-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:c272af6813dd14d70532a2a7505f0629f42940fde6757a78c9a3bb6f18e09833].
solbuildpackpusher/solidity-buildpack-deps:ubuntu1604.clang.ossfuzz-20 [solbuildpackpusher/solidity-buildpack-deps@sha256:2a3edda109c40165aa0c4123f1fd6b1e75da2e610797720f9e6a389b2870a095].
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:f609a27be985e3dae160116139b471252a34cd0d3dc949a8a86be84b8c106f7b].
@axic install_evmone.ps1 powershell script for Windows also pulls the 0.8.0 evmone, although this script doesn't seem to be used in any of the CI steps, so my assumption is that it's just a convenience script for building the compiler on Windows.
Should this also be bumped? @cameel
@nikola-matic The script is used by t_win_soltest.
But yeah, I think that the version should be bumped everywhere.
I've rebased against fresh develop to get this into a merge-able state. The PR is approved, however I see @bshastry would prefer for this to stay open until #13532 is close to being merged. With our current push to trim our PR queue, I'd be fine with merging this right away. @bshastry, @cameel, any counter points?
I've rebased against fresh develop to get this into a merge-able state.
Oh, that means now need to change the hashes in the other PR. These two PRs should only be merged/rebased at the same time, they are unfortunately not independent. The reason they are split is due to the way buildpack works.
I've rebased against fresh develop to get this into a merge-able state.
Oh, that means now need to change the hashes in the other PR. These two PRs should only be merged/rebased at the same time, they are unfortunately not independent. The reason they are split is due to the way buildpack works.
Sure thing, although if this isn't merge-able, then the PR should be converted to a draft so it's clear to everyone. At least the rebase + force push dismissed @cameel's approval.
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:e4f83457bf1d6475c3189e9013da77289793a5ecd6a0e15dbec9411880b11e22].
solbuildpackpusher/solidity-buildpack-deps:ubuntu2004.clang-15 [solbuildpackpusher/solidity-buildpack-deps@sha256:8dda4fdae312f840fbb4e25b9ef01ad3209e9014e49e4564ab0f0d2510225131].
solbuildpackpusher/solidity-buildpack-deps:ubuntu1604.clang.ossfuzz-20 [solbuildpackpusher/solidity-buildpack-deps@sha256:7b3ccaed3b5d37dc2cc4bbe1a1e40949266292dcdbc3ad015271ab4e5e6f5038].
I've converted this to a draft as it should be merged together with https://github.com/ethereum/solidity/pull/13532. @axic feel free to revert this to a review-ready PR if needed.
Well, it's not mergeable but it's ready so we don't want people to skip reviewing it. So perhaps has dependencies label would be better here. These two PRs have a cyclic dependency on each other :)
Well, it's not mergeable but it's ready so we don't want people to skip reviewing it. So perhaps
has dependencieslabel would be better here. These two PRs have a cyclic dependency on each other :)
Yup, I though about adding the has dependencies label, but at the same time the circular dependency made me think otherwise. The PR that does depend on this however, is in fact a draft, so really no idea what to think about this :)
I mean, I don't consider it a draft because there's nothing more to do in it. It's done.
@cameel the only risk is merging this and then due to some other package update the hashes are changing in circle will cause a failure due to incompatible EVMC ABIs. If we expect the other PR to be merged very soon then it is safe to merge this, otherwise not.
@cameel the only risk is merging this and then due to some other package update the hashes are changing in circle will cause a failure due to incompatible EVMC ABIs. If we expect the other PR to be merged very soon then it is safe to merge this, otherwise not.
So that's kinda the reason I converted it to a draft - mainly so that it blocks merging before the other PR is ready as well. Not sure what to do know. You decide @axic, you're the boss.
If we expect the other PR to be merged very soon then it is safe to merge this, otherwise not.
Should be pretty soon - the only blocker was this gas cost change to investigate. I see you pushed new commits there. So is that resolved?
But I'd still keep this open and merge them both at the same time.
So that's kinda the reason I converted it to a draft - mainly so that it blocks merging before the other PR is ready as well. Not sure what to do know. You decide @axic, you're the boss.
At this point it's moot since this PR is already approved so either way achieves the goal. But my point was more in general - if you want reviews on such a PR it's probably better not to have it marked as draft so has dependencies would work better :) That label is a warning that the PR cannot be "just" merged.
For the record, I'd also tend to keep these things drafts to prevent accidental merging, until both PRs are ready :-). Draft meaning "cannot be merged" here, which is true for either of them, regardless of reviews, unless both of them are ready.
