fix(op-deployer): avoid embedded artifacts recompilation #17699
Conversation
serpixel
requested review from
tynes and
bitwiseguy
and removed request for
a team
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #17699 +/- ##
===========================================
+ Coverage 74.31% 80.72% +6.41%
===========================================
Files 172 117 -55
Lines 10955 6170 -4785
===========================================
- Hits 8141 4981 -3160
+ Misses 2670 1189 -1481
+ Partials 144 0 -144
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
serpixel
force-pushed
the
feat/op-deployer/avoid-embedded-artifacts-recompilation
branch
from
b7c348d to
3495f92
Compare
serpixel
requested review from
stevennevins and
ajsutton
and removed request for
a team
| outFile = flag.String("out", "", "path to output tzst") | ||
| ) | ||
|
|
||
| func main() { |
There was a problem hiding this comment.
Would be good to add some documentation (via small comment block) explaining what this program does. I'm curious what made you go with the golang program approach instead of via bash commands in the justfile as originally implemented. Maybe the tar cli command didn't provide enough configurability?
There was a problem hiding this comment.
Sadly zstd is not available through mise, we would need to ask anyone to install it on their OS using a package manager or recompile it. Using a go dependency is the less troublesome way to get zstd.
| copy-contract-artifacts: | ||
| rm -f ./pkg/deployer/artifacts/forge-artifacts/artifacts.tgz | ||
| tar -cvzf ./pkg/deployer/artifacts/forge-artifacts/artifacts.tgz -C ../packages/contracts-bedrock/forge-artifacts --exclude="*.t.sol" . | ||
| #!/bin/bash | ||
| set -euo pipefail | ||
|
|
||
| rm -f ./pkg/deployer/artifacts/forge-artifacts/artifacts.tzst | ||
| go run ./pkg/deployer/artifacts/cmd/mktar \ | ||
| -base ../packages/contracts-bedrock \ | ||
| -out ./pkg/deployer/artifacts/forge-artifacts/artifacts.tzst |
There was a problem hiding this comment.
Currently there is a publish-contract-artifacts job in ci (ref). It expects to store a .tar.gz file type in our gcp bucket (ref). Seems like the filetype update would break that script and anything that expects to download .tar.gz files from that bucket). I'm not sure what downstream tools depend on downloading from that gcp bucket
There was a problem hiding this comment.
Actually it seems that the script it calls, publish-artifacts.sh, does recompress the files itself so it wouldn't be breaking this. Same for pull-artifacts.sh, it's still also using gzip.
There was a problem hiding this comment.
Ok got it. And op-deployer has code to decompress either file type so seems we're compatible with either on the op-deployer side?
Description
Right now the artifacts get recompiled every time when using forge. This is an impediment to completing the forge migration.
Tests
Additional context
Metadata