Draft: Steel Merge #1763
Draft: Steel Merge #1763
Conversation
There was a problem hiding this comment.
Excellent, thanks so much for this @gurukamath!
I would suggest splitting this into 3 PRs that:
- Removes the
ethereum-executiondependency.- I think this could be insta-merged. Is there any reason we couldn't merge this to
maintoday? cc @marioevz
- I think this could be insta-merged. Is there any reason we couldn't merge this to
- Adds a new transition tool class for the
ethereum-spec-evmand keeps the resolver.- This can be insta-merged to
mainand would allow filling with./testslocated (via git submodule) in EELS (by explicitly specifying the EELS t8n entrypoint).
- This can be insta-merged to
- Removes
ethereum-spec-evm-resolverand makes EELS the default.- We can't remove this until the
./testsmove out of EEST.
- We can't remove this until the
Rationale tldr: This allows us to test EEST as a package with ./tests/ in EELS while maintaining filling functionality in EEST until that point.
This also helps avoid accumulating changes into 1 large PR and allows us to get these changes merged incrementally.
Small comment about uv.lock below.
| @@ -10,27 +9,27 @@ resolution-markers = [ | |||
| name = "annotated-types" | |||
| version = "0.7.0" | |||
| source = { registry = "https://pypi.org/simple" } | |||
| sdist = { url = "https://files.pythonhosted.org/packages/ee/67/531ea369ba64dcff5ec9c3402f9f51bf748cec26dde048a2f973a4eea7f5/annotated_types-0.7.0.tar.gz", hash = "sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89", size = 16081, upload-time = "2024-05-20T21:33:25.928Z" } | |||
| sdist = { url = "https://files.pythonhosted.org/packages/ee/67/531ea369ba64dcff5ec9c3402f9f51bf748cec26dde048a2f973a4eea7f5/annotated_types-0.7.0.tar.gz", hash = "sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89", size = 16081 } | |||
There was a problem hiding this comment.
Could you please bump your uv version and replay your upgrade commands to avoid removing the upload-time fields that will be added back by someone updating uv.lock with a newer uv?
If you curl-installed uv, you can do uv self update.
Explanation here:
| else: | ||
| # improve behavior of which by resolving the path: ~/relative paths don't work | ||
| resolved_path = Path(os.path.expanduser(binary)).resolve() | ||
| if resolved_path.exists(): | ||
| binary = resolved_path | ||
| return | ||
|
|
||
| # improve behavior of which by resolving the path: ~/relative paths don't work | ||
| resolved_path = Path(os.path.expanduser(binary)).resolve() | ||
| if resolved_path.exists(): | ||
| binary = resolved_path | ||
| binary = shutil.which(binary) # type: ignore | ||
| if not binary: | ||
| raise CLINotFoundInPathError(binary=binary) |
There was a problem hiding this comment.
This looks like it'll give bad ux/unexpected errors for non-eels tools that don't get found in the PATH. Would need to test.
🗒️ Description
This PR has some of the changes needed to be able to perform the EEST and EELS merge. Has two distinct components to it
evm_binisn't explicitly provided in the command lineThis merge unlocks great developer experience features while debugging. It also removes the need for an intermediate eels resolver.
Related EELS PR
🔗 Related Issues
✅ Checklist
mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.